[bindings,heap] Prepare PerIsolateData for holding different heap tracer

- Prepare V8PerIsolateData for holding V8HeapController.
- Harden existing tests to swap in the expected visitor/controller. This way it
  is possible to test ScriptWrappableVisitor while already running on a different
  embedder heap tracer.

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

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

@@ -65,6 +65,7 @@
 #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/heap/v8_heap_controller.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
 #include "third_party/blink/renderer/platform/loader/fetch/access_control_status.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -622,12 +623,11 @@ 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));
+  V8PerIsolateData::From(isolate)->SetV8HeapController(
+      std::unique_ptr<V8HeapController>{
+          new ScriptWrappableMarkingVisitor(isolate)});
   isolate->SetEmbedderHeapTracer(
-      V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor());
+      V8PerIsolateData::From(isolate)->GetV8HeapController());
 
   isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kScoped);
 

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

@@ -67,6 +67,7 @@ void ScriptWrappableMarkingVisitor::TraceEpilogue() {
 }
 
 void ScriptWrappableMarkingVisitor::AbortTracing() {
+  CHECK(tracing_in_progress_);
   CHECK(ThreadState::Current());
   should_cleanup_ = true;
   tracing_in_progress_ = false;
@@ -218,10 +219,12 @@ void ScriptWrappableMarkingVisitor::MarkWrapperHeader(
 void ScriptWrappableMarkingVisitor::WriteBarrier(
     v8::Isolate* isolate,
     const TraceWrapperV8Reference<v8::Value>& dst_object) {
-  ScriptWrappableMarkingVisitor* visitor = CurrentVisitor(isolate);
-  if (dst_object.IsEmpty() || !visitor->WrapperTracingInProgress())
+  if (!ThreadState::IsAnyWrapperTracing() || dst_object.IsEmpty())
     return;
 
+  ScriptWrappableMarkingVisitor* visitor = CurrentVisitor(isolate);
+  if (!visitor->WrapperTracingInProgress())
+    return;
   // Conservatively assume that the source object containing |dst_object| is
   // marked.
   visitor->Trace(dst_object);
@@ -231,6 +234,9 @@ void ScriptWrappableMarkingVisitor::WriteBarrier(
     v8::Isolate* isolate,
     DOMWrapperMap<ScriptWrappable>* wrapper_map,
     ScriptWrappable* key) {
+  if (!ThreadState::IsAnyWrapperTracing())
+    return;
+
   ScriptWrappableMarkingVisitor* visitor = CurrentVisitor(isolate);
   if (!visitor->WrapperTracingInProgress())
     return;
@@ -301,24 +307,32 @@ void ScriptWrappableMarkingVisitor::InvalidateDeadObjectsInMarkingDeque() {
   }
 }
 
+void ScriptWrappableMarkingVisitor::FinalizeAndCleanup() {
+  FinalizeTracing();
+  PerformCleanup();
+}
+
 void ScriptWrappableMarkingVisitor::InvalidateDeadObjectsInMarkingDeque(
     v8::Isolate* isolate) {
   ScriptWrappableMarkingVisitor* script_wrappable_visitor =
-      V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor();
+      static_cast<ScriptWrappableMarkingVisitor*>(
+          V8PerIsolateData::From(isolate)->GetV8HeapController());
   if (script_wrappable_visitor)
     script_wrappable_visitor->InvalidateDeadObjectsInMarkingDeque();
 }
 
 void ScriptWrappableMarkingVisitor::PerformCleanup(v8::Isolate* isolate) {
   ScriptWrappableMarkingVisitor* script_wrappable_visitor =
-      V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor();
+      static_cast<ScriptWrappableMarkingVisitor*>(
+          V8PerIsolateData::From(isolate)->GetV8HeapController());
   if (script_wrappable_visitor)
     script_wrappable_visitor->PerformCleanup();
 }
 
 ScriptWrappableMarkingVisitor* ScriptWrappableMarkingVisitor::CurrentVisitor(
     v8::Isolate* isolate) {
-  return V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor();
+  return static_cast<ScriptWrappableMarkingVisitor*>(
+      V8PerIsolateData::From(isolate)->GetV8HeapController());
 }
 
 bool ScriptWrappableMarkingVisitor::MarkingDequeContains(void* needle) {

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

@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/platform/bindings/script_wrappable_visitor.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/v8_heap_controller.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/time.h"
@@ -29,8 +30,8 @@ class TraceWrapperV8Reference;
 // reachable wrappers. V8 calls this visitor during its garbage collection,
 // see v8::EmbedderHeapTracer.
 class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
-    : public v8::EmbedderHeapTracer,
-      public ScriptWrappableVisitor {
+    : public ScriptWrappableVisitor,
+      public V8HeapController {
   DISALLOW_IMPLICIT_CONSTRUCTORS(ScriptWrappableMarkingVisitor);
 
  public:
@@ -69,7 +70,6 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   bool WrapperTracingInProgress() const { return tracing_in_progress_; }
 
   // v8::EmbedderHeapTracer interface.
-
   void TracePrologue() override;
   void RegisterV8References(const std::vector<std::pair<void*, void*>>&
                                 internal_fields_of_potential_wrappers) override;
@@ -81,8 +81,10 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   void EnterFinalPause() override;
   size_t NumberOfWrappersToTrace() override;
 
-  // ScriptWrappableVisitor interface.
+  // V8HeapController interface.
+  void FinalizeAndCleanup() override;
 
+  // ScriptWrappableVisitor interface.
   void Visit(const TraceWrapperV8Reference<v8::Value>&) override;
   void VisitWithWrappers(void*, TraceDescriptor) override;
   void Visit(DOMWrapperMap<ScriptWrappable>*,

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

@@ -114,13 +114,12 @@ inline void V8DOMWrapper::SetNativeInfoInternal(
   wrapper->SetAlignedPointerInInternalFields(base::size(indices), indices,
                                              values);
   auto* per_isolate_data = V8PerIsolateData::From(isolate);
-  // We notify ScriptWrappableVisitor about the new wrapper association,
-  // so the visitor can make sure to trace the association (in case it is
+  // We notify V8HeapController about the new wrapper association,
+  // so the controller can make sure to trace the association (in case it is
   // currently tracing).  Because of some optimizations, V8 will not
   // necessarily detect wrappers created during its incremental marking.
-  per_isolate_data->GetScriptWrappableMarkingVisitor()->RegisterV8Reference(
-      std::make_pair(const_cast<WrapperTypeInfo*>(wrapper_type_info),
-                     wrappable));
+  per_isolate_data->GetV8HeapController()->RegisterV8References({std::make_pair(
+      const_cast<WrapperTypeInfo*>(wrapper_type_info), wrappable)});
 }
 
 inline void V8DOMWrapper::ClearNativeInfo(v8::Isolate* isolate,

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

@@ -153,9 +153,8 @@ void V8PerIsolateData::WillBeDestroyed(v8::Isolate* isolate) {
 
   // Detach V8's garbage collector.
   isolate->SetEmbedderHeapTracer(nullptr);
-  if (data->script_wrappable_visitor_->WrapperTracingInProgress())
-    data->script_wrappable_visitor_->AbortTracing();
-  data->script_wrappable_visitor_.reset();
+  data->v8_heap_controller_->FinalizeAndCleanup();
+  data->v8_heap_controller_.reset();
 }
 
 // destroy() clear things that should be cleared after ThreadState::detach()
@@ -368,16 +367,4 @@ void V8PerIsolateData::AddActiveScriptWrappable(
   active_script_wrappables_->insert(wrappable);
 }
 
-void V8PerIsolateData::TemporaryScriptWrappableVisitorScope::
-    SwapWithV8PerIsolateDataVisitor(
-        std::unique_ptr<ScriptWrappableMarkingVisitor>& visitor) {
-  ScriptWrappableMarkingVisitor* current = CurrentVisitor();
-  if (current)
-    ScriptWrappableMarkingVisitor::PerformCleanup(isolate_);
-
-  V8PerIsolateData::From(isolate_)->script_wrappable_visitor_.swap(
-      saved_visitor_);
-  isolate_->SetEmbedderHeapTracer(CurrentVisitor());
-}
-
 }  // namespace blink

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

@@ -33,9 +33,9 @@
 #include "gin/public/isolate_holder.h"
 #include "third_party/blink/renderer/platform/bindings/runtime_call_stats.h"
 #include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
-#include "third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.h"
 #include "third_party/blink/renderer/platform/bindings/v8_global_value_map.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/heap/v8_heap_controller.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
@@ -200,41 +200,19 @@ class PLATFORM_EXPORT V8PerIsolateData {
     return active_script_wrappables_.Get();
   }
 
-  class PLATFORM_EXPORT TemporaryScriptWrappableVisitorScope {
-    WTF_MAKE_NONCOPYABLE(TemporaryScriptWrappableVisitorScope);
-    STACK_ALLOCATED();
-
-   public:
-    TemporaryScriptWrappableVisitorScope(
-        v8::Isolate* isolate,
-        std::unique_ptr<ScriptWrappableMarkingVisitor> visitor)
-        : isolate_(isolate), saved_visitor_(std::move(visitor)) {
-      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
-    }
-    ~TemporaryScriptWrappableVisitorScope() {
-      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
-    }
-
-    inline ScriptWrappableMarkingVisitor* CurrentVisitor() {
-      return V8PerIsolateData::From(isolate_)
-          ->GetScriptWrappableMarkingVisitor();
-    }
-
-   private:
-    void SwapWithV8PerIsolateDataVisitor(
-        std::unique_ptr<ScriptWrappableMarkingVisitor>&);
-
-    v8::Isolate* isolate_;
-    std::unique_ptr<ScriptWrappableMarkingVisitor> saved_visitor_;
-  };
+  void SetV8HeapController(std::unique_ptr<V8HeapController> controller) {
+    DCHECK(!v8_heap_controller_);
+    v8_heap_controller_ = std::move(controller);
+  }
 
-  void SetScriptWrappableMarkingVisitor(
-      std::unique_ptr<ScriptWrappableMarkingVisitor> visitor) {
-    script_wrappable_visitor_ = std::move(visitor);
+  V8HeapController* GetV8HeapController() const {
+    return v8_heap_controller_.get();
   }
-  ScriptWrappableMarkingVisitor* GetScriptWrappableMarkingVisitor() {
-    return script_wrappable_visitor_.get();
+
+  void SwapV8HeapController(std::unique_ptr<V8HeapController>& other) {
+    v8_heap_controller_.swap(other);
   }
+
   int IsNearV8HeapLimitHandled() { return handled_near_v8_heap_limit_; }
 
   void HandledNearV8HeapLimit() { handled_near_v8_heap_limit_ = true; }
@@ -306,7 +284,8 @@ class PLATFORM_EXPORT V8PerIsolateData {
   std::unique_ptr<Data> thread_debugger_;
 
   Persistent<ActiveScriptWrappableSet> active_script_wrappables_;
-  std::unique_ptr<ScriptWrappableMarkingVisitor> script_wrappable_visitor_;
+
+  std::unique_ptr<V8HeapController> v8_heap_controller_;
 
   RuntimeCallStats runtime_call_stats_;
   bool handled_near_v8_heap_limit_;

third_party/blink/renderer/platform/heap/BUILD.gn

@@ -79,6 +79,7 @@ blink_platform_sources("heap") {
     "thread_state.h",
     "threading_traits.h",
     "trace_traits.h",
+    "v8_heap_controller.h",
     "visitor.h",
     "worklist.h",
   ]

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

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_V8_HEAP_CONTROLLER_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_V8_HEAP_CONTROLLER_H_
+
+#include "v8/include/v8.h"
+
+namespace blink {
+
+// Common interface for all V8 heap tracers used in Blink.
+class V8HeapController : public v8::EmbedderHeapTracer {
+ public:
+  ~V8HeapController() override = default;
+
+  virtual void FinalizeAndCleanup() = 0;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_V8_HEAP_CONTROLLER_H_