heap,bindings: Fix detaching from Isolate for the Oilpan library

The Oilpan library version does not require detaching manually from an
Isolate as the heap is tied directly to V8's JS heap for its lifetime.

Bug: 1056170
Change-Id: I61e7c9d017a523aaf4ad09430fc1e8363e2b2fc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629307
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843564}

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

@@ -92,6 +92,10 @@ void V8GCController::GcPrologue(v8::Isolate* isolate,
       IsNestedInV8GC(ThreadState::Current(), type)
           ? ThreadState::Current()->Heap().stats_collector()
           : nullptr);
+
+  auto* per_isolate_data = V8PerIsolateData::From(isolate);
+  per_isolate_data->EnterGC();
+
   ScriptForbiddenScope::Enter();
 
   // Attribute garbage collection to the all frames instead of a specific
@@ -105,14 +109,12 @@ void V8GCController::GcPrologue(v8::Isolate* isolate,
     case v8::kGCTypeIncrementalMarking:
       // Recomputing ASWs is opportunistic during incremental marking as they
       // only need to be recomputing during the atomic pause for corectness.
-      V8PerIsolateData::From(isolate)
-          ->GetActiveScriptWrappableManager()
+      per_isolate_data->GetActiveScriptWrappableManager()
           ->RecomputeActiveScriptWrappables(
               ActiveScriptWrappableManager::RecomputeMode::kOpportunistic);
       break;
     case v8::kGCTypeMarkSweepCompact:
-      V8PerIsolateData::From(isolate)
-          ->GetActiveScriptWrappableManager()
+      per_isolate_data->GetActiveScriptWrappableManager()
           ->RecomputeActiveScriptWrappables(
               ActiveScriptWrappableManager::RecomputeMode::kRequired);
       break;
@@ -129,6 +131,9 @@ void V8GCController::GcEpilogue(v8::Isolate* isolate,
       IsNestedInV8GC(ThreadState::Current(), type)
           ? ThreadState::Current()->Heap().stats_collector()
           : nullptr);
+
+  V8PerIsolateData::From(isolate)->LeaveGC();
+
   ScriptForbiddenScope::Exit();
 
   if (BlameContext* blame_context =

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

@@ -159,16 +159,12 @@ void V8PerIsolateData::WillBeDestroyed(v8::Isolate* isolate) {
 
   data->ClearScriptRegexpContext();
 
-  // Detach V8's garbage collector.
-  // Need to finalize an already running garbage collection as otherwise
-  // callbacks are missing and state gets out of sync.
-  ThreadState* const thread_state = ThreadState::Current();
-  thread_state->FinishIncrementalMarkingIfRunning(
-      BlinkGC::CollectionType::kMajor, BlinkGC::kHeapPointersOnStack,
-      BlinkGC::kAtomicMarking, BlinkGC::kEagerSweeping,
-      BlinkGC::GCReason::kThreadTerminationGC);
+  ThreadState::Current()->DetachFromIsolate();
+
   data->active_script_wrappable_manager_.Clear();
-  thread_state->DetachFromIsolate();
+  // Callbacks can be removed as they only cover single events (e.g. atomic
+  // pause) and they cannot get out of sync.
+  DCHECK_EQ(0u, data->gc_callback_depth_);
   isolate->RemoveGCPrologueCallback(data->prologue_callback_);
   isolate->RemoveGCEpilogueCallback(data->epilogue_callback_);
 }

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

@@ -207,6 +207,10 @@ class PLATFORM_EXPORT V8PerIsolateData final {
                       v8::Isolate::GCCallback prologue_callback,
                       v8::Isolate::GCCallback epilogue_callback);
 
+  void EnterGC() { gc_callback_depth_++; }
+
+  void LeaveGC() { gc_callback_depth_--; }
+
  private:
   V8PerIsolateData(scoped_refptr<base::SingleThreadTaskRunner>,
                    V8ContextSnapshotMode);
@@ -268,6 +272,7 @@ class PLATFORM_EXPORT V8PerIsolateData final {
 
   v8::Isolate::GCCallback prologue_callback_;
   v8::Isolate::GCCallback epilogue_callback_;
+  size_t gc_callback_depth_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(V8PerIsolateData);
 };

third_party/blink/renderer/platform/heap/impl/thread_state.cc

@@ -233,6 +233,10 @@ void ThreadState::AttachToIsolate(
 }
 
 void ThreadState::DetachFromIsolate() {
+  FinishIncrementalMarkingIfRunning(
+      BlinkGC::CollectionType::kMajor, BlinkGC::kHeapPointersOnStack,
+      BlinkGC::kAtomicMarking, BlinkGC::kEagerSweeping,
+      BlinkGC::GCReason::kThreadTerminationGC);
   if (isolate_) {
     isolate_->SetEmbedderHeapTracer(nullptr);
     if (v8::HeapProfiler* profiler = isolate_->GetHeapProfiler()) {

third_party/blink/renderer/platform/heap/v8_wrapper/thread_state.h

@@ -86,6 +86,11 @@ class ThreadState final {
     // TODO(1056170): Implement.
   }
 
+  void DetachFromIsolate() {
+    // No-op for the library implementation.
+    // TODO(1056170): Remove when removing Oilpan from Blink.
+  }
+
  private:
   // Main-thread ThreadState avoids TLS completely by using a regular global.
   // The object is manually managed and should not rely on global ctor/dtor.