Revert "[heap] Merge mechanisms for disabling CSS"

This reverts commit f51e0bb1db.

Reason for revert: Dependent on crrev.com/c/4092737 that is being reverted.

Original change's description:
> [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.
>
> Bug: v8:13257
> Change-Id: Ieca082657854fe2eff9eb5d95a30d48bb8eab44f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4111954
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#84964}

Bug: v8:13257
Change-Id: Id769af6215a2ed319ec96b354734a5362b2384cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4111179
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84972}

src/heap/heap.cc

@@ -4872,16 +4872,16 @@ void Heap::IterateStackRoots(RootVisitor* v, StackState stack_state,
   isolate()->Iterate(v);
 
 #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  if (stack_state == StackState::kNoHeapPointers || !IsGCWithStack()) return;
+  if (stack_state == StackState::kNoHeapPointers ||
+      disable_conservative_stack_scanning_for_testing_)
+    return;
 
   // In case of a shared GC, we're interested in the main isolate for CSS.
-  Isolate* main_isolate;
-  if (mode == IterateRootsMode::kShared) {
-    main_isolate = isolate()->shared_heap_isolate();
-    if (!main_isolate->heap()->IsGCWithStack()) return;
-  } else {
-    main_isolate = isolate();
-  }
+  Isolate* main_isolate = mode == IterateRootsMode::kShared
+                              ? isolate()->shared_heap_isolate()
+                              : isolate();
+  if (main_isolate->heap()->disable_conservative_stack_scanning_for_testing_)
+    return;
 
   ConservativeStackVisitor stack_visitor(main_isolate, v);
   stack().IteratePointers(&stack_visitor);

src/heap/heap.h

@@ -2409,6 +2409,7 @@ 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_;
@@ -2685,6 +2686,23 @@ 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_;
+  }
+
+ private:
+  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.
@@ -2840,17 +2858,6 @@ 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,7 +2115,8 @@ void MarkCompactCollector::MarkRoots(RootVisitor* root_visitor) {
     // v8::TracedReference alive from the stack. This is only needed when using
     // `EmbedderHeapTracer` and not using `CppHeap`.
     auto& stack = heap()->stack();
-    if (heap_->IsGCWithStack()) {
+    if (heap_->local_embedder_heap_tracer()->embedder_stack_state() ==
+        cppgc::EmbedderStackState::kMayContainHeapPointers) {
       ConservativeTracedHandlesMarkingVisitor conservative_marker(
           *heap_, *local_marking_worklists_,
           cppgc::internal::CollectionType::kMajor);

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

@@ -517,6 +517,12 @@ 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());
@@ -559,13 +565,7 @@ V8_NOINLINE void HeapToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle.Reset();
-  {
-    // 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);
-  }
+  FullGC(v8_isolate);
   EXPECT_TRUE(observer.IsEmpty());
 }
 
@@ -603,13 +603,7 @@ V8_NOINLINE void StackToStackTest(v8::Isolate* v8_isolate, Operation op,
   FullGC(v8_isolate);
   EXPECT_FALSE(observer.IsEmpty());
   stack_handle2.Reset();
-  {
-    // 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);
-  }
+  FullGC(v8_isolate);
   EXPECT_TRUE(observer.IsEmpty());
 }
 

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

@@ -484,6 +484,12 @@ TEST_F(EmbedderTracingTest, TracedReferenceHandlesMarking) {
       // Disable scanning, assuming the slots are overwritten.
       DisableConservativeStackScanningScopeForTesting no_stack_scanning(
           i_isolate()->heap());
+      EmbedderStackStateScope scope =
+          EmbedderStackStateScope::ExplicitScopeForTesting(
+              reinterpret_cast<i::Isolate*>(v8_isolate())
+                  ->heap()
+                  ->local_embedder_heap_tracer(),
+              EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
       FullGC();
     }
     const size_t final_count = traced_handles->used_node_count();
@@ -584,6 +590,12 @@ TEST_F(EmbedderTracingTest, BasicTracedReference) {
     // Disable scanning, assuming the slots are overwritten.
     DisableConservativeStackScanningScopeForTesting no_stack_scanning(
         i_isolate()->heap());
+    EmbedderStackStateScope scope =
+        EmbedderStackStateScope::ExplicitScopeForTesting(
+            reinterpret_cast<i::Isolate*>(v8_isolate())
+                ->heap()
+                ->local_embedder_heap_tracer(),
+            EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
     FullGC();
   }
   EXPECT_EQ(initial_count, traced_handles->used_node_count());