testing: OverrideEmbedderStackStateScope should only affect implicit GCs

The only user of OverrideEmbedderStackStateScope is Blink where it is
used to override stack state of top-level tasks. Adjust the behavior
here to allow using this scope broadly while still supporting explicit
garbage collection calls.

Bug: chromium:1300492
Change-Id: I78c418c5f08991bf6857147cd4a537246bfcc556
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3497744Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79420}

include/cppgc/testing.h

@@ -19,8 +19,13 @@ class HeapHandle;
 namespace testing {
 
 /**
- * Overrides the state of the stack with the provided value. Takes precedence
- * over other parameters that set the stack state. Must no be nested.
+ * Overrides the state of the stack with the provided value. Parameters passed
+ * to explicit garbage collection calls still take precedence. Must not be
+ * nested.
+ *
+ * This scope is useful to make the garbage collector consider the stack when
+ * tasks that invoke garbage collection (through the provided platform) contain
+ * interesting pointers on its stack.
  */
 class V8_EXPORT V8_NODISCARD OverrideEmbedderStackStateScope final {
   CPPGC_STACK_ALLOCATED();

src/api/api.cc

@@ -8531,10 +8531,11 @@ void Isolate::RequestGarbageCollectionForTesting(GarbageCollectionType type) {
 void Isolate::RequestGarbageCollectionForTesting(
     GarbageCollectionType type,
     EmbedderHeapTracer::EmbedderStackState stack_state) {
+  base::Optional<i::EmbedderStackStateScope> stack_scope;
   if (type == kFullGarbageCollection) {
-    reinterpret_cast<i::Isolate*>(this)
-        ->heap()
-        ->SetEmbedderStackStateForNextFinalization(stack_state);
+    stack_scope.emplace(reinterpret_cast<i::Isolate*>(this)->heap(),
+                        i::EmbedderStackStateScope::kExplicitInvocation,
+                        stack_state);
   }
   RequestGarbageCollectionForTesting(type);
 }

src/debug/debug-interface.cc

@@ -15,6 +15,7 @@
 #include "src/debug/debug-type-profile.h"
 #include "src/debug/debug.h"
 #include "src/execution/vm-state-inl.h"
+#include "src/heap/heap.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/profiler/heap-profiler.h"
 #include "src/strings/string-builder-inl.h"
@@ -613,8 +614,9 @@ Platform* GetCurrentPlatform() { return i::V8::GetCurrentPlatform(); }
 void ForceGarbageCollection(
     Isolate* isolate,
     EmbedderHeapTracer::EmbedderStackState embedder_stack_state) {
-  i::Heap* heap = reinterpret_cast<i::Isolate*>(isolate)->heap();
-  heap->SetEmbedderStackStateForNextFinalization(embedder_stack_state);
+  i::EmbedderStackStateScope stack_scope(
+      reinterpret_cast<i::Isolate*>(isolate)->heap(),
+      i::EmbedderStackStateScope::kImplicitThroughTask, embedder_stack_state);
   isolate->LowMemoryNotification();
 }
 

src/extensions/gc-extension.cc

@@ -9,6 +9,7 @@
 #include "include/v8-persistent-handle.h"
 #include "include/v8-primitive.h"
 #include "include/v8-template.h"
+#include "src/base/optional.h"
 #include "src/base/platform/platform.h"
 #include "src/execution/isolate.h"
 #include "src/heap/heap.h"
@@ -77,8 +78,8 @@ Maybe<GCOptions> Parse(v8::Isolate* isolate,
   return Just<GCOptions>(options);
 }
 
-void InvokeGC(v8::Isolate* isolate, v8::Isolate::GarbageCollectionType type,
-              v8::EmbedderHeapTracer::EmbedderStackState embedder_stack_state) {
+void InvokeGC(v8::Isolate* isolate, ExecutionType execution_type,
+              v8::Isolate::GarbageCollectionType type) {
   Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
   switch (type) {
     case v8::Isolate::GarbageCollectionType::kMinorGarbageCollection:
@@ -86,7 +87,15 @@ void InvokeGC(v8::Isolate* isolate, v8::Isolate::GarbageCollectionType type,
                            kGCCallbackFlagForced);
       break;
     case v8::Isolate::GarbageCollectionType::kFullGarbageCollection:
-      heap->SetEmbedderStackStateForNextFinalization(embedder_stack_state);
+      EmbedderStackStateScope stack_scope(
+          heap,
+          execution_type == ExecutionType::kAsync
+              ? EmbedderStackStateScope::kImplicitThroughTask
+              : EmbedderStackStateScope::kExplicitInvocation,
+          execution_type == ExecutionType::kAsync
+              ? v8::EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers
+              : v8::EmbedderHeapTracer::EmbedderStackState::
+                    kMayContainHeapPointers);
       heap->PreciseCollectAllGarbage(i::Heap::kNoGCFlags,
                                      i::GarbageCollectionReason::kTesting,
                                      kGCCallbackFlagForced);
@@ -110,8 +119,7 @@ class AsyncGC final : public CancelableTask {
 
   void RunInternal() final {
     v8::HandleScope scope(isolate_);
-    InvokeGC(isolate_, type_,
-             v8::EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+    InvokeGC(isolate_, ExecutionType::kAsync, type_);
     auto resolver = v8::Local<v8::Promise::Resolver>::New(isolate_, resolver_);
     auto ctx = Local<v8::Context>::New(isolate_, ctx_);
     resolver->Resolve(ctx, v8::Undefined(isolate_)).ToChecked();
@@ -136,9 +144,8 @@ void GCExtension::GC(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   // Immediate bailout if no arguments are provided.
   if (args.Length() == 0) {
-    InvokeGC(
-        isolate, v8::Isolate::GarbageCollectionType::kFullGarbageCollection,
-        v8::EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
+    InvokeGC(isolate, ExecutionType::kSync,
+             v8::Isolate::GarbageCollectionType::kFullGarbageCollection);
     return;
   }
 
@@ -147,9 +154,7 @@ void GCExtension::GC(const v8::FunctionCallbackInfo<v8::Value>& args) {
   GCOptions options = maybe_options.ToChecked();
   switch (options.execution) {
     case ExecutionType::kSync:
-      InvokeGC(
-          isolate, options.type,
-          v8::EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
+      InvokeGC(isolate, ExecutionType::kSync, options.type);
       break;
     case ExecutionType::kAsync: {
       v8::HandleScope scope(isolate);

src/heap/cppgc-js/cpp-heap.cc

@@ -578,9 +578,6 @@ bool CppHeap::IsTracingDone() { return marking_done_; }
 void CppHeap::EnterFinalPause(cppgc::EmbedderStackState stack_state) {
   CHECK(!in_disallow_gc_scope());
   in_atomic_pause_ = true;
-  if (override_stack_state_) {
-    stack_state = *override_stack_state_;
-  }
   marker_->EnterAtomicPause(stack_state);
   if (isolate_ &&
       *collection_type_ ==

src/heap/cppgc/garbage-collector.h

@@ -5,6 +5,7 @@
 #ifndef V8_HEAP_CPPGC_GARBAGE_COLLECTOR_H_
 #define V8_HEAP_CPPGC_GARBAGE_COLLECTOR_H_
 
+#include "include/cppgc/common.h"
 #include "src/heap/cppgc/marker.h"
 #include "src/heap/cppgc/sweeper.h"
 
@@ -75,6 +76,9 @@ class GarbageCollector {
   // The current epoch that the GC maintains. The epoch is increased on every
   // GC invocation.
   virtual size_t epoch() const = 0;
+
+  // Returns a non-null state if the stack state if overriden.
+  virtual const EmbedderStackState* override_stack_state() const = 0;
 };
 
 }  // namespace internal

src/heap/cppgc/gc-invoker.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "include/cppgc/common.h"
 #include "include/cppgc/platform.h"
 #include "src/heap/cppgc/heap.h"
 #include "src/heap/cppgc/task-handle.h"
@@ -24,6 +25,9 @@ class GCInvoker::GCInvokerImpl final : public GarbageCollector {
   void CollectGarbage(GarbageCollector::Config) final;
   void StartIncrementalGarbageCollection(GarbageCollector::Config) final;
   size_t epoch() const final { return collector_->epoch(); }
+  const EmbedderStackState* override_stack_state() const final {
+    return collector_->override_stack_state();
+  }
 
  private:
   class GCTask final : public cppgc::Task {
@@ -48,6 +52,8 @@ class GCInvoker::GCInvokerImpl final : public GarbageCollector {
 
    private:
     void Run() final {
+      CHECK_NULL(collector_->override_stack_state());
+
       if (handle_.IsCanceled() || (collector_->epoch() != saved_epoch_)) return;
 
       collector_->CollectGarbage(config_);
@@ -94,6 +100,8 @@ void GCInvoker::GCInvokerImpl::CollectGarbage(GarbageCollector::Config config) {
       // Force a precise GC since it will run in a non-nestable task.
       config.stack_state =
           GarbageCollector::Config::StackState::kNoHeapPointers;
+      DCHECK_NE(cppgc::Heap::StackSupport::kSupportsConservativeStackScan,
+                stack_support_);
       gc_task_handle_ = GCTask::Post(
           collector_, platform_->GetForegroundTaskRunner().get(), config);
     }
@@ -137,5 +145,9 @@ void GCInvoker::StartIncrementalGarbageCollection(
 
 size_t GCInvoker::epoch() const { return impl_->epoch(); }
 
+const EmbedderStackState* GCInvoker::override_stack_state() const {
+  return impl_->override_stack_state();
+}
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/gc-invoker.h

@@ -5,6 +5,7 @@
 #ifndef V8_HEAP_CPPGC_GC_INVOKER_H_
 #define V8_HEAP_CPPGC_GC_INVOKER_H_
 
+#include "include/cppgc/common.h"
 #include "include/cppgc/heap.h"
 #include "src/base/macros.h"
 #include "src/heap/cppgc/garbage-collector.h"
@@ -36,6 +37,7 @@ class V8_EXPORT_PRIVATE GCInvoker final : public GarbageCollector {
   void CollectGarbage(GarbageCollector::Config) final;
   void StartIncrementalGarbageCollection(GarbageCollector::Config) final;
   size_t epoch() const final;
+  const EmbedderStackState* override_stack_state() const final;
 
  private:
   class GCInvokerImpl;

src/heap/cppgc/heap.cc

@@ -168,9 +168,6 @@ void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
   DCHECK(!in_no_gc_scope());
   CHECK(!in_disallow_gc_scope());
   config_.stack_state = stack_state;
-  if (override_stack_state_) {
-    config_.stack_state = *override_stack_state_;
-  }
   SetStackEndOfCurrentGC(v8::base::Stack::GetCurrentStackPosition());
   in_atomic_pause_ = true;
   {

src/heap/cppgc/heap.h

@@ -37,6 +37,9 @@ class V8_EXPORT_PRIVATE Heap final : public HeapBase,
   void FinalizeIncrementalGarbageCollectionIfRunning(Config);
 
   size_t epoch() const final { return epoch_; }
+  const EmbedderStackState* override_stack_state() const final {
+    return HeapBase::override_stack_state();
+  }
 
   void DisableHeapGrowingForTesting();
 

src/heap/embedder-tracing.cc

@@ -113,15 +113,6 @@ bool LocalEmbedderHeapTracer::IsRemoteTracingDone() {
                                 : remote_tracer_->IsTracingDone());
 }
 
-void LocalEmbedderHeapTracer::SetEmbedderStackStateForNextFinalization(
-    EmbedderHeapTracer::EmbedderStackState stack_state) {
-  if (!InUse()) return;
-
-  embedder_stack_state_ = stack_state;
-  if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
-    NotifyEmptyEmbedderStack();
-}
-
 LocalEmbedderHeapTracer::ProcessingScope::ProcessingScope(
     LocalEmbedderHeapTracer* tracer)
     : tracer_(tracer), wrapper_descriptor_(tracer->wrapper_descriptor_) {

src/heap/embedder-tracing.h

@@ -117,9 +117,6 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
            (IsRemoteTracingDone() && embedder_worklist_empty_);
   }
 
-  void SetEmbedderStackStateForNextFinalization(
-      EmbedderHeapTracer::EmbedderStackState stack_state);
-
   void SetEmbedderWorklistEmpty(bool is_empty) {
     embedder_worklist_empty_ = is_empty;
   }
@@ -230,26 +227,6 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
   friend class EmbedderStackStateScope;
 };
 
-class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
- public:
-  EmbedderStackStateScope(LocalEmbedderHeapTracer* local_tracer,
-                          EmbedderHeapTracer::EmbedderStackState stack_state)
-      : local_tracer_(local_tracer),
-        old_stack_state_(local_tracer_->embedder_stack_state_) {
-    local_tracer_->embedder_stack_state_ = stack_state;
-    if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
-      local_tracer_->NotifyEmptyEmbedderStack();
-  }
-
-  ~EmbedderStackStateScope() {
-    local_tracer_->embedder_stack_state_ = old_stack_state_;
-  }
-
- private:
-  LocalEmbedderHeapTracer* const local_tracer_;
-  const EmbedderHeapTracer::EmbedderStackState old_stack_state_;
-};
-
 }  // namespace internal
 }  // namespace v8
 

src/heap/heap.cc

@@ -1789,7 +1789,7 @@ bool Heap::CollectGarbage(AllocationSpace space,
     // Temporary override any embedder stack state as callbacks may create
     // their own state on the stack and recursively trigger GC.
     EmbedderStackStateScope embedder_scope(
-        local_embedder_heap_tracer(),
+        this, EmbedderStackStateScope::kExplicitInvocation,
         EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
     if (scope.CheckReenter()) {
       AllowGarbageCollection allow_gc;
@@ -7376,12 +7376,6 @@ bool Heap::PageFlagsAreConsistent(HeapObject object) {
   return true;
 }
 
-void Heap::SetEmbedderStackStateForNextFinalization(
-    EmbedderHeapTracer::EmbedderStackState stack_state) {
-  local_embedder_heap_tracer()->SetEmbedderStackStateForNextFinalization(
-      stack_state);
-}
-
 #ifdef DEBUG
 void Heap::IncrementObjectCounters() {
   isolate_->counters()->objs_since_last_full()->Increment();
@@ -7435,5 +7429,40 @@ void Heap::set_allocation_timeout(int allocation_timeout) {
 }
 #endif  // V8_ENABLE_ALLOCATION_TIMEOUT
 
+EmbedderStackStateScope::EmbedderStackStateScope(
+    Heap* heap, Origin origin,
+    EmbedderHeapTracer::EmbedderStackState stack_state)
+    : local_tracer_(heap->local_embedder_heap_tracer()),
+      old_stack_state_(local_tracer_->embedder_stack_state_) {
+  if (origin == kImplicitThroughTask && heap->overriden_stack_state()) {
+    stack_state = *heap->overriden_stack_state();
+  }
+
+  local_tracer_->embedder_stack_state_ = stack_state;
+  if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
+    local_tracer_->NotifyEmptyEmbedderStack();
+}
+
+// static
+EmbedderStackStateScope EmbedderStackStateScope::ExplicitScopeForTesting(
+    LocalEmbedderHeapTracer* local_tracer,
+    EmbedderHeapTracer::EmbedderStackState stack_state) {
+  return EmbedderStackStateScope(local_tracer, stack_state);
+}
+
+EmbedderStackStateScope::EmbedderStackStateScope(
+    LocalEmbedderHeapTracer* local_tracer,
+    EmbedderHeapTracer::EmbedderStackState stack_state)
+    : local_tracer_(local_tracer),
+      old_stack_state_(local_tracer_->embedder_stack_state_) {
+  local_tracer_->embedder_stack_state_ = stack_state;
+  if (EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers == stack_state)
+    local_tracer_->NotifyEmptyEmbedderStack();
+}
+
+EmbedderStackStateScope::~EmbedderStackStateScope() {
+  local_tracer_->embedder_stack_state_ = old_stack_state_;
+}
+
 }  // namespace internal
 }  // namespace v8

src/heap/heap.h

@@ -1149,8 +1149,6 @@ class Heap {
   EmbedderHeapTracer* GetEmbedderHeapTracer() const;
 
   void RegisterExternallyReferencedObject(Address* location);
-  V8_EXPORT_PRIVATE void SetEmbedderStackStateForNextFinalization(
-      EmbedderHeapTracer::EmbedderStackState stack_state);
 
   EmbedderHeapTracer::TraceFlags flags_for_embedder_tracer() const;
 
@@ -2774,6 +2772,30 @@ struct StrongRootBlockAllocator::rebind {
   };
 };
 
+class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
+ public:
+  enum Origin {
+    kImplicitThroughTask,
+    kExplicitInvocation,
+  };
+
+  // Only used for testing where the Origin is always an explicit invocation.
+  static EmbedderStackStateScope ExplicitScopeForTesting(
+      LocalEmbedderHeapTracer* local_tracer,
+      EmbedderHeapTracer::EmbedderStackState stack_state);
+
+  EmbedderStackStateScope(Heap* heap, Origin origin,
+                          EmbedderHeapTracer::EmbedderStackState stack_state);
+  ~EmbedderStackStateScope();
+
+ private:
+  EmbedderStackStateScope(LocalEmbedderHeapTracer* local_tracer,
+                          EmbedderHeapTracer::EmbedderStackState stack_state);
+
+  LocalEmbedderHeapTracer* const local_tracer_;
+  const EmbedderHeapTracer::EmbedderStackState old_stack_state_;
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/heap/incremental-marking-job.cc

@@ -56,6 +56,7 @@ void IncrementalMarkingJob::ScheduleTask(Heap* heap, TaskType task_type) {
     SetTaskPending(task_type, true);
     auto taskrunner =
         V8::GetCurrentPlatform()->GetForegroundTaskRunner(isolate);
+
     const EmbedderHeapTracer::EmbedderStackState stack_state =
         taskrunner->NonNestableTasksEnabled()
             ? EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers
@@ -97,8 +98,8 @@ void IncrementalMarkingJob::Task::RunInternal() {
   TRACE_EVENT_CALL_STATS_SCOPED(isolate(), "v8", "V8.Task");
 
   Heap* heap = isolate()->heap();
-  EmbedderStackStateScope scope(heap->local_embedder_heap_tracer(),
-                                stack_state_);
+  EmbedderStackStateScope scope(
+      heap, EmbedderStackStateScope::kImplicitThroughTask, stack_state_);
   if (task_type_ == TaskType::kNormal) {
     heap->tracer()->RecordTimeToIncrementalMarkingTask(
         heap->MonotonicallyIncreasingTimeInMs() - job_->scheduled_time_);

test/unittests/heap/cppgc-js/unified-heap-unittest.cc

@@ -165,7 +165,8 @@ TEST_F(UnifiedHeapDetachedTest, AllocationBeforeConfigureHeap) {
   }
   USE(object);
   {
-    js_heap.SetEmbedderStackStateForNextFinalization(
+    EmbedderStackStateScope stack_scope(
+        &js_heap, EmbedderStackStateScope::kExplicitInvocation,
         EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
     CollectGarbage(OLD_SPACE);
     cpp_heap.AsBase().sweeper().FinishIfRunning();

test/unittests/heap/cppgc-js/unified-heap-utils.cc

@@ -29,7 +29,8 @@ UnifiedHeapTest::UnifiedHeapTest(
 
 void UnifiedHeapTest::CollectGarbageWithEmbedderStack(
     cppgc::Heap::SweepingType sweeping_type) {
-  heap()->SetEmbedderStackStateForNextFinalization(
+  EmbedderStackStateScope stack_scope(
+      heap(), EmbedderStackStateScope::kExplicitInvocation,
       EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
   CollectGarbage(OLD_SPACE);
   if (sweeping_type == cppgc::Heap::SweepingType::kAtomic) {
@@ -39,7 +40,8 @@ void UnifiedHeapTest::CollectGarbageWithEmbedderStack(
 
 void UnifiedHeapTest::CollectGarbageWithoutEmbedderStack(
     cppgc::Heap::SweepingType sweeping_type) {
-  heap()->SetEmbedderStackStateForNextFinalization(
+  EmbedderStackStateScope stack_scope(
+      heap(), EmbedderStackStateScope::kExplicitInvocation,
       EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
   CollectGarbage(OLD_SPACE);
   if (sweeping_type == cppgc::Heap::SweepingType::kAtomic) {

test/unittests/heap/cppgc/gc-invoker-unittest.cc

@@ -22,6 +22,8 @@ class MockGarbageCollector : public GarbageCollector {
   MOCK_METHOD(void, StartIncrementalGarbageCollection,
               (GarbageCollector::Config), (override));
   MOCK_METHOD(size_t, epoch, (), (const, override));
+  MOCK_METHOD(const EmbedderStackState*, override_stack_state, (),
+              (const, override));
 };
 
 class MockTaskRunner : public cppgc::TaskRunner {

test/unittests/heap/cppgc/heap-growing-unittest.cc

@@ -37,6 +37,9 @@ class FakeGarbageCollector : public GarbageCollector {
   }
 
   size_t epoch() const override { return callcount_; }
+  const EmbedderStackState* override_stack_state() const override {
+    return nullptr;
+  }
 
  private:
   StatsCollector* stats_collector_;
@@ -50,6 +53,8 @@ class MockGarbageCollector : public GarbageCollector {
   MOCK_METHOD(void, StartIncrementalGarbageCollection,
               (GarbageCollector::Config), (override));
   MOCK_METHOD(size_t, epoch, (), (const, override));
+  MOCK_METHOD(const EmbedderStackState*, override_stack_state, (),
+              (const, override));
 };
 
 void FakeAllocate(StatsCollector* stats_collector, size_t bytes) {

test/unittests/heap/cppgc/testing-unittest.cc

@@ -22,7 +22,8 @@ class GCed : public GarbageCollected<GCed> {
 };
 }  // namespace
 
-TEST_F(TestingTest, OverrideEmbeddertackStateScope) {
+TEST_F(TestingTest,
+       OverrideEmbeddertackStateScopeDoesNotOverrideExplicitCalls) {
   {
     auto* gced = MakeGarbageCollected<GCed>(GetHeap()->GetAllocationHandle());
     WeakPersistent<GCed> weak{gced};
@@ -38,7 +39,7 @@ TEST_F(TestingTest, OverrideEmbeddertackStateScope) {
         EmbedderStackState::kMayContainHeapPointers);
     internal::Heap::From(GetHeap())->CollectGarbage(
         Heap::Config::PreciseAtomicConfig());
-    EXPECT_TRUE(weak);
+    EXPECT_FALSE(weak);
   }
   {
     auto* gced = MakeGarbageCollected<GCed>(GetHeap()->GetAllocationHandle());
@@ -47,7 +48,7 @@ TEST_F(TestingTest, OverrideEmbeddertackStateScope) {
         GetHeap()->GetHeapHandle(), EmbedderStackState::kNoHeapPointers);
     internal::Heap::From(GetHeap())->CollectGarbage(
         Heap::Config::ConservativeAtomicConfig());
-    EXPECT_FALSE(weak);
+    EXPECT_TRUE(weak);
   }
 }
 

test/unittests/heap/embedder-tracing-unittest.cc

@@ -120,8 +120,10 @@ TEST_F(LocalEmbedderHeapTracerWithIsolate,
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
   LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
-  local_tracer.SetEmbedderStackStateForNextFinalization(
-      EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+  EmbedderStackStateScope scope =
+      EmbedderStackStateScope::ExplicitScopeForTesting(
+          &local_tracer,
+          EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
   EXPECT_CALL(
       remote_tracer,
       EnterFinalPause(EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers));
@@ -134,8 +136,10 @@ TEST_F(LocalEmbedderHeapTracerWithIsolate, TemporaryEmbedderStackState) {
   local_tracer.SetRemoteTracer(&remote_tracer);
   // Default is unknown, see above.
   {
-    EmbedderStackStateScope scope(
-        &local_tracer, EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+    EmbedderStackStateScope scope =
+        EmbedderStackStateScope::ExplicitScopeForTesting(
+            &local_tracer,
+            EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
     EXPECT_CALL(remote_tracer,
                 EnterFinalPause(
                     EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers));
@@ -150,12 +154,15 @@ TEST_F(LocalEmbedderHeapTracerWithIsolate,
   local_tracer.SetRemoteTracer(&remote_tracer);
   // Default is unknown, see above.
   {
-    EmbedderStackStateScope scope(
-        &local_tracer, EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+    EmbedderStackStateScope scope =
+        EmbedderStackStateScope::ExplicitScopeForTesting(
+            &local_tracer,
+            EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
     {
-      EmbedderStackStateScope nested_scope(
-          &local_tracer,
-          EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
+      EmbedderStackStateScope nested_scope =
+          EmbedderStackStateScope::ExplicitScopeForTesting(
+              &local_tracer,
+              EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
       EXPECT_CALL(
           remote_tracer,
           EnterFinalPause(
@@ -173,8 +180,10 @@ TEST_F(LocalEmbedderHeapTracerWithIsolate, TraceEpilogueStackStateResets) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
   LocalEmbedderHeapTracer local_tracer(isolate());
   local_tracer.SetRemoteTracer(&remote_tracer);
-  local_tracer.SetEmbedderStackStateForNextFinalization(
-      EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+  EmbedderStackStateScope scope =
+      EmbedderStackStateScope::ExplicitScopeForTesting(
+          &local_tracer,
+          EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
   EXPECT_CALL(
       remote_tracer,
       EnterFinalPause(EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers));