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

This reverts commit 1186c664.

Reason for revert: Temporary reverting to investigate micro benchmark regression.

No-try: true
Bug: chromium:872581

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

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:843903
Change-Id: I75383f8b0fd825fdaf04202df3cc6a2ac312bb9b
Reviewed-on: https://chromium-review.googlesource.com/1170743
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582141}

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

@@ -65,7 +65,6 @@
 #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"
@@ -623,11 +622,12 @@ static void HostGetImportMetaProperties(v8::Local<v8::Context> context,
 static void InitializeV8Common(v8::Isolate* isolate) {
   isolate->AddGCPrologueCallback(V8GCController::GcPrologue);
   isolate->AddGCEpilogueCallback(V8GCController::GcEpilogue);
-  V8PerIsolateData::From(isolate)->SetV8HeapController(
-      std::unique_ptr<V8HeapController>{
-          new ScriptWrappableMarkingVisitor(isolate)});
+  std::unique_ptr<ScriptWrappableMarkingVisitor> visitor(
+      new ScriptWrappableMarkingVisitor(isolate));
+  V8PerIsolateData::From(isolate)->SetScriptWrappableMarkingVisitor(
+      std::move(visitor));
   isolate->SetEmbedderHeapTracer(
-      V8PerIsolateData::From(isolate)->GetV8HeapController());
+      V8PerIsolateData::From(isolate)->GetScriptWrappableMarkingVisitor());
 
   isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kScoped);
 

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

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

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

@@ -9,7 +9,6 @@
 #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"
@@ -30,8 +29,8 @@ class TraceWrapperV8Reference;
 // reachable wrappers. V8 calls this visitor during its garbage collection,
 // see v8::EmbedderHeapTracer.
 class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
-    : public ScriptWrappableVisitor,
-      public V8HeapController {
+    : public v8::EmbedderHeapTracer,
+      public ScriptWrappableVisitor {
   DISALLOW_IMPLICIT_CONSTRUCTORS(ScriptWrappableMarkingVisitor);
 
  public:
@@ -70,6 +69,7 @@ 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,10 +81,8 @@ class PLATFORM_EXPORT ScriptWrappableMarkingVisitor
   void EnterFinalPause() override;
   size_t NumberOfWrappersToTrace() override;
 
-  // 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,12 +114,13 @@ inline void V8DOMWrapper::SetNativeInfoInternal(
   wrapper->SetAlignedPointerInInternalFields(base::size(indices), indices,
                                              values);
   auto* per_isolate_data = V8PerIsolateData::From(isolate);
-  // We notify V8HeapController about the new wrapper association,
-  // so the controller can make sure to trace the association (in case it is
+  // We notify ScriptWrappableVisitor about the new wrapper association,
+  // so the visitor 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->GetV8HeapController()->RegisterV8References({std::make_pair(
-      const_cast<WrapperTypeInfo*>(wrapper_type_info), wrappable)});
+  per_isolate_data->GetScriptWrappableMarkingVisitor()->RegisterV8Reference(
+      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,8 +153,9 @@ void V8PerIsolateData::WillBeDestroyed(v8::Isolate* isolate) {
 
   // Detach V8's garbage collector.
   isolate->SetEmbedderHeapTracer(nullptr);
-  data->v8_heap_controller_->FinalizeAndCleanup();
-  data->v8_heap_controller_.reset();
+  if (data->script_wrappable_visitor_->WrapperTracingInProgress())
+    data->script_wrappable_visitor_->AbortTracing();
+  data->script_wrappable_visitor_.reset();
 }
 
 // destroy() clear things that should be cleared after ThreadState::detach()
@@ -367,4 +368,16 @@ 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,19 +200,41 @@ class PLATFORM_EXPORT V8PerIsolateData {
     return active_script_wrappables_.Get();
   }
 
-  void SetV8HeapController(std::unique_ptr<V8HeapController> controller) {
-    DCHECK(!v8_heap_controller_);
-    v8_heap_controller_ = std::move(controller);
-  }
+  class PLATFORM_EXPORT TemporaryScriptWrappableVisitorScope {
+    WTF_MAKE_NONCOPYABLE(TemporaryScriptWrappableVisitorScope);
+    STACK_ALLOCATED();
 
-  V8HeapController* GetV8HeapController() const {
-    return v8_heap_controller_.get();
-  }
+   public:
+    TemporaryScriptWrappableVisitorScope(
+        v8::Isolate* isolate,
+        std::unique_ptr<ScriptWrappableMarkingVisitor> visitor)
+        : isolate_(isolate), saved_visitor_(std::move(visitor)) {
+      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
+    }
+    ~TemporaryScriptWrappableVisitorScope() {
+      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
+    }
 
-  void SwapV8HeapController(std::unique_ptr<V8HeapController>& other) {
-    v8_heap_controller_.swap(other);
-  }
+    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 SetScriptWrappableMarkingVisitor(
+      std::unique_ptr<ScriptWrappableMarkingVisitor> visitor) {
+    script_wrappable_visitor_ = std::move(visitor);
+  }
+  ScriptWrappableMarkingVisitor* GetScriptWrappableMarkingVisitor() {
+    return script_wrappable_visitor_.get();
+  }
   int IsNearV8HeapLimitHandled() { return handled_near_v8_heap_limit_; }
 
   void HandledNearV8HeapLimit() { handled_near_v8_heap_limit_ = true; }
@@ -284,8 +306,7 @@ class PLATFORM_EXPORT V8PerIsolateData {
   std::unique_ptr<Data> thread_debugger_;
 
   Persistent<ActiveScriptWrappableSet> active_script_wrappables_;
-
-  std::unique_ptr<V8HeapController> v8_heap_controller_;
+  std::unique_ptr<ScriptWrappableMarkingVisitor> script_wrappable_visitor_;
 
   RuntimeCallStats runtime_call_stats_;
   bool handled_near_v8_heap_limit_;

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

@@ -79,7 +79,6 @@ 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_