[oilpan] Move conservative marking from HeapPage to MarkingVisitor

Bug: chromium:757440
Change-Id: Ibb3961a8ccf96578e4b5493714a79a0a55411025
Reviewed-on: https://chromium-review.googlesource.com/968506
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544035}

third_party/WebKit/Source/platform/heap/BlinkGC.h

@@ -14,6 +14,7 @@
 
 namespace blink {
 
+class HeapObjectHeader;
 class MarkingVisitor;
 class Visitor;
 class ScriptWrappableVisitor;
@@ -30,6 +31,12 @@ using EphemeronCallback = VisitorCallback;
 using MissedWriteBarrierCallback = void (*)();
 using NameCallback = const char* (*)(const void* self);
 
+// Callback used for unit testing the marking of conservative pointers
+// (|CheckAndMarkPointer|). For each pointer that has been discovered to point
+// to a heap object, the callback is invoked with a pointer to its header. If
+// the callback returns true, the object will not be marked.
+using MarkedPointerCallbackForTesting = bool (*)(HeapObjectHeader*);
+
 // Simple alias to avoid heap compaction type signatures turning into
 // a sea of generic |void*|s.
 using MovableReference = void*;

third_party/WebKit/Source/platform/heap/Heap.cpp

@@ -178,7 +178,7 @@ Address ThreadHeap::CheckAndMarkPointer(MarkingVisitor* visitor,
 #endif
     DCHECK(!heap_does_not_contain_cache_->Lookup(address));
     DCHECK(&visitor->Heap() == &page->Arena()->GetThreadState()->Heap());
-    page->CheckAndMarkPointer(visitor, address);
+    visitor->ConservativelyMarkAddress(page, address);
     return address;
   }
 
@@ -205,7 +205,7 @@ Address ThreadHeap::CheckAndMarkPointer(
     DCHECK(page->Contains(address));
     DCHECK(!heap_does_not_contain_cache_->Lookup(address));
     DCHECK(&visitor->Heap() == &page->Arena()->GetThreadState()->Heap());
-    page->CheckAndMarkPointer(visitor, address, callback);
+    visitor->ConservativelyMarkAddress(page, address, callback);
     return address;
   }
   if (!heap_does_not_contain_cache_->Lookup(address))

third_party/WebKit/Source/platform/heap/HeapPage.cpp

@@ -38,7 +38,6 @@
 #include "platform/heap/CallbackStack.h"
 #include "platform/heap/HeapCompact.h"
 #include "platform/heap/MarkingVerifier.h"
-#include "platform/heap/MarkingVisitor.h"
 #include "platform/heap/PageMemory.h"
 #include "platform/heap/PagePool.h"
 #include "platform/heap/SafePoint.h"
@@ -1636,64 +1635,6 @@ HeapObjectHeader* NormalPage::FindHeaderFromAddress(Address address) {
   return header;
 }
 
-#if DCHECK_IS_ON()
-static bool IsUninitializedMemory(void* object_pointer, size_t object_size) {
-  // Scan through the object's fields and check that they are all zero.
-  Address* object_fields = reinterpret_cast<Address*>(object_pointer);
-  for (size_t i = 0; i < object_size / sizeof(Address); ++i) {
-    if (object_fields[i])
-      return false;
-  }
-  return true;
-}
-#endif
-
-static void MarkPointer(MarkingVisitor* visitor, HeapObjectHeader* header) {
-  const GCInfo* gc_info = ThreadHeap::GcInfo(header->GcInfoIndex());
-  if (gc_info->HasVTable() && !VTableInitialized(header->Payload())) {
-    // We hit this branch when a GC strikes before GarbageCollected<>'s
-    // constructor runs.
-    //
-    // class A : public GarbageCollected<A> { virtual void f() = 0; };
-    // class B : public A {
-    //   B() : A(foo()) { };
-    // };
-    //
-    // If foo() allocates something and triggers a GC, the vtable of A
-    // has not yet been initialized. In this case, we should mark the A
-    // object without tracing any member of the A object.
-    visitor->MarkHeaderNoTracing(header);
-#if DCHECK_IS_ON()
-    DCHECK(IsUninitializedMemory(header->Payload(), header->PayloadSize()));
-#endif
-  } else {
-    visitor->MarkHeader(header, gc_info->trace_);
-  }
-}
-
-void NormalPage::CheckAndMarkPointer(MarkingVisitor* visitor, Address address) {
-#if DCHECK_IS_ON()
-  DCHECK(Contains(address));
-#endif
-  HeapObjectHeader* header = FindHeaderFromAddress(address);
-  if (!header)
-    return;
-  MarkPointer(visitor, header);
-}
-
-#if DCHECK_IS_ON()
-void NormalPage::CheckAndMarkPointer(MarkingVisitor* visitor,
-                                     Address address,
-                                     MarkedPointerCallbackForTesting callback) {
-  DCHECK(Contains(address));
-  HeapObjectHeader* header = FindHeaderFromAddress(address);
-  if (!header)
-    return;
-  if (!callback(header))
-    MarkPointer(visitor, header);
-}
-#endif
-
 void NormalPage::TakeSnapshot(base::trace_event::MemoryAllocatorDump* page_dump,
                               ThreadState::GCSnapshotInfo& info,
                               HeapSnapshotInfo& heap_info) {
@@ -1790,29 +1731,6 @@ void LargeObjectPage::PoisonUnmarkedObjects() {
 }
 #endif
 
-void LargeObjectPage::CheckAndMarkPointer(MarkingVisitor* visitor,
-                                          Address address) {
-#if DCHECK_IS_ON()
-  DCHECK(Contains(address));
-#endif
-  if (!ContainedInObjectPayload(address))
-    return;
-  MarkPointer(visitor, GetHeapObjectHeader());
-}
-
-#if DCHECK_IS_ON()
-void LargeObjectPage::CheckAndMarkPointer(
-    MarkingVisitor* visitor,
-    Address address,
-    MarkedPointerCallbackForTesting callback) {
-  DCHECK(Contains(address));
-  if (!ContainedInObjectPayload(address))
-    return;
-  if (!callback(GetHeapObjectHeader()))
-    MarkPointer(visitor, GetHeapObjectHeader());
-}
-#endif
-
 void LargeObjectPage::TakeSnapshot(
     base::trace_event::MemoryAllocatorDump* page_dump,
     ThreadState::GCSnapshotInfo& info,

third_party/WebKit/Source/platform/heap/HeapPage.h

@@ -333,11 +333,6 @@ inline bool IsPageHeaderAddress(Address address) {
            kBlinkGuardPageSize);
 }
 
-// Callback used for unit testing the marking of conservative pointers
-// (|CheckAndMarkPointer|). For each pointer that has been discovered to point
-// to a heap object, the callback is invoked with a pointer to its header. If
-// the callback returns true, the object will not be marked.
-using MarkedPointerCallbackForTesting = bool (*)(HeapObjectHeader*);
 #endif
 
 // |BasePage| is a base class for |NormalPage| and |LargeObjectPage|.
@@ -382,20 +377,6 @@ class BasePage {
   virtual void PoisonUnmarkedObjects() = 0;
 #endif
 
-  // Check if the given address points to an object in this heap page. If so,
-  // find the start of that object and mark it using the given |Visitor|.
-  // Otherwise do nothing. The pointer must be within the same aligned
-  // |kBlinkPageSize| as |this|.
-  //
-  // This is used during conservative stack scanning to conservatively mark all
-  // objects that could be referenced from the stack.
-  virtual void CheckAndMarkPointer(MarkingVisitor*, Address) = 0;
-#if DCHECK_IS_ON()
-  virtual void CheckAndMarkPointer(MarkingVisitor*,
-                                   Address,
-                                   MarkedPointerCallbackForTesting) = 0;
-#endif
-
   class HeapSnapshotInfo {
     STACK_ALLOCATED();
 
@@ -535,12 +516,6 @@ class PLATFORM_EXPORT NormalPage final : public BasePage {
 #if defined(ADDRESS_SANITIZER)
   void PoisonUnmarkedObjects() override;
 #endif
-  void CheckAndMarkPointer(MarkingVisitor*, Address) override;
-#if DCHECK_IS_ON()
-  void CheckAndMarkPointer(MarkingVisitor*,
-                           Address,
-                           MarkedPointerCallbackForTesting) override;
-#endif
 
   void TakeSnapshot(base::trace_event::MemoryAllocatorDump*,
                     ThreadState::GCSnapshotInfo&,
@@ -633,12 +608,6 @@ class LargeObjectPage final : public BasePage {
 #if defined(ADDRESS_SANITIZER)
   void PoisonUnmarkedObjects() override;
 #endif
-  void CheckAndMarkPointer(MarkingVisitor*, Address) override;
-#if DCHECK_IS_ON()
-  void CheckAndMarkPointer(MarkingVisitor*,
-                           Address,
-                           MarkedPointerCallbackForTesting) override;
-#endif
 
   void TakeSnapshot(base::trace_event::MemoryAllocatorDump*,
                     ThreadState::GCSnapshotInfo&,

third_party/WebKit/Source/platform/heap/MarkingVisitor.cpp

@@ -21,11 +21,81 @@ MarkingVisitor::MarkingVisitor(ThreadState* state, MarkingMode marking_mode)
   DCHECK(state->IsGCForbidden());
 #if DCHECK_IS_ON()
   DCHECK(state->CheckThread());
-#endif
+#endif  // DCHECK_IS_ON
 }
 
 MarkingVisitor::~MarkingVisitor() = default;
 
+void MarkingVisitor::ConservativelyMarkAddress(BasePage* page,
+                                               Address address) {
+#if DCHECK_IS_ON()
+  DCHECK(page->Contains(address));
+#endif
+  HeapObjectHeader* const header =
+      page->IsLargeObjectPage()
+          ? static_cast<LargeObjectPage*>(page)->GetHeapObjectHeader()
+          : static_cast<NormalPage*>(page)->FindHeaderFromAddress(address);
+  if (!header)
+    return;
+  ConservativelyMarkHeader(header);
+}
+
+#if DCHECK_IS_ON()
+void MarkingVisitor::ConservativelyMarkAddress(
+    BasePage* page,
+    Address address,
+    MarkedPointerCallbackForTesting callback) {
+  DCHECK(page->Contains(address));
+  HeapObjectHeader* const header =
+      page->IsLargeObjectPage()
+          ? static_cast<LargeObjectPage*>(page)->GetHeapObjectHeader()
+          : static_cast<NormalPage*>(page)->FindHeaderFromAddress(address);
+  if (!header)
+    return;
+  if (!callback(header))
+    ConservativelyMarkHeader(header);
+}
+#endif  // DCHECK_IS_ON
+
+namespace {
+
+#if DCHECK_IS_ON()
+bool IsUninitializedMemory(void* object_pointer, size_t object_size) {
+  // Scan through the object's fields and check that they are all zero.
+  Address* object_fields = reinterpret_cast<Address*>(object_pointer);
+  for (size_t i = 0; i < object_size / sizeof(Address); ++i) {
+    if (object_fields[i])
+      return false;
+  }
+  return true;
+}
+#endif
+
+}  // namespace
+
+void MarkingVisitor::ConservativelyMarkHeader(HeapObjectHeader* header) {
+  const GCInfo* gc_info = ThreadHeap::GcInfo(header->GcInfoIndex());
+  if (gc_info->HasVTable() && !VTableInitialized(header->Payload())) {
+    // We hit this branch when a GC strikes before GarbageCollected<>'s
+    // constructor runs.
+    //
+    // class A : public GarbageCollected<A> { virtual void f() = 0; };
+    // class B : public A {
+    //   B() : A(foo()) { };
+    // };
+    //
+    // If foo() allocates something and triggers a GC, the vtable of A
+    // has not yet been initialized. In this case, we should mark the A
+    // object without tracing any member of the A object.
+    MarkHeaderNoTracing(header);
+#if DCHECK_IS_ON()
+    DCHECK(IsUninitializedMemory(header->Payload(), header->PayloadSize()));
+#endif
+  } else {
+    MarkHeader(header, gc_info->trace_);
+  }
+}
+
 void MarkingVisitor::MarkNoTracingCallback(Visitor* visitor, void* object) {
   reinterpret_cast<MarkingVisitor*>(visitor)->MarkHeaderNoTracing(
       HeapObjectHeader::FromPayload(object));

third_party/WebKit/Source/platform/heap/MarkingVisitor.h

@@ -11,6 +11,8 @@
 
 namespace blink {
 
+class BasePage;
+
 // Visitor used to mark Oilpan objects.
 class PLATFORM_EXPORT MarkingVisitor final : public Visitor {
  public:
@@ -37,6 +39,14 @@ class PLATFORM_EXPORT MarkingVisitor final : public Visitor {
 
   // Marking implementation.
 
+  // Conservatively marks an object if pointed to by Address.
+  void ConservativelyMarkAddress(BasePage*, Address);
+#if DCHECK_IS_ON()
+  void ConservativelyMarkAddress(BasePage*,
+                                 Address,
+                                 MarkedPointerCallbackForTesting);
+#endif  // DCHECK_IS_ON()
+
   // Marks an object and adds a tracing callback for processing of the object.
   inline void MarkHeader(HeapObjectHeader*, TraceCallback);
 
@@ -125,6 +135,8 @@ class PLATFORM_EXPORT MarkingVisitor final : public Visitor {
 
   void RegisterBackingStoreReference(void* slot);
 
+  void ConservativelyMarkHeader(HeapObjectHeader*);
+
   const MarkingMode marking_mode_;
 };