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

This reverts commit 9c72d479.

Reason for revert: Likely culprit of https://crbug.com/819134

Original change's description:
> [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}

TBR=haraken@chromium.org,mlippautz@chromium.org

Change-Id: Iaced3e8e5f9d46540a5fef2f8e1d6c1682217786
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:802273
Reviewed-on: https://chromium-review.googlesource.com/950779Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541063}

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

@@ -501,6 +501,7 @@ 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());
 
@@ -515,7 +516,7 @@ TEST(ScriptWrappableMarkingVisitorTest, MixinTracing) {
   TraceWrapperMember<Mixin> mixin_handle = mixin;
   EXPECT_TRUE(base_header->IsWrapperHeaderMarked());
   EXPECT_FALSE(visitor->MarkingDeque()->IsEmpty());
-  EXPECT_TRUE(visitor->MarkingDequeContains(base));
+  EXPECT_TRUE(visitor->MarkingDequeContains(mixin));
 
   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.base_object_payload;
+  const void* traceable = wrapper_descriptor.traceable;
   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.base_object_payload,
+  return {node, wrapper_descriptor.traceable,
           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, const_cast<void*>(item.traceable));
+    item.trace_wrappers_callback(this, 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 =
-      HeapObjectHeader::FromPayload(wrapper_descriptor.base_object_payload);
+  HeapObjectHeader* header = wrapper_descriptor.heap_object_header_callback(
+      wrapper_descriptor.traceable);
   if (header->IsWrapperHeaderMarked())
     return;
   MarkWrapperHeader(header);

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

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

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

@@ -17,6 +17,7 @@ namespace blink {
 
 template <typename T>
 class DOMWrapperMap;
+class HeapObjectHeader;
 class ScriptWrappable;
 class ScriptWrappableVisitor;
 template <typename T>
@@ -26,17 +27,18 @@ 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>::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);                       \
+#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);                               \
   }
 
 // WrapperDescriptor contains enough information to visit a
@@ -44,8 +46,9 @@ using NameCallback = const char* (*)(const void* self);
 // It is passed to ScriptWrappableVisitor::Visit method.
 struct WrapperDescriptor {
   STACK_ALLOCATED();
-  const void* base_object_payload;
+  const void* traceable;
   TraceWrappersCallback trace_wrappers_callback;
+  HeapObjectHeaderCallback heap_object_header_callback;
   MissedWriteBarrierCallback missed_write_barrier_callback;
   NameCallback name_callback;
 };
@@ -122,9 +125,8 @@ class PLATFORM_EXPORT ScriptWrappableVisitor {
 
   template <typename T>
   static WrapperDescriptor WrapperDescriptorFor(const T* traceable) {
-    TraceWrapperDescriptor desc =
-        TraceTrait<T>::GetTraceWrapperDescriptor(const_cast<T*>(traceable));
-    return {desc.base_object_payload, desc.callback,
+    return {traceable, TraceTrait<T>::TraceMarkedWrapper,
+            TraceTrait<T>::GetHeapObjectHeader,
             ScriptWrappableVisitor::MissedWriteBarrier<T>,
             ScriptWrappableVisitor::NameCallback<T>};
   }
@@ -146,7 +148,6 @@ 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,8 +16,9 @@ class ScriptWrappableVisitorVerifier final : public ScriptWrappableVisitor {
  protected:
   void Visit(const TraceWrapperV8Reference<v8::Value>&) const final {}
   void Visit(const WrapperDescriptor& wrapper_descriptor) const final {
-    if (!HeapObjectHeader::FromPayload(wrapper_descriptor.base_object_payload)
-             ->IsWrapperHeaderMarked()) {
+    HeapObjectHeader* header = wrapper_descriptor.heap_object_header_callback(
+        wrapper_descriptor.traceable);
+    if (!header->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,7 +16,6 @@ namespace blink {
 
 class MarkingVisitor;
 class Visitor;
-class ScriptWrappableVisitor;
 
 using Address = uint8_t*;
 
@@ -24,7 +23,6 @@ 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,6 +16,7 @@ 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
@@ -53,11 +54,6 @@ 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
@@ -75,39 +71,43 @@ struct TraceWrapperDescriptor {
 // };
 //
 // With the helper, as long as we are using Member<B>, TypeTrait<B> will
-// dispatch dynamically to retrieve the necessary tracing and header methods.
+// dispatch adjustAndMark dynamically to find collect addr of the object header.
 // Note that this is only enabled for Member<B>. For Member<A> which we can
-// compute the necessary methods and pointers statically and this dynamic
-// dispatch is not used.
+// compute the object header addr statically, this dynamic dispatch is not used.
+//
 class PLATFORM_EXPORT GarbageCollectedMixin {
  public:
   typedef int IsGarbageCollectedMixinMarker;
   virtual void Trace(Visitor*) {}
-  virtual HeapObjectHeader* GetHeapObjectHeader() const = 0;
   virtual TraceDescriptor GetTraceDescriptor() const = 0;
-  virtual TraceWrapperDescriptor GetTraceWrapperDescriptor() const = 0;
+  virtual HeapObjectHeader* GetHeapObjectHeader() const = 0;
+  virtual void AdjustAndTraceMarkedWrapper(
+      const ScriptWrappableVisitor*) const = 0;
 };
 
-#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};                                \
-  }                                                                          \
-                                                                             \
+#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));     \
+  }                                                                           \
+                                                                              \
  private:
 
 // A C++ object's vptr will be initialized to its leftmost base's vtable after
@@ -212,13 +212,14 @@ class GarbageCollectedMixinConstructorMarker
 //    // ambiguous. USING_GARBAGE_COLLECTED_MIXIN(TYPE) overrides them later
 //    // and provides the implementations.
 //  };
-#define MERGE_GARBAGE_COLLECTED_MIXINS()                                 \
- public:                                                                 \
-  HeapObjectHeader* GetHeapObjectHeader() const override = 0;            \
-  TraceDescriptor GetTraceDescriptor() const override = 0;               \
-  TraceWrapperDescriptor GetTraceWrapperDescriptor() const override = 0; \
-                                                                         \
- private:                                                                \
+#define MERGE_GARBAGE_COLLECTED_MIXINS()                          \
+ public:                                                          \
+  TraceDescriptor GetTraceDescriptor() const override = 0;        \
+  HeapObjectHeader* GetHeapObjectHeader() const override = 0;     \
+  void AdjustAndTraceMarkedWrapper(const ScriptWrappableVisitor*) \
+      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,16 +54,20 @@ class AdjustAndMarkTrait<T, false> {
     return {self, TraceTrait<T>::Trace, TraceEagerlyTrait<T>::value};
   }
 
-  static TraceWrapperDescriptor GetTraceWrapperDescriptor(void* self) {
-    return {self, TraceTrait<T>::TraceWrappers};
-  }
-
-  static HeapObjectHeader* GetHeapObjectHeader(void* self) {
+  static HeapObjectHeader* GetHeapObjectHeader(const T* 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>
@@ -71,19 +75,18 @@ class AdjustAndMarkTrait<T, true> {
   STATIC_ONLY(AdjustAndMarkTrait);
 
  public:
-  static TraceDescriptor GetTraceDescriptor(const T* self) {
-    DCHECK(self);
-    return self->GetTraceDescriptor();
+  static HeapObjectHeader* GetHeapObjectHeader(const T* self) {
+    return self->GetHeapObjectHeader();
   }
 
-  static TraceWrapperDescriptor GetTraceWrapperDescriptor(const T* self) {
+  static TraceDescriptor GetTraceDescriptor(T* self) {
     DCHECK(self);
-    return self->GetTraceWrapperDescriptor();
+    return self->GetTraceDescriptor();
   }
 
-  static HeapObjectHeader* GetHeapObjectHeader(const T* self) {
-    DCHECK(self);
-    return self->GetHeapObjectHeader();
+  static void TraceMarkedWrapper(const ScriptWrappableVisitor* visitor,
+                                 const T* self) {
+    self->AdjustAndTraceMarkedWrapper(visitor);
   }
 };
 
@@ -170,11 +173,10 @@ struct TraceCollectionIfEnabled {
   }
 };
 
-// The TraceTrait is used to specify how to trace and object for Oilpan and
-// wrapper tracing.
+// The TraceTrait is used to specify how to mark an object pointer and
+// how to trace all of the pointers in the object.
 //
-//
-// 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
@@ -191,17 +193,17 @@ class TraceTrait {
     return AdjustAndMarkTrait<T>::GetTraceDescriptor(static_cast<T*>(self));
   }
 
-  static TraceWrapperDescriptor GetTraceWrapperDescriptor(void* self) {
-    return AdjustAndMarkTrait<T>::GetTraceWrapperDescriptor(
-        static_cast<T*>(self));
-  }
+  static void Trace(Visitor*, void* self);
+  static void TraceMarkedWrapper(const ScriptWrappableVisitor*, const void*);
+  static HeapObjectHeader* GetHeapObjectHeader(const void*);
 
-  static HeapObjectHeader* GetHeapObjectHeader(void* self) {
-    return AdjustAndMarkTrait<T>::GetHeapObjectHeader(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 void Trace(Visitor*, void* self);
-  static void TraceWrappers(ScriptWrappableVisitor*, void*);
 };
 
 template <typename T>
@@ -214,13 +216,16 @@ void TraceTrait<T>::Trace(Visitor* visitor, void* self) {
 }
 
 template <typename T>
-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));
+void TraceTrait<T>::TraceMarkedWrapper(const ScriptWrappableVisitor* visitor,
+                                       const void* t) {
+  const T* traceable = ToWrapperTracingType(t);
+  AdjustAndMarkTrait<T>::TraceMarkedWrapper(visitor, traceable);
 }
 
+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>> {