Simplify visitor marking mode tracking.

Move the marking mode to VisitorHelper<>, so that
both Visitor and InlinedGlobalMarkingVisitor can access
and reuse it.

In order to do so, hoist out Visitor::MarkingMode as an
enum class.

R=
BUG=

Review-Url: https://codereview.chromium.org/2625363002
Cr-Commit-Position: refs/heads/master@{#443444}

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

@@ -318,7 +318,7 @@ class TestGCScope {
 class CountingVisitor : public Visitor {
  public:
   explicit CountingVisitor(ThreadState* state)
-      : Visitor(state, Visitor::ThreadLocalMarking),
+      : Visitor(state, VisitorMarkingMode::ThreadLocalMarking),
         m_scope(&state->heap().stackFrameDepth()),
         m_count(0) {}
 

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

@@ -19,8 +19,8 @@ class InlinedGlobalMarkingVisitor final
   using Impl = MarkingVisitorImpl<InlinedGlobalMarkingVisitor>;
 
   InlinedGlobalMarkingVisitor(ThreadState* state,
-                              Visitor::MarkingMode markingMode)
-      : VisitorHelper(state), m_markingMode(markingMode) {}
+                              VisitorMarkingMode markingMode)
+      : VisitorHelper(state, markingMode) {}
 
   // Hack to unify interface to visitor->trace().
   // Without this hack, we need to use visitor.trace() for
@@ -47,18 +47,10 @@ class InlinedGlobalMarkingVisitor final
     Helper::template registerWeakMembers<T, method>(obj);
   }
 
- protected:
-  // Methods to be called from MarkingVisitorImpl.
-
-  inline Visitor::MarkingMode getMarkingMode() const { return m_markingMode; }
-
  private:
   static InlinedGlobalMarkingVisitor fromHelper(Helper* helper) {
     return *static_cast<InlinedGlobalMarkingVisitor*>(helper);
   }
-
-  // TODO(sof): attempt to unify this field with Visitor::m_markingMode.
-  const Visitor::MarkingMode m_markingMode;
 };
 
 inline void GarbageCollectedMixin::trace(InlinedGlobalMarkingVisitor) {}

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

@@ -14,7 +14,7 @@ class MarkingVisitor final : public Visitor,
  public:
   using Impl = MarkingVisitorImpl<MarkingVisitor>;
 
-  MarkingVisitor(ThreadState* state, Visitor::MarkingMode mode)
+  MarkingVisitor(ThreadState* state, VisitorMarkingMode mode)
       : Visitor(state, mode) {}
 
   void markHeader(HeapObjectHeader* header, TraceCallback callback) override {

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

@@ -35,7 +35,7 @@ class MarkingVisitorImpl {
       return;
 
     ASSERT(ThreadState::current()->isInGC());
-    ASSERT(toDerived()->getMarkingMode() != Visitor::WeakProcessing);
+    DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing);
 
     // A GC should only mark the objects that belong in its heap.
     DCHECK(&pageFromObject(objectPointer)->arena()->getThreadState()->heap() ==
@@ -56,7 +56,7 @@ class MarkingVisitorImpl {
   }
 
   inline void registerDelayedMarkNoTracing(const void* objectPointer) {
-    ASSERT(toDerived()->getMarkingMode() != Visitor::WeakProcessing);
+    DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing);
     toDerived()->heap().pushPostMarkingCallback(
         const_cast<void*>(objectPointer), &markNoTracingCallback);
   }
@@ -64,9 +64,9 @@ class MarkingVisitorImpl {
   inline void registerWeakMembers(const void* closure,
                                   const void* objectPointer,
                                   WeakCallback callback) {
-    ASSERT(toDerived()->getMarkingMode() != Visitor::WeakProcessing);
+    DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing);
     // We don't want to run weak processings when taking a snapshot.
-    if (toDerived()->getMarkingMode() == Visitor::SnapshotMarking)
+    if (toDerived()->getMarkingMode() == VisitorMarkingMode::SnapshotMarking)
       return;
     toDerived()->heap().pushThreadLocalWeakCallback(
         const_cast<void*>(closure), const_cast<void*>(objectPointer), callback);
@@ -75,7 +75,7 @@ class MarkingVisitorImpl {
   inline void registerWeakTable(const void* closure,
                                 EphemeronCallback iterationCallback,
                                 EphemeronCallback iterationDoneCallback) {
-    ASSERT(toDerived()->getMarkingMode() != Visitor::WeakProcessing);
+    DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing);
     toDerived()->heap().registerWeakTable(
         const_cast<void*>(closure), iterationCallback, iterationDoneCallback);
   }
@@ -87,7 +87,8 @@ class MarkingVisitorImpl {
 #endif
 
   inline void registerMovingObjectReference(MovableReference* slot) {
-    if (toDerived()->getMarkingMode() != Visitor::GlobalMarkingWithCompaction)
+    if (toDerived()->getMarkingMode() !=
+        VisitorMarkingMode::GlobalMarkingWithCompaction)
       return;
     toDerived()->heap().registerMovingObjectReference(slot);
   }
@@ -95,7 +96,8 @@ class MarkingVisitorImpl {
   inline void registerMovingObjectCallback(MovableReference reference,
                                            MovingObjectCallback callback,
                                            void* callbackData) {
-    if (toDerived()->getMarkingMode() != Visitor::GlobalMarkingWithCompaction)
+    if (toDerived()->getMarkingMode() !=
+        VisitorMarkingMode::GlobalMarkingWithCompaction)
       return;
     toDerived()->heap().registerMovingObjectCallback(reference, callback,
                                                      callbackData);
@@ -119,9 +121,9 @@ class MarkingVisitorImpl {
   }
 
   inline void registerWeakCellWithCallback(void** cell, WeakCallback callback) {
-    ASSERT(toDerived()->getMarkingMode() != Visitor::WeakProcessing);
+    DCHECK(toDerived()->getMarkingMode() != VisitorMarkingMode::WeakProcessing);
     // We don't want to run weak processings when taking a snapshot.
-    if (toDerived()->getMarkingMode() == Visitor::SnapshotMarking)
+    if (toDerived()->getMarkingMode() == VisitorMarkingMode::SnapshotMarking)
       return;
     toDerived()->heap().pushGlobalWeakCallback(cell, callback);
   }

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

@@ -479,7 +479,7 @@ void ThreadState::threadLocalWeakProcessing() {
 
   GCForbiddenScope gcForbiddenScope(this);
   std::unique_ptr<Visitor> visitor =
-      Visitor::create(this, Visitor::WeakProcessing);
+      Visitor::create(this, VisitorMarkingMode::WeakProcessing);
 
   // Perform thread-specific weak processing.
   while (popAndInvokeThreadLocalWeakCallback(visitor.get())) {
@@ -1697,14 +1697,15 @@ void ThreadState::collectGarbage(BlinkGC::StackState stackState,
 
   std::unique_ptr<Visitor> visitor;
   if (gcType == BlinkGC::TakeSnapshot) {
-    visitor = Visitor::create(this, Visitor::SnapshotMarking);
+    visitor = Visitor::create(this, VisitorMarkingMode::SnapshotMarking);
   } else {
     DCHECK(gcType == BlinkGC::GCWithSweep || gcType == BlinkGC::GCWithoutSweep);
     if (heap().compaction()->shouldCompact(this, gcType, reason)) {
       heap().compaction()->initialize(this);
-      visitor = Visitor::create(this, Visitor::GlobalMarkingWithCompaction);
+      visitor = Visitor::create(
+          this, VisitorMarkingMode::GlobalMarkingWithCompaction);
     } else {
-      visitor = Visitor::create(this, Visitor::GlobalMarking);
+      visitor = Visitor::create(this, VisitorMarkingMode::GlobalMarking);
     }
   }
 
@@ -1810,7 +1811,7 @@ void ThreadState::collectGarbageForTerminatingThread() {
     // ThreadTerminationGC.
     GCForbiddenScope gcForbiddenScope(this);
     std::unique_ptr<Visitor> visitor =
-        Visitor::create(this, Visitor::ThreadLocalMarking);
+        Visitor::create(this, VisitorMarkingMode::ThreadLocalMarking);
 
     NoAllocationScope noAllocationScope(this);
 

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

@@ -12,12 +12,13 @@
 
 namespace blink {
 
-std::unique_ptr<Visitor> Visitor::create(ThreadState* state, MarkingMode mode) {
+std::unique_ptr<Visitor> Visitor::create(ThreadState* state,
+                                         VisitorMarkingMode mode) {
   return WTF::makeUnique<MarkingVisitor>(state, mode);
 }
 
-Visitor::Visitor(ThreadState* state, MarkingMode markingMode)
-    : VisitorHelper(state), m_markingMode(markingMode) {
+Visitor::Visitor(ThreadState* state, VisitorMarkingMode markingMode)
+    : VisitorHelper(state, markingMode) {
   // See ThreadState::runScheduledGC() why we need to already be in a
   // GCForbiddenScope before any safe point is entered.
   DCHECK(state->isGCForbidden());

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

@@ -133,6 +133,28 @@ struct TraceMethodDelegate {
 #define DEFINE_INLINE_TRACE() DEFINE_INLINE_TRACE_IMPL(EMPTY_MACRO_ARGUMENT)
 #define DEFINE_INLINE_VIRTUAL_TRACE() DEFINE_INLINE_TRACE_IMPL(virtual)
 
+enum class VisitorMarkingMode {
+  // This is a default visitor. This is used for GCType=GCWithSweep
+  // and GCType=GCWithoutSweep.
+  GlobalMarking,
+  // This visitor does not trace objects outside the heap of the
+  // GCing thread. This is used for GCType=ThreadTerminationGC.
+  ThreadLocalMarking,
+  // This visitor just marks objects and ignores weak processing.
+  // This is used for GCType=TakeSnapshot.
+  SnapshotMarking,
+  // This visitor is used to trace objects during weak processing.
+  // This visitor is allowed to trace only already marked objects.
+  WeakProcessing,
+  // Perform global marking along with preparing for additional sweep
+  // compaction of heap arenas afterwards. Compared to the GlobalMarking
+  // visitor, this visitor will also register references to objects
+  // that might be moved during arena compaction -- the compaction
+  // pass will then fix up those references when the object move goes
+  // ahead.
+  GlobalMarkingWithCompaction,
+};
+
 // VisitorHelper contains common implementation of Visitor helper methods.
 //
 // VisitorHelper avoids virtual methods by using CRTP.
@@ -140,7 +162,8 @@ struct TraceMethodDelegate {
 template <typename Derived>
 class VisitorHelper {
  public:
-  VisitorHelper(ThreadState* state) : m_state(state) {}
+  VisitorHelper(ThreadState* state, VisitorMarkingMode markingMode)
+      : m_state(state), m_markingMode(markingMode) {}
 
   // One-argument templated mark method. This uses the static type of
   // the argument to get the TraceTrait. By default, the mark method
@@ -275,11 +298,19 @@ class VisitorHelper {
   inline ThreadState* state() const { return m_state; }
   inline ThreadHeap& heap() const { return state()->heap(); }
 
+  inline VisitorMarkingMode getMarkingMode() const { return m_markingMode; }
+
+  inline bool isGlobalMarking() const {
+    return m_markingMode == VisitorMarkingMode::GlobalMarking ||
+           m_markingMode == VisitorMarkingMode::GlobalMarkingWithCompaction;
+  }
+
  private:
   template <typename T>
   static void handleWeakCell(Visitor* self, void* object);
 
   ThreadState* const m_state;
+  const VisitorMarkingMode m_markingMode;
 };
 
 // Visitor is used to traverse the Blink object graph. Used for the
@@ -296,29 +327,7 @@ class PLATFORM_EXPORT Visitor : public VisitorHelper<Visitor> {
   friend class VisitorHelper<Visitor>;
   friend class InlinedGlobalMarkingVisitor;
 
-  enum MarkingMode {
-    // This is a default visitor. This is used for GCType=GCWithSweep
-    // and GCType=GCWithoutSweep.
-    GlobalMarking,
-    // This visitor does not trace objects outside the heap of the
-    // GCing thread. This is used for GCType=ThreadTerminationGC.
-    ThreadLocalMarking,
-    // This visitor just marks objects and ignores weak processing.
-    // This is used for GCType=TakeSnapshot.
-    SnapshotMarking,
-    // This visitor is used to trace objects during weak processing.
-    // This visitor is allowed to trace only already marked objects.
-    WeakProcessing,
-    // Perform global marking along with preparing for additional sweep
-    // compaction of heap arenas afterwards. Compared to the GlobalMarking
-    // visitor, this visitor will also register references to objects
-    // that might be moved during arena compaction -- the compaction
-    // pass will then fix up those references when the object move goes
-    // ahead.
-    GlobalMarkingWithCompaction,
-  };
-
-  static std::unique_ptr<Visitor> create(ThreadState*, MarkingMode);
+  static std::unique_ptr<Visitor> create(ThreadState*, VisitorMarkingMode);
 
   virtual ~Visitor();
 
@@ -384,22 +393,13 @@ class PLATFORM_EXPORT Visitor : public VisitorHelper<Visitor> {
 
   virtual void registerWeakCellWithCallback(void**, WeakCallback) = 0;
 
-  inline MarkingMode getMarkingMode() const { return m_markingMode; }
-
-  inline bool isGlobalMarking() const {
-    return m_markingMode == GlobalMarking ||
-           m_markingMode == GlobalMarkingWithCompaction;
-  }
-
  protected:
-  Visitor(ThreadState*, MarkingMode);
+  Visitor(ThreadState*, VisitorMarkingMode);
 
  private:
   static Visitor* fromHelper(VisitorHelper<Visitor>* helper) {
     return static_cast<Visitor*>(helper);
   }
-
-  const MarkingMode m_markingMode;
 };
 
 }  // namespace blink