[bindings] Construct ScriptWrappable visitor in V8PerIsolateData

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

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

@@ -56,8 +56,8 @@ class TemporaryScriptWrappableVisitorScope {
 class InterceptingScriptWrappableMarkingVisitor
     : public blink::ScriptWrappableMarkingVisitor {
  public:
-  InterceptingScriptWrappableMarkingVisitor(v8::Isolate* isolate)
-      : ScriptWrappableMarkingVisitor(isolate),
+  InterceptingScriptWrappableMarkingVisitor()
+      : ScriptWrappableMarkingVisitor(ThreadState::Current()),
         marked_wrappers_(new size_t(0)) {}
   ~InterceptingScriptWrappableMarkingVisitor() override {
     delete marked_wrappers_;
@@ -93,7 +93,7 @@ class InterceptingScriptWrappableMarkingVisitorScope
       : TemporaryScriptWrappableVisitorScope(
             isolate,
             std::unique_ptr<InterceptingScriptWrappableMarkingVisitor>(
-                new InterceptingScriptWrappableMarkingVisitor(isolate))) {
+                new InterceptingScriptWrappableMarkingVisitor())) {
     Visitor()->Start();
   }
 
@@ -285,9 +285,8 @@ TEST(ScriptWrappableMarkingVisitorTest, VtableAtObjectStart) {
   // at the start of a ScriptWrappableMarkingVisitor object. We do this to
   // mitigate potential problems that could be caused by LTO when passing
   // v8::EmbedderHeapTracer across the API boundary.
-  V8TestingScope scope;
   std::unique_ptr<blink::ScriptWrappableMarkingVisitor> visitor(
-      new ScriptWrappableMarkingVisitor(scope.GetIsolate()));
+      new ScriptWrappableMarkingVisitor(ThreadState::Current()));
   CHECK_EQ(
       static_cast<void*>(visitor.get()),
       static_cast<void*>(dynamic_cast<v8::EmbedderHeapTracer*>(visitor.get())));

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

@@ -13,6 +13,9 @@ namespace {
 
 class VerifyingScriptWrappableVisitor : public ScriptWrappableVisitor {
  public:
+  VerifyingScriptWrappableVisitor()
+      : ScriptWrappableVisitor(ThreadState::Current()) {}
+
   // Visitor interface.
   void Visit(const TraceWrapperV8Reference<v8::Value>&) override {}
   void Visit(DOMWrapperMap<ScriptWrappable>*,

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

@@ -17,7 +17,10 @@ namespace blink {
 
 V8EmbedderGraphBuilder::V8EmbedderGraphBuilder(v8::Isolate* isolate,
                                                Graph* graph)
-    : isolate_(isolate), current_parent_(nullptr), graph_(graph) {}
+    : ScriptWrappableVisitor(ThreadState::Current()),
+      isolate_(isolate),
+      current_parent_(nullptr),
+      graph_(graph) {}
 
 void V8EmbedderGraphBuilder::BuildEmbedderGraphCallback(
     v8::Isolate* isolate,

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

@@ -63,7 +63,6 @@
 #include "third_party/blink/renderer/core/script/modulator.h"
 #include "third_party/blink/renderer/core/workers/worker_global_scope.h"
 #include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"
-#include "third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.h"
 #include "third_party/blink/renderer/platform/bindings/v8_per_context_data.h"
 #include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
@@ -632,10 +631,7 @@ static void HostGetImportMetaProperties(v8::Local<v8::Context> context,
 static void InitializeV8Common(v8::Isolate* isolate) {
   isolate->AddGCPrologueCallback(V8GCController::GcPrologue);
   isolate->AddGCEpilogueCallback(V8GCController::GcEpilogue);
-  std::unique_ptr<ScriptWrappableMarkingVisitor> visitor(
-      new ScriptWrappableMarkingVisitor(isolate));
-  V8PerIsolateData::From(isolate)->SetScriptWrappableMarkingVisitor(
-      std::move(visitor));
+
   isolate->SetEmbedderHeapTracer(
       V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor());
 

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

@@ -45,7 +45,7 @@ void ScriptWrappableMarkingVisitor::TracePrologue() {
 void ScriptWrappableMarkingVisitor::EnterFinalPause(EmbedderStackState) {
   CHECK(ThreadState::Current());
   CHECK(!ThreadState::Current()->IsWrapperTracingForbidden());
-  ActiveScriptWrappableBase::TraceActiveScriptWrappables(isolate_, this);
+  ActiveScriptWrappableBase::TraceActiveScriptWrappables(isolate(), this);
 }
 
 void ScriptWrappableMarkingVisitor::TraceEpilogue() {
@@ -53,7 +53,7 @@ void ScriptWrappableMarkingVisitor::TraceEpilogue() {
   CHECK(!ThreadState::Current()->IsWrapperTracingForbidden());
   DCHECK(marking_deque_.IsEmpty());
 #if DCHECK_IS_ON()
-  ScriptWrappableVisitorVerifier verifier;
+  ScriptWrappableVisitorVerifier verifier(ThreadState::Current());
   for (auto& marking_data : verifier_deque_) {
     // Check that all children of this object are marked.
     marking_data.Trace(&verifier);
@@ -246,7 +246,7 @@ void ScriptWrappableMarkingVisitor::Visit(
   // requires us to bail out here when tracing is not in progress.
   if (!tracing_in_progress_ || traced_wrapper.Get().IsEmpty())
     return;
-  traced_wrapper.Get().RegisterExternalReference(isolate_);
+  traced_wrapper.Get().RegisterExternalReference(isolate());
 }
 
 void ScriptWrappableMarkingVisitor::VisitWithWrappers(

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

@@ -63,7 +63,8 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
                            DOMWrapperMap<ScriptWrappable>*,
                            ScriptWrappable* key);
 
-  ScriptWrappableMarkingVisitor(v8::Isolate* isolate) : isolate_(isolate){};
+  explicit ScriptWrappableMarkingVisitor(ThreadState* thread_state)
+      : ScriptWrappableVisitor(thread_state) {}
   ~ScriptWrappableMarkingVisitor() override;
 
   bool WrapperTracingInProgress() const { return tracing_in_progress_; }
@@ -91,8 +92,6 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
  protected:
   using Visitor::Visit;
 
-  v8::Isolate* isolate() const { return isolate_; }
-
  private:
   class MarkingDequeItem {
    public:
@@ -189,7 +188,6 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   // - objects this headers belong to are invalidated by the oilpan GC in
   //   invalidateDeadObjectsInMarkingDeque.
   WTF::Vector<HeapObjectHeader*> headers_to_unmark_;
-  v8::Isolate* isolate_;
 
   FRIEND_TEST_ALL_PREFIXES(ScriptWrappableMarkingVisitorTest, MixinTracing);
   FRIEND_TEST_ALL_PREFIXES(ScriptWrappableMarkingVisitorTest,

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

@@ -15,7 +15,8 @@ namespace blink {
 // references related to wrappers.
 class PLATFORM_EXPORT ScriptWrappableVisitor : public Visitor {
  public:
-  ScriptWrappableVisitor() : Visitor(ThreadState::Current()) {}
+  explicit ScriptWrappableVisitor(ThreadState* thread_state)
+      : Visitor(thread_state) {}
 
   // Unused blink::Visitor overrides. Derived visitors should still override
   // the cross-component visitation methods. See Visitor documentation.

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

@@ -16,6 +16,9 @@ namespace blink {
 // is also marked.
 class ScriptWrappableVisitorVerifier final : public ScriptWrappableVisitor {
  public:
+  ScriptWrappableVisitorVerifier(ThreadState* thread_state)
+      : ScriptWrappableVisitor(thread_state) {}
+
   void Visit(const TraceWrapperV8Reference<v8::Value>&) final {}
 
   void VisitWithWrappers(void* object, TraceDescriptor descriptor) final {

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

@@ -38,6 +38,7 @@
 #include "third_party/blink/renderer/platform/bindings/dom_data_store.h"
 #include "third_party/blink/renderer/platform/bindings/script_forbidden_scope.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
+#include "third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.h"
 #include "third_party/blink/renderer/platform/bindings/v8_binding.h"
 #include "third_party/blink/renderer/platform/bindings/v8_object_constructor.h"
 #include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
@@ -80,6 +81,8 @@ V8PerIsolateData::V8PerIsolateData(
       use_counter_disabled_(false),
       is_handling_recursion_level_error_(false),
       is_reporting_exception_(false),
+      script_wrappable_visitor_(
+          new ScriptWrappableMarkingVisitor(ThreadState::Current())),
       runtime_call_stats_(base::DefaultTickClock::GetInstance()),
       handled_near_v8_heap_limit_(false) {
   // FIXME: Remove once all v8::Isolate::GetCurrent() calls are gone.

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

@@ -200,11 +200,6 @@ class PLATFORM_EXPORT V8PerIsolateData {
     return active_script_wrappables_.Get();
   }
 
-  void SetScriptWrappableMarkingVisitor(
-      std::unique_ptr<ScriptWrappableMarkingVisitor> visitor) {
-    script_wrappable_visitor_ = std::move(visitor);
-  }
-
   ScriptWrappableMarkingVisitor* GetScriptWrappableMarkingVisitor() const {
     return script_wrappable_visitor_.get();
   }