[wrapper-tracing] Only return trace descriptor for wrapper tracing

Similar to Oilpan, only return a struct of callbacks and don't actually
call further into visitors.

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

third_party/WebKit/Source/bindings/core/v8/ScriptWrappableMarkingVisitorTest.cpp

@@ -501,7 +501,6 @@ TEST(ScriptWrappableMarkingVisitorTest, MixinTracing) {
       DeathAwareScriptWrappable::Create();
   Base* base = Base::Create(base_wrapper, mixin_wrapper);
   Mixin* mixin = static_cast<Mixin*>(base);
-
   HeapObjectHeader* base_header = HeapObjectHeader::FromPayload(base);
   EXPECT_FALSE(base_header->IsWrapperHeaderMarked());
 
@@ -516,7 +515,7 @@ TEST(ScriptWrappableMarkingVisitorTest, MixinTracing) {
   TraceWrapperMember<Mixin> mixin_handle = mixin;
   EXPECT_TRUE(base_header->IsWrapperHeaderMarked());
   EXPECT_FALSE(visitor->MarkingDeque()->IsEmpty());
-  EXPECT_TRUE(visitor->MarkingDequeContains(mixin));
+  EXPECT_TRUE(visitor->MarkingDequeContains(base));
 
   visitor->AdvanceTracing(
       0, v8::EmbedderHeapTracer::AdvanceTracingActions(

third_party/WebKit/Source/bindings/core/v8/V8EmbedderGraphBuilder.cpp

@@ -64,7 +64,7 @@ void V8EmbedderGraphBuilder::Visit(
     const WrapperDescriptor& wrapper_descriptor) const {
   // 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.traceable;
+  const void* traceable = wrapper_descriptor.base_object_payload;
   Graph::Node* graph_node =
       GraphNode(traceable, wrapper_descriptor.name_callback(traceable));
   graph_->AddEdge(current_parent_, graph_node);
@@ -113,7 +113,7 @@ void V8EmbedderGraphBuilder::VisitPendingActivities() {
 V8EmbedderGraphBuilder::WorklistItem V8EmbedderGraphBuilder::ToWorklistItem(
     Graph::Node* node,
     const WrapperDescriptor& wrapper_descriptor) const {
-  return {node, wrapper_descriptor.traceable,
+  return {node, wrapper_descriptor.base_object_payload,
           wrapper_descriptor.trace_wrappers_callback};
 }
 
@@ -123,7 +123,7 @@ void V8EmbedderGraphBuilder::VisitTransitiveClosure() {
     auto item = worklist_.back();
     worklist_.pop_back();
     ParentScope parent(this, item.node);
-    item.trace_wrappers_callback(this, item.traceable);
+    item.trace_wrappers_callback(this, const_cast<void*>(item.traceable));
   }
 }
 

third_party/WebKit/Source/platform/bindings/ScriptWrappableMarkingVisitor.cpp

@@ -254,8 +254,8 @@ void ScriptWrappableMarkingVisitor::Visit(
 
 void ScriptWrappableMarkingVisitor::Visit(
     const WrapperDescriptor& wrapper_descriptor) const {
-  HeapObjectHeader* header = wrapper_descriptor.heap_object_header_callback(
-      wrapper_descriptor.traceable);
+  HeapObjectHeader* header =
+      HeapObjectHeader::FromPayload(wrapper_descriptor.base_object_payload);
   if (header->IsWrapperHeaderMarked())
     return;
   MarkWrapperHeader(header);

third_party/WebKit/Source/platform/bindings/ScriptWrappableMarkingVisitor.h

@@ -66,7 +66,8 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
       return;
 
     // If the wrapper is already marked we can bail out here.
-    if (TraceTrait<T>::GetHeapObjectHeader(dst_object)->IsWrapperHeaderMarked())
+    if (TraceTrait<T>::GetHeapObjectHeader(const_cast<T*>(dst_object))
+            ->IsWrapperHeaderMarked())
       return;
 
     CurrentVisitor(thread_state->GetIsolate())
@@ -109,19 +110,17 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   class MarkingDequeItem {
    public:
     explicit MarkingDequeItem(const WrapperDescriptor& wrapper_descriptor)
-        : trace_wrappers_callback_(wrapper_descriptor.trace_wrappers_callback),
-          heap_object_header_callback_(
-              wrapper_descriptor.heap_object_header_callback),
-          raw_object_pointer_(wrapper_descriptor.traceable) {
-      DCHECK(trace_wrappers_callback_);
-      DCHECK(heap_object_header_callback_);
+        : raw_object_pointer_(wrapper_descriptor.base_object_payload),
+          trace_wrappers_callback_(wrapper_descriptor.trace_wrappers_callback) {
       DCHECK(raw_object_pointer_);
+      DCHECK(trace_wrappers_callback_);
     }
 
     // Traces wrappers if the underlying object has not yet been invalidated.
     inline void TraceWrappers(ScriptWrappableVisitor* visitor) const {
       if (raw_object_pointer_) {
-        trace_wrappers_callback_(visitor, raw_object_pointer_);
+        trace_wrappers_callback_(visitor,
+                                 const_cast<void*>(raw_object_pointer_));
       }
     }
 
@@ -139,13 +138,11 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
 
    private:
     inline const HeapObjectHeader* GetHeapObjectHeader() {
-      DCHECK(raw_object_pointer_);
-      return heap_object_header_callback_(raw_object_pointer_);
+      return HeapObjectHeader::FromPayload(raw_object_pointer_);
     }
 
-    TraceWrappersCallback trace_wrappers_callback_;
-    HeapObjectHeaderCallback heap_object_header_callback_;
     const void* raw_object_pointer_;
+    TraceWrappersCallback trace_wrappers_callback_;
   };
 
   void MarkWrapperHeader(HeapObjectHeader*) const;

third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitor.h

@@ -17,7 +17,6 @@ namespace blink {
 
 template <typename T>
 class DOMWrapperMap;
-class HeapObjectHeader;
 class ScriptWrappable;
 class ScriptWrappableVisitor;
 template <typename T>
@@ -27,18 +26,17 @@ class TraceWrapperBaseForSupplement;
 template <typename T>
 class TraceWrapperV8Reference;
 
-using HeapObjectHeaderCallback = HeapObjectHeader* (*)(const void*);
 using MissedWriteBarrierCallback = void (*)();
-using TraceWrappersCallback = void (*)(const ScriptWrappableVisitor*,
-                                       const void* self);
 using NameCallback = const char* (*)(const void* self);
 
-#define DEFINE_TRAIT_FOR_TRACE_WRAPPERS(ClassName)                   \
-  template <>                                                        \
-  inline void TraceTrait<ClassName>::TraceMarkedWrapper(             \
-      const ScriptWrappableVisitor* visitor, const void* t) {        \
-    const ClassName* traceable = ToWrapperTracingType(t);            \
-    traceable->TraceWrappers(visitor);                               \
+#define DEFINE_TRAIT_FOR_TRACE_WRAPPERS(ClassName)                            \
+  template <>                                                                 \
+  inline void TraceTrait<ClassName>::TraceWrappers(                           \
+      ScriptWrappableVisitor* visitor, void* t) {                             \
+    static_assert(sizeof(ClassName), "type needs to be defined");             \
+    static_assert(IsGarbageCollectedType<ClassName>::value,                   \
+                  "only objects deriving from GarbageCollected can be used"); \
+    static_cast<ClassName*>(t)->TraceWrappers(visitor);                       \
   }
 
 // WrapperDescriptor contains enough information to visit a
@@ -46,9 +44,8 @@ using NameCallback = const char* (*)(const void* self);
 // It is passed to ScriptWrappableVisitor::Visit method.
 struct WrapperDescriptor {
   STACK_ALLOCATED();
-  const void* traceable;
+  const void* base_object_payload;
   TraceWrappersCallback trace_wrappers_callback;
-  HeapObjectHeaderCallback heap_object_header_callback;
   MissedWriteBarrierCallback missed_write_barrier_callback;
   NameCallback name_callback;
 };
@@ -125,8 +122,9 @@ class PLATFORM_EXPORT ScriptWrappableVisitor {
 
   template <typename T>
   static WrapperDescriptor WrapperDescriptorFor(const T* traceable) {
-    return {traceable, TraceTrait<T>::TraceMarkedWrapper,
-            TraceTrait<T>::GetHeapObjectHeader,
+    TraceWrapperDescriptor desc =
+        TraceTrait<T>::GetTraceWrapperDescriptor(const_cast<T*>(traceable));
+    return {desc.base_object_payload, desc.callback,
             ScriptWrappableVisitor::MissedWriteBarrier<T>,
             ScriptWrappableVisitor::NameCallback<T>};
   }
@@ -148,6 +146,7 @@ class PLATFORM_EXPORT ScriptWrappableVisitor {
 
   template <typename T>
   static const char* NameCallback(const void* traceable) {
+    // Mixns never inherit from TraceWrapperBase.
     return NameInHeapSnapshot(static_cast<const T*>(traceable));
   }
 

third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitorVerifier.h

@@ -16,9 +16,8 @@ class ScriptWrappableVisitorVerifier final : public ScriptWrappableVisitor {
  protected:
   void Visit(const TraceWrapperV8Reference<v8::Value>&) const final {}
   void Visit(const WrapperDescriptor& wrapper_descriptor) const final {
-    HeapObjectHeader* header = wrapper_descriptor.heap_object_header_callback(
-        wrapper_descriptor.traceable);
-    if (!header->IsWrapperHeaderMarked()) {
+    if (!HeapObjectHeader::FromPayload(wrapper_descriptor.base_object_payload)
+             ->IsWrapperHeaderMarked()) {
       // If this branch is hit, it means that a white (not discovered by
       // traceWrappers) object was assigned as a member to a black object
       // (already processed by traceWrappers). Black object will not be

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

@@ -16,6 +16,7 @@ namespace blink {
 
 class MarkingVisitor;
 class Visitor;
+class ScriptWrappableVisitor;
 
 using Address = uint8_t*;
 
@@ -23,6 +24,7 @@ using FinalizationCallback = void (*)(void*);
 using VisitorCallback = void (*)(Visitor*, void*);
 using MarkingVisitorCallback = void (*)(MarkingVisitor*, void*);
 using TraceCallback = VisitorCallback;
+using TraceWrappersCallback = void (*)(ScriptWrappableVisitor*, void*);
 using WeakCallback = VisitorCallback;
 using EphemeronCallback = VisitorCallback;
 

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

@@ -16,7 +16,6 @@ namespace blink {
 template <typename T>
 class GarbageCollected;
 class HeapObjectHeader;
-class ScriptWrappableVisitor;
 
 // GC_PLUGIN_IGNORE is used to make the plugin ignore a particular class or
 // field when checking for proper usage.  When using GC_PLUGIN_IGNORE
@@ -54,6 +53,11 @@ struct TraceDescriptor {
   bool can_trace_eagerly;
 };
 
+struct TraceWrapperDescriptor {
+  void* base_object_payload;
+  TraceWrappersCallback callback;
+};
+
 // The GarbageCollectedMixin interface and helper macro
 // USING_GARBAGE_COLLECTED_MIXIN can be used to automatically define
 // TraceTrait/ObjectAliveTrait on non-leftmost deriving classes
@@ -71,43 +75,39 @@ struct TraceDescriptor {
 // };
 //
 // With the helper, as long as we are using Member<B>, TypeTrait<B> will
-// dispatch adjustAndMark dynamically to find collect addr of the object header.
+// dispatch dynamically to retrieve the necessary tracing and header methods.
 // Note that this is only enabled for Member<B>. For Member<A> which we can
-// compute the object header addr statically, this dynamic dispatch is not used.
-//
+// compute the necessary methods and pointers statically and this dynamic
+// dispatch is not used.
 class PLATFORM_EXPORT GarbageCollectedMixin {
  public:
   typedef int IsGarbageCollectedMixinMarker;
   virtual void Trace(Visitor*) {}
-  virtual TraceDescriptor GetTraceDescriptor() const = 0;
   virtual HeapObjectHeader* GetHeapObjectHeader() const = 0;
-  virtual void AdjustAndTraceMarkedWrapper(
-      const ScriptWrappableVisitor*) const = 0;
+  virtual TraceDescriptor GetTraceDescriptor() const = 0;
+  virtual TraceWrapperDescriptor GetTraceWrapperDescriptor() const = 0;
 };
 
-#define DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(TYPE)                          \
- public:                                                                      \
-  TraceDescriptor GetTraceDescriptor() const override {                       \
-    typedef WTF::IsSubclassOfTemplate<typename std::remove_const<TYPE>::type, \
-                                      blink::GarbageCollected>                \
-        IsSubclassOfGarbageCollected;                                         \
-    static_assert(                                                            \
-        IsSubclassOfGarbageCollected::value,                                  \
-        "only garbage collected objects can have garbage collected mixins");  \
-    return {const_cast<TYPE*>(static_cast<const TYPE*>(this)),                \
-            TraceTrait<TYPE>::Trace, TraceEagerlyTrait<TYPE>::value};         \
-  }                                                                           \
-                                                                              \
-  void AdjustAndTraceMarkedWrapper(const ScriptWrappableVisitor* visitor)     \
-      const override {                                                        \
-    const TYPE* base = static_cast<const TYPE*>(this);                        \
-    TraceTrait<TYPE>::TraceMarkedWrapper(visitor, base);                      \
-  }                                                                           \
-                                                                              \
-  HeapObjectHeader* GetHeapObjectHeader() const override {                    \
-    return HeapObjectHeader::FromPayload(static_cast<const TYPE*>(this));     \
-  }                                                                           \
-                                                                              \
+#define DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(TYPE)                         \
+ public:                                                                     \
+  HeapObjectHeader* GetHeapObjectHeader() const override {                   \
+    static_assert(                                                           \
+        WTF::IsSubclassOfTemplate<typename std::remove_const<TYPE>::type,    \
+                                  blink::GarbageCollected>::value,           \
+        "only garbage collected objects can have garbage collected mixins"); \
+    return HeapObjectHeader::FromPayload(static_cast<const TYPE*>(this));    \
+  }                                                                          \
+                                                                             \
+  TraceDescriptor GetTraceDescriptor() const override {                      \
+    return {const_cast<TYPE*>(static_cast<const TYPE*>(this)),               \
+            TraceTrait<TYPE>::Trace, TraceEagerlyTrait<TYPE>::value};        \
+  }                                                                          \
+                                                                             \
+  TraceWrapperDescriptor GetTraceWrapperDescriptor() const override {        \
+    return {const_cast<TYPE*>(static_cast<const TYPE*>(this)),               \
+            TraceTrait<TYPE>::TraceWrappers};                                \
+  }                                                                          \
+                                                                             \
  private:
 
 // A C++ object's vptr will be initialized to its leftmost base's vtable after
@@ -212,14 +212,13 @@ class GarbageCollectedMixinConstructorMarker
 //    // ambiguous. USING_GARBAGE_COLLECTED_MIXIN(TYPE) overrides them later
 //    // and provides the implementations.
 //  };
-#define MERGE_GARBAGE_COLLECTED_MIXINS()                          \
- public:                                                          \
-  TraceDescriptor GetTraceDescriptor() const override = 0;        \
-  HeapObjectHeader* GetHeapObjectHeader() const override = 0;     \
-  void AdjustAndTraceMarkedWrapper(const ScriptWrappableVisitor*) \
-      const override = 0;                                         \
-                                                                  \
- private:                                                         \
+#define MERGE_GARBAGE_COLLECTED_MIXINS()                                 \
+ public:                                                                 \
+  HeapObjectHeader* GetHeapObjectHeader() const override = 0;            \
+  TraceDescriptor GetTraceDescriptor() const override = 0;               \
+  TraceWrapperDescriptor GetTraceWrapperDescriptor() const override = 0; \
+                                                                         \
+ private:                                                                \
   using merge_garbage_collected_mixins_requires_semicolon = void
 
 // Base class for objects allocated in the Blink garbage-collected heap.

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

@@ -54,20 +54,16 @@ class AdjustAndMarkTrait<T, false> {
     return {self, TraceTrait<T>::Trace, TraceEagerlyTrait<T>::value};
   }
 
-  static HeapObjectHeader* GetHeapObjectHeader(const T* self) {
+  static TraceWrapperDescriptor GetTraceWrapperDescriptor(void* self) {
+    return {self, TraceTrait<T>::TraceWrappers};
+  }
+
+  static HeapObjectHeader* GetHeapObjectHeader(void* self) {
 #if DCHECK_IS_ON()
     HeapObjectHeader::CheckFromPayload(self);
 #endif
     return HeapObjectHeader::FromPayload(self);
   }
-
-  static void TraceMarkedWrapper(const ScriptWrappableVisitor* visitor,
-                                 const T* self) {
-    // The term *mark* is misleading here as we effectively trace through the
-    // API boundary, i.e., tell V8 that an object is alive. Actual marking
-    // will be done in V8.
-    visitor->DispatchTraceWrappers(self);
-  }
 };
 
 template <typename T>
@@ -75,18 +71,19 @@ class AdjustAndMarkTrait<T, true> {
   STATIC_ONLY(AdjustAndMarkTrait);
 
  public:
-  static HeapObjectHeader* GetHeapObjectHeader(const T* self) {
-    return self->GetHeapObjectHeader();
+  static TraceDescriptor GetTraceDescriptor(const T* self) {
+    DCHECK(self);
+    return self->GetTraceDescriptor();
   }
 
-  static TraceDescriptor GetTraceDescriptor(T* self) {
+  static TraceWrapperDescriptor GetTraceWrapperDescriptor(const T* self) {
     DCHECK(self);
-    return self->GetTraceDescriptor();
+    return self->GetTraceWrapperDescriptor();
   }
 
-  static void TraceMarkedWrapper(const ScriptWrappableVisitor* visitor,
-                                 const T* self) {
-    self->AdjustAndTraceMarkedWrapper(visitor);
+  static HeapObjectHeader* GetHeapObjectHeader(const T* self) {
+    DCHECK(self);
+    return self->GetHeapObjectHeader();
   }
 };
 
@@ -173,10 +170,11 @@ struct TraceCollectionIfEnabled {
   }
 };
 
-// The TraceTrait is used to specify how to mark an object pointer and
-// how to trace all of the pointers in the object.
+// The TraceTrait is used to specify how to trace and object for Oilpan and
+// wrapper tracing.
 //
-// By default, the 'trace' method implemented on an object itself is
+//
+// By default, the 'Trace' method implemented on an object itself is
 // used to trace the pointers to other heap objects inside the object.
 //
 // However, the TraceTrait can be specialized to use a different
@@ -193,17 +191,17 @@ class TraceTrait {
     return AdjustAndMarkTrait<T>::GetTraceDescriptor(static_cast<T*>(self));
   }
 
-  static void Trace(Visitor*, void* self);
-  static void TraceMarkedWrapper(const ScriptWrappableVisitor*, const void*);
-  static HeapObjectHeader* GetHeapObjectHeader(const void*);
+  static TraceWrapperDescriptor GetTraceWrapperDescriptor(void* self) {
+    return AdjustAndMarkTrait<T>::GetTraceWrapperDescriptor(
+        static_cast<T*>(self));
+  }
 
- private:
-  static const T* ToWrapperTracingType(const void* t) {
-    static_assert(sizeof(T), "type needs to be defined");
-    static_assert(IsGarbageCollectedType<T>::value,
-                  "only objects deriving from GarbageCollected can be used");
-    return reinterpret_cast<const T*>(t);
+  static HeapObjectHeader* GetHeapObjectHeader(void* self) {
+    return AdjustAndMarkTrait<T>::GetHeapObjectHeader(static_cast<T*>(self));
   }
+
+  static void Trace(Visitor*, void* self);
+  static void TraceWrappers(ScriptWrappableVisitor*, void*);
 };
 
 template <typename T>
@@ -216,16 +214,13 @@ void TraceTrait<T>::Trace(Visitor* visitor, void* self) {
 }
 
 template <typename T>
-void TraceTrait<T>::TraceMarkedWrapper(const ScriptWrappableVisitor* visitor,
-                                       const void* t) {
-  const T* traceable = ToWrapperTracingType(t);
-  AdjustAndMarkTrait<T>::TraceMarkedWrapper(visitor, traceable);
+void TraceTrait<T>::TraceWrappers(ScriptWrappableVisitor* visitor, void* self) {
+  static_assert(sizeof(T), "type needs to be defined");
+  static_assert(IsGarbageCollectedType<T>::value,
+                "only objects deriving from GarbageCollected can be used");
+  visitor->DispatchTraceWrappers(static_cast<T*>(self));
 }
 
-template <typename T>
-HeapObjectHeader* TraceTrait<T>::GetHeapObjectHeader(const void* t) {
-  return AdjustAndMarkTrait<T>::GetHeapObjectHeader(ToWrapperTracingType(t));
-}
 
 template <typename T, typename Traits>
 struct TraceTrait<HeapVectorBacking<T, Traits>> {