[wrapper-tracing] Move visitor parts to blink::Visitor

Move wrapper tracing visitation to regular blink::Visitor and inherit
from that in blink::ScriptWrappableVisitor. This is prework for removing
TraceWrappers and solely rely on Trace methods, even for wrapper tracing.

The bigger idea is to bring Trace in sync with TraceWrappers and drop
TraceWrappers completely. The visitors should distinguish which part
of the graph the would like to trace.

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

third_party/blink/renderer/bindings/core/v8/v8_embedder_graph_builder.cc

@@ -110,7 +110,7 @@ void V8EmbedderGraphBuilder::VisitPersistentHandle(
   EmbedderNode* graph_node = GraphNode(
       traceable, traceable->NameInHeapSnapshot(), wrapper, dom_tree_state);
   const TraceWrapperDescriptor& wrapper_descriptor =
-      WrapperDescriptorFor<ScriptWrappable>(traceable);
+      TraceWrapperDescriptorFor<ScriptWrappable>(traceable);
   WorklistItem item = ToWorklistItem(graph_node, wrapper_descriptor);
   switch (graph_node->GetDomTreeState()) {
     case DomTreeState::kAttached:
@@ -135,8 +135,8 @@ void V8EmbedderGraphBuilder::Visit(
   }
 }
 
-void V8EmbedderGraphBuilder::Visit(
-    const TraceWrapperDescriptor& wrapper_descriptor) {
+void V8EmbedderGraphBuilder::Visit(void* object,
+                                   TraceWrapperDescriptor wrapper_descriptor) {
   // Add an edge from the current parent to this object.
   // Also push the object to the worklist in order to process its members.
   const void* traceable = wrapper_descriptor.base_object_payload;

third_party/blink/renderer/bindings/core/v8/v8_embedder_graph_builder.h

@@ -26,12 +26,14 @@ class V8EmbedderGraphBuilder : public ScriptWrappableVisitor,
   void VisitPersistentHandle(v8::Persistent<v8::Value>*,
                              uint16_t class_id) override;
 
- protected:
-  // ScriptWrappableVisitor overrides.
+  // Visitor overrides.
   void Visit(const TraceWrapperV8Reference<v8::Value>&) final;
-  void Visit(const TraceWrapperDescriptor&) final;
+  void Visit(void*, TraceWrapperDescriptor) final;
   void Visit(DOMWrapperMap<ScriptWrappable>*, const ScriptWrappable*) final;
 
+ protected:
+  using Visitor::Visit;
+
  private:
   // Information about whether a node is attached to the main DOM tree
   // or not. It is computed as follows:

third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.cc

@@ -253,7 +253,8 @@ void ScriptWrappableMarkingVisitor::Visit(
 }
 
 void ScriptWrappableMarkingVisitor::Visit(
-    const TraceWrapperDescriptor& wrapper_descriptor) {
+    void* object,
+    TraceWrapperDescriptor wrapper_descriptor) {
   HeapObjectHeader* header =
       HeapObjectHeader::FromPayload(wrapper_descriptor.base_object_payload);
   if (header->IsWrapperHeaderMarked())

third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.h

@@ -72,7 +72,8 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
       return;
 
     CurrentVisitor(thread_state->GetIsolate())
-        ->Visit(WrapperDescriptorFor(dst_object));
+        ->Visit(const_cast<T*>(dst_object),
+                TraceWrapperDescriptorFor(dst_object));
   }
 
   static void WriteBarrier(v8::Isolate*,
@@ -98,13 +99,15 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   void EnterFinalPause() override;
   size_t NumberOfWrappersToTrace() override;
 
- protected:
-  // ScriptWrappableVisitor interface.
+  // Visitor interface.
   void Visit(const TraceWrapperV8Reference<v8::Value>&) override;
-  void Visit(const TraceWrapperDescriptor&) override;
+  void Visit(void*, TraceWrapperDescriptor) override;
   void Visit(DOMWrapperMap<ScriptWrappable>*,
              const ScriptWrappable* key) override;
 
+ protected:
+  using Visitor::Visit;
+
   v8::Isolate* isolate() const { return isolate_; }
 
  private:

third_party/blink/renderer/platform/bindings/script_wrappable_visitor.h

@@ -8,6 +8,7 @@
 #include "third_party/blink/renderer/platform/bindings/trace_wrapper_base.h"
 #include "third_party/blink/renderer/platform/heap/heap_page.h"
 #include "third_party/blink/renderer/platform/heap/threading_traits.h"
+#include "third_party/blink/renderer/platform/heap/visitor.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/deque.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
@@ -43,13 +44,15 @@ class TraceWrapperV8Reference;
 // - Call visitor.DispatchTraceWrappers(traceable).
 // DispatchTraceWrappers will invoke Visit() method for all
 // wrapper references in traceable.
-class PLATFORM_EXPORT ScriptWrappableVisitor {
+class PLATFORM_EXPORT ScriptWrappableVisitor : public Visitor {
  public:
   template <typename T>
   static NOINLINE void MissedWriteBarrier() {
     NOTREACHED();
   }
 
+  ScriptWrappableVisitor() : Visitor(ThreadState::Current()) {}
+
   // Trace all wrappers of |tracable|.
   //
   // If you cannot use TraceWrapperMember & the corresponding TraceWrappers()
@@ -103,18 +106,25 @@ class PLATFORM_EXPORT ScriptWrappableVisitor {
   // Catch all handlers needed because of mixins except for Supplement<T>.
   void DispatchTraceWrappers(const void*) const { CHECK(false); }
 
- protected:
-  // The visitor interface. Derived visitors should override this
-  // function to visit V8 references and ScriptWrappables.
-  virtual void Visit(const TraceWrapperV8Reference<v8::Value>&) = 0;
-  virtual void Visit(const TraceWrapperDescriptor&) = 0;
-  virtual void Visit(DOMWrapperMap<ScriptWrappable>*,
-                     const ScriptWrappable* key) = 0;
+  // Unused blink::Visitor overrides. Derived visitors should still override
+  // the cross-component visitation methods. See Visitor documentation.
+  void Visit(void* object, TraceDescriptor desc) final {}
+  void VisitWeak(void* object,
+                 void** object_slot,
+                 TraceDescriptor desc,
+                 WeakCallback callback) final {}
+  void VisitBackingStoreStrongly(void*, void**, TraceDescriptor) final {}
+  void VisitBackingStoreWeakly(void*,
+                               void**,
+                               TraceDescriptor,
+                               WeakCallback,
+                               void*) final {}
+  void VisitBackingStoreOnly(void*, void**) final {}
+  void RegisterBackingStoreCallback(void*, MovingObjectCallback, void*) final {}
+  void RegisterWeakCallback(void*, WeakCallback) final {}
 
-  template <typename T>
-  static TraceWrapperDescriptor WrapperDescriptorFor(const T* traceable) {
-    return TraceTrait<T>::GetTraceWrapperDescriptor(const_cast<T*>(traceable));
-  }
+ protected:
+  using Visitor::Visit;
 
  private:
   // Helper method to invoke the virtual Visit method with wrapper descriptor.
@@ -123,7 +133,7 @@ class PLATFORM_EXPORT ScriptWrappableVisitor {
     static_assert(sizeof(T), "T must be fully defined");
     if (!traceable)
       return;
-    Visit(WrapperDescriptorFor(traceable));
+    Visit(const_cast<T*>(traceable), TraceWrapperDescriptorFor(traceable));
   }
 
   // Supplement-specific implementation of DispatchTraceWrappers.  The suffix of

third_party/blink/renderer/platform/bindings/script_wrappable_visitor_verifier.h

@@ -15,9 +15,10 @@ namespace blink {
 // after marking is complete. The Visit method checks that the given wrapper
 // is also marked.
 class ScriptWrappableVisitorVerifier final : public ScriptWrappableVisitor {
- protected:
+ public:
   void Visit(const TraceWrapperV8Reference<v8::Value>&) final {}
-  void Visit(const TraceWrapperDescriptor& descriptor) final {
+
+  void Visit(void* object, TraceWrapperDescriptor descriptor) final {
     HeapObjectHeader* header =
         HeapObjectHeader::FromPayload(descriptor.base_object_payload);
     const char* name = GCInfoTable::Get()
@@ -35,8 +36,14 @@ class ScriptWrappableVisitorVerifier final : public ScriptWrappableVisitor {
     LOG_IF(FATAL, !header->IsWrapperHeaderMarked())
         << "Write barrier missed for " << name;
   }
+
   void Visit(DOMWrapperMap<ScriptWrappable>*,
              const ScriptWrappable* key) final {}
+
+ protected:
+  using Visitor::Visit;
 };
-}
-#endif
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_BINDINGS_SCRIPT_WRAPPABLE_VISITOR_VERIFIER_H_

third_party/blink/renderer/platform/heap/incremental_marking_test.cc

@@ -69,6 +69,10 @@ class BackingVisitor : public Visitor {
                                     MovingObjectCallback,
                                     void* callback_data) final {}
   void RegisterWeakCallback(void* closure, WeakCallback) final {}
+  void Visit(const TraceWrapperV8Reference<v8::Value>&) final {}
+  void Visit(DOMWrapperMap<ScriptWrappable>*,
+             const ScriptWrappable* key) final {}
+  void Visit(void*, TraceWrapperDescriptor) final {}
 
  private:
   std::vector<void*>* objects_;

third_party/blink/renderer/platform/heap/marking_verifier.h

@@ -55,6 +55,10 @@ class MarkingVerifier final : public Visitor {
   void VisitBackingStoreOnly(void*, void**) final {}
   void RegisterBackingStoreCallback(void*, MovingObjectCallback, void*) final {}
   void RegisterWeakCallback(void*, WeakCallback) final {}
+  void Visit(const TraceWrapperV8Reference<v8::Value>&) final {}
+  void Visit(DOMWrapperMap<ScriptWrappable>*,
+             const ScriptWrappable* key) final {}
+  void Visit(void*, TraceWrapperDescriptor) final {}
 
  private:
   void VerifyChild(void* base_object_payload) {

third_party/blink/renderer/platform/heap/marking_visitor.h

@@ -109,6 +109,10 @@ class PLATFORM_EXPORT MarkingVisitor final : public Visitor {
                desc.callback);
   }
 
+  void Visit(void*, TraceWrapperDescriptor) final {
+    // Ignore as the object is also passed to Visit(void*, TraceDescriptor).
+  }
+
   void VisitWeak(void* object,
                  void** object_slot,
                  TraceDescriptor desc,
@@ -147,6 +151,11 @@ class PLATFORM_EXPORT MarkingVisitor final : public Visitor {
                          EphemeronCallback iteration_callback) final;
   void RegisterWeakCallback(void* closure, WeakCallback) final;
 
+  // Unused cross-component visit methods.
+  void Visit(const TraceWrapperV8Reference<v8::Value>&) final {}
+  void Visit(DOMWrapperMap<ScriptWrappable>*,
+             const ScriptWrappable* key) final {}
+
  private:
   void RegisterBackingStoreReference(void* slot);
 

third_party/blink/renderer/platform/heap/visitor.h

@@ -39,19 +39,25 @@
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 #include "third_party/blink/renderer/platform/wtf/hash_traits.h"
 #include "third_party/blink/renderer/platform/wtf/type_traits.h"
+#include "v8/include/v8.h"
 
 namespace blink {
 
 template <typename T>
 class GarbageCollected;
 template <typename T>
+class DOMWrapperMap;
+template <typename T>
 class TraceTrait;
 class ThreadState;
 class Visitor;
 template <typename T>
 class SameThreadCheckedMember;
+class ScriptWrappable;
 template <typename T>
 class TraceWrapperMember;
+template <typename T>
+class TraceWrapperV8Reference;
 
 // The TraceMethodDelegate is used to convert a trace method for type T to a
 // TraceCallback.  This allows us to pass a type's trace method as a parameter
@@ -85,11 +91,6 @@ class PLATFORM_EXPORT Visitor {
     Trace(t.Get());
   }
 
-  template <typename T>
-  void Trace(const TraceWrapperMember<T>& t) {
-    Trace(*(static_cast<const Member<T>*>(&t)));
-  }
-
   template <typename T>
   void Trace(const SameThreadCheckedMember<T>& t) {
     Trace(*(static_cast<const Member<T>*>(&t)));
@@ -110,8 +111,7 @@ class PLATFORM_EXPORT Visitor {
     if (!t)
       return;
     Visit(const_cast<void*>(reinterpret_cast<const void*>(t)),
-          TraceTrait<T>::GetTraceDescriptor(
-              const_cast<void*>(reinterpret_cast<const void*>(t))));
+          TraceDescriptorFor(t));
   }
 
   template <typename T>
@@ -124,8 +124,7 @@ class PLATFORM_EXPORT Visitor {
       return;
     VisitBackingStoreStrongly(reinterpret_cast<void*>(backing_store),
                               reinterpret_cast<void**>(backing_store_slot),
-                              TraceTrait<T>::GetTraceDescriptor(
-                                  reinterpret_cast<void*>(backing_store)));
+                              TraceDescriptorFor(backing_store));
   }
 
   template <typename T>
@@ -208,10 +207,42 @@ class PLATFORM_EXPORT Visitor {
                          &TraceMethodDelegate<T, method>::Trampoline);
   }
 
+  // Cross-component tracing interface.
+  // TODO(mlippautz): Add required Trace methods to delegate to the subgraph
+  // relevant for wrapper tracing.
+
+  template <typename T>
+  void Trace(const TraceWrapperMember<T>& t) {
+    DCHECK(!t.IsHashTableDeletedValue());
+    TraceWithWrappers(t.Get());
+  }
+
+  template <typename T>
+  void TraceWithWrappers(T* t) {
+    static_assert(sizeof(T), "T must be fully defined");
+    static_assert(IsGarbageCollectedType<T>::value,
+                  "T needs to be a garbage collected object");
+    if (!t)
+      return;
+
+    // Dispatch two both, the TraceDescritpor and the TraceWrapperDescriptor,
+    // versions of the visitor. This way the wrapper-tracing world can ignore
+    // the TraceDescriptor versions.
+    Visit(const_cast<void*>(reinterpret_cast<const void*>(t)),
+          TraceDescriptorFor(t));
+    Visit(const_cast<void*>(reinterpret_cast<const void*>(t)),
+          TraceWrapperDescriptorFor(t));
+  }
+
   // Dynamic visitor interface.
 
   // Visits an object through a strong reference.
   virtual void Visit(void*, TraceDescriptor) = 0;
+  // Subgraph of objects that are interested in wrappers. Note that the same
+  // object is also passed to Visit(void*, TraceDescriptor).
+  // TODO(mlippautz): Remove this visit method once wrapper tracing also uses
+  // Trace() instead of TraceWrappers().
+  virtual void Visit(void*, TraceWrapperDescriptor) = 0;
 
   // Visits an object through a weak reference.
   virtual void VisitWeak(void*, void**, TraceDescriptor, WeakCallback) = 0;
@@ -225,6 +256,12 @@ class PLATFORM_EXPORT Visitor {
                                        void*) = 0;
   virtual void VisitBackingStoreOnly(void*, void**) = 0;
 
+  // Visits cross-component references to V8.
+
+  virtual void Visit(const TraceWrapperV8Reference<v8::Value>&) = 0;
+  virtual void Visit(DOMWrapperMap<ScriptWrappable>*,
+                     const ScriptWrappable* key) = 0;
+
   // Registers backing store pointers so that they can be moved and properly
   // updated.
   virtual void RegisterBackingStoreCallback(void* backing_store,
@@ -247,6 +284,18 @@ class PLATFORM_EXPORT Visitor {
   // WeakMember elements though.
   virtual void RegisterWeakCallback(void* closure, WeakCallback) = 0;
 
+ protected:
+  template <typename T>
+  static inline TraceDescriptor TraceDescriptorFor(const T* traceable) {
+    return TraceTrait<T>::GetTraceDescriptor(const_cast<T*>(traceable));
+  }
+
+  template <typename T>
+  static inline TraceWrapperDescriptor TraceWrapperDescriptorFor(
+      const T* traceable) {
+    return TraceTrait<T>::GetTraceWrapperDescriptor(const_cast<T*>(traceable));
+  }
+
  private:
   template <typename T>
   static void HandleWeakCell(Visitor* self, void*);