Revert "cppgc: Fix testing APIs that enable garbage collection"

This reverts commit ea818f07.

Reason for revert: Test failure in Linux64 UBSan https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/15251/overview

Original change's description:
> cppgc: Fix testing APIs that enable garbage collection
>
> The APIs require that the CppHeap is moved into a permanently detached
> state that moves the heap out of a no-gc scope.
>
> Bug: chromium:1056170
> Change-Id: I1fc08451b3fdfaa4cfe58e6a1ddbe5dbed7efe5c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2718146
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#73025}

Bug: chromium:1056170
Change-Id: Id00cb18274cbe7d255e7e95bd9e8e4dbc4b0c6e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2718658
Auto-Submit: Zhi An Ng <zhin@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Francis McCabe <fgm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73029}

include/cppgc/testing.h

@@ -23,17 +23,13 @@ namespace testing {
  */
 class V8_EXPORT Heap final {
  public:
-  explicit Heap(HeapHandle& heap_handle) : heap_handle_(heap_handle) {}
-
   /**
    * Atomically collects garbage on the C++ heap.
    *
+   * \param heap_handle The corresponding heap.
    * \param stack_state The stack state to assume for the garbage collection.
    */
-  void CollectGarbage(EmbedderStackState stack_state);
-
- private:
-  HeapHandle& heap_handle_;
+  void CollectGarbage(HeapHandle& heap_handle, EmbedderStackState stack_state);
 };
 
 /**

include/v8-cppgc.h

@@ -118,13 +118,6 @@ class V8_EXPORT CppHeap {
   cppgc::HeapStatistics CollectStatistics(
       cppgc::HeapStatistics::DetailLevel detail_level);
 
-  /**
-   * Enables a detached mode that allows testing garbage collection using
-   * `cppgc::testing` APIs. Once used, the heap cannot be attached to an
-   * `Isolate` anymore.
-   */
-  void EnableDetachedGarbageCollectionsForTesting();
-
  private:
   CppHeap() = default;
 

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

@@ -10,7 +10,6 @@
 #include "include/cppgc/platform.h"
 #include "include/v8-platform.h"
 #include "include/v8.h"
-#include "src/base/logging.h"
 #include "src/base/macros.h"
 #include "src/base/platform/time.h"
 #include "src/execution/isolate.h"
@@ -65,11 +64,6 @@ cppgc::HeapStatistics CppHeap::CollectStatistics(
       detail_level);
 }
 
-void CppHeap::EnableDetachedGarbageCollectionsForTesting() {
-  return internal::CppHeap::From(this)
-      ->EnableDetachedGarbageCollectionsForTesting();
-}
-
 void JSHeapConsistency::DijkstraMarkingBarrierSlow(
     cppgc::HeapHandle& heap_handle, const TracedReferenceBase& ref) {
   auto& heap_base = cppgc::internal::HeapBase::From(heap_handle);
@@ -227,7 +221,6 @@ void CppHeap::Terminate() {
 }
 
 void CppHeap::AttachIsolate(Isolate* isolate) {
-  CHECK(!in_detached_testing_mode);
   CHECK_NULL(isolate_);
   isolate_ = isolate;
   static_cast<CppgcPlatformAdapter*>(platform())
@@ -397,12 +390,5 @@ void CppHeap::ReportBufferedAllocationSizeIfPossible() {
   buffered_allocated_bytes_ = 0;
 }
 
-void CppHeap::EnableDetachedGarbageCollectionsForTesting() {
-  CHECK(!in_detached_testing_mode);
-  CHECK_NULL(isolate_);
-  no_gc_scope_--;
-  in_detached_testing_mode = true;
-}
-
 }  // namespace internal
 }  // namespace v8

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

@@ -64,8 +64,6 @@ class V8_EXPORT_PRIVATE CppHeap final
   void AllocatedObjectSizeDecreased(size_t) final;
   void ResetAllocatedObjectSize(size_t) final {}
 
-  void EnableDetachedGarbageCollectionsForTesting();
-
  private:
   void FinalizeIncrementalGarbageCollectionIfNeeded(
       cppgc::Heap::StackState) final {
@@ -85,8 +83,6 @@ class V8_EXPORT_PRIVATE CppHeap final
   int64_t buffered_allocated_bytes_ = 0;
 
   v8::WrapperDescriptor wrapper_descriptor_;
-
-  bool in_detached_testing_mode = false;
 };
 
 }  // namespace internal

src/heap/cppgc/heap-base.cc

@@ -153,6 +153,7 @@ void HeapBase::StandAloneGarbageCollectionForTesting(
       GarbageCollector::Config::CollectionType::kMajor, stack_state,
       GarbageCollector::Config::MarkingType::kAtomic,
       GarbageCollector::Config::SweepingType::kAtomic};
+
   if (in_no_gc_scope()) return;
 
   if (!IsMarking()) {

src/heap/cppgc/testing.cc

@@ -10,8 +10,9 @@
 namespace cppgc {
 namespace testing {
 
-void Heap::CollectGarbage(EmbedderStackState stack_state) {
-  auto& heap = internal::HeapBase::From(heap_handle_);
+void Heap::CollectGarbage(HeapHandle& heap_handle,
+                          EmbedderStackState stack_state) {
+  auto& heap = internal::HeapBase::From(heap_handle);
   heap.StandAloneGarbageCollectionForTesting(stack_state);
 }
 

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

@@ -6,7 +6,6 @@
 #include "include/cppgc/garbage-collected.h"
 #include "include/cppgc/persistent.h"
 #include "include/cppgc/platform.h"
-#include "include/cppgc/testing.h"
 #include "include/v8-cppgc.h"
 #include "include/v8.h"
 #include "src/api/api-inl.h"
@@ -141,7 +140,6 @@ TEST_F(UnifiedHeapDetachedTest, AllocationBeforeConfigureHeap) {
     cpp_heap.AsBase().sweeper().FinishIfRunning();
     EXPECT_TRUE(weak_holder);
   }
-  USE(object);
   {
     js_heap.SetEmbedderStackStateForNextFinalization(
         EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
@@ -151,27 +149,5 @@ TEST_F(UnifiedHeapDetachedTest, AllocationBeforeConfigureHeap) {
   }
 }
 
-TEST_F(UnifiedHeapDetachedTest, StandAloneCppGC) {
-  auto heap = v8::CppHeap::Create(
-      V8::GetCurrentPlatform(),
-      CppHeapCreateParams{{}, WrapperHelper::DefaultWrapperDescriptor()});
-  auto* object =
-      cppgc::MakeGarbageCollected<Wrappable>(heap->GetAllocationHandle());
-  cppgc::WeakPersistent<Wrappable> weak_holder{object};
-
-  heap->EnableDetachedGarbageCollectionsForTesting();
-  cppgc::testing::Heap testing_heap{heap->GetHeapHandle()};
-  {
-    testing_heap.CollectGarbage(
-        cppgc::EmbedderStackState::kMayContainHeapPointers);
-    EXPECT_TRUE(weak_holder);
-  }
-  USE(object);
-  {
-    testing_heap.CollectGarbage(cppgc::EmbedderStackState::kNoHeapPointers);
-    EXPECT_FALSE(weak_holder);
-  }
-}
-
 }  // namespace internal
 }  // namespace v8