[heap] Merge mechanisms for disabling CSS

EmbedderStackStateScope is used to disable conservative stack scanning
for cppgc when the stack is known to not contain heap pointers. Also,
DisableConservativeStackScanningScopeForTesting is used to disable CSS
for the V8 heap in tests that assume a precise GC. Until now, these two
have used two different mechanisms for disabling CSS. This CL merges
the two mechanisms and implements the latter scope via the former.

This is a reland of commit f51e0bb1db
reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/4111954

Bug: v8:13257
Change-Id: Ia124a4201686e0ea79f9cd07bc3888b9781cafa9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4128141
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85066}

src/heap/heap.cc

@@ -4869,11 +4869,10 @@ void Heap::IterateStackRoots(RootVisitor* v, StackState stack_state) {
   isolate_->Iterate(v);
 
 #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  if (stack_state == StackState::kMayContainHeapPointers &&
-      !disable_conservative_stack_scanning_for_testing_) {
-    ConservativeStackVisitor stack_visitor(isolate(), v);
-    stack().IteratePointers(&stack_visitor);
-  }
+  if (stack_state == StackState::kNoHeapPointers || !IsGCWithStack()) return;
+
+  ConservativeStackVisitor stack_visitor(isolate_, v);
+  stack().IteratePointers(&stack_visitor);
 #endif  // V8_ENABLE_CONSERVATIVE_STACK_SCANNING
 }
 

src/heap/heap.h

@@ -2393,7 +2393,6 @@ class Heap {
   bool force_oom_ = false;
   bool force_gc_on_next_allocation_ = false;
   bool delay_sweeper_tasks_for_testing_ = false;
-  bool disable_conservative_stack_scanning_for_testing_ = false;
 
   UnorderedHeapObjectMap<HeapObject> retainer_;
   UnorderedHeapObjectMap<Root> retaining_root_;
@@ -2670,23 +2669,6 @@ class V8_EXPORT_PRIVATE V8_NODISCARD SaveStackContextScope {
   ::heap::base::Stack* stack_;
 };
 
-class V8_NODISCARD DisableConservativeStackScanningScopeForTesting {
- public:
-  explicit inline DisableConservativeStackScanningScopeForTesting(Heap* heap)
-      : heap_(heap),
-        old_value_(heap_->disable_conservative_stack_scanning_for_testing_) {
-    heap_->disable_conservative_stack_scanning_for_testing_ = true;
-  }
-
-  inline ~DisableConservativeStackScanningScopeForTesting() {
-    heap_->disable_conservative_stack_scanning_for_testing_ = old_value_;
-  }
-
- protected:
-  Heap* heap_;
-  bool old_value_;
-};
-
 // Space iterator for iterating over all the paged spaces of the heap: Map
 // space, old space and code space. Returns each space in turn, and null when it
 // is done.
@@ -2842,6 +2824,17 @@ class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
   const StackState old_stack_state_;
 };
 
+class V8_NODISCARD DisableConservativeStackScanningScopeForTesting {
+ public:
+  explicit inline DisableConservativeStackScanningScopeForTesting(Heap* heap)
+      : embedder_scope_(EmbedderStackStateScope::ExplicitScopeForTesting(
+            heap->local_embedder_heap_tracer(),
+            cppgc::EmbedderStackState::kNoHeapPointers)) {}
+
+ private:
+  EmbedderStackStateScope embedder_scope_;
+};
+
 class V8_NODISCARD CppClassNamesAsHeapObjectNameScope final {
  public:
   explicit CppClassNamesAsHeapObjectNameScope(v8::CppHeap* heap);

src/heap/mark-compact.cc

@@ -2115,8 +2115,7 @@ void MarkCompactCollector::MarkRoots(RootVisitor* root_visitor) {
     //
     // TODO(v8:v8:13207): Remove as this is not required when using `CppHeap`.
     auto& stack = heap()->stack();
-    if (heap_->local_embedder_heap_tracer()->embedder_stack_state() ==
-        cppgc::EmbedderStackState::kMayContainHeapPointers) {
+    if (heap_->IsGCWithStack()) {
       ConservativeTracedHandlesMarkingVisitor conservative_marker(
           *heap_, *local_marking_worklists_,
           cppgc::internal::CollectionType::kMajor);

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

@@ -517,12 +517,6 @@ V8_NOINLINE void StackToHeapTest(v8::Isolate* v8_isolate, Operation op,
     // Disable scanning, assuming the slots are overwritten.
     DisableConservativeStackScanningScopeForTesting no_stack_scanning(
         reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
-    EmbedderStackStateScope scope =
-        EmbedderStackStateScope::ExplicitScopeForTesting(
-            reinterpret_cast<i::Isolate*>(v8_isolate)
-                ->heap()
-                ->local_embedder_heap_tracer(),
-            cppgc::EmbedderStackState::kNoHeapPointers);
     FullGC(v8_isolate);
   }
   ASSERT_TRUE(observer.IsEmpty());
@@ -565,7 +559,13 @@ V8_NOINLINE void HeapToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle.Reset();
-  FullGC(v8_isolate);
+  {
+    // Conservative scanning may find stale pointers to on-stack handles.
+    // Disable scanning, assuming the slots are overwritten.
+    DisableConservativeStackScanningScopeForTesting no_stack_scanning(
+        reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
+    FullGC(v8_isolate);
+  }
   EXPECT_TRUE(observer.IsEmpty());
 }
 
@@ -603,7 +603,13 @@ V8_NOINLINE void StackToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle2.Reset();
-  FullGC(v8_isolate);
+  {
+    // Conservative scanning may find stale pointers to on-stack handles.
+    // Disable scanning, assuming the slots are overwritten.
+    DisableConservativeStackScanningScopeForTesting no_stack_scanning(
+        reinterpret_cast<i::Isolate*>(v8_isolate)->heap());
+    FullGC(v8_isolate);
+  }
   EXPECT_TRUE(observer.IsEmpty());
 }