cppgc-js: Clean up unified heap tests

Bug: chromium:1056170
Change-Id: I6512792cc088f1ffaf1d6f3e367db02a1f2c8277
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504511
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70824}

src/api/api.cc

@@ -9819,7 +9819,7 @@ void debug::ForceGarbageCollection(
     v8::Isolate* isolate,
     v8::EmbedderHeapTracer::EmbedderStackState embedder_stack_state) {
   i::Heap* heap = reinterpret_cast<i::Isolate*>(isolate)->heap();
-  heap->SetEmbedderStackStateForNextFinalizaton(embedder_stack_state);
+  heap->SetEmbedderStackStateForNextFinalization(embedder_stack_state);
   isolate->LowMemoryNotification();
 }
 
@@ -11033,7 +11033,7 @@ void EmbedderHeapTracer::GarbageCollectionForTesting(
   CHECK(isolate_);
   CHECK(i::FLAG_expose_gc);
   i::Heap* const heap = reinterpret_cast<i::Isolate*>(isolate_)->heap();
-  heap->SetEmbedderStackStateForNextFinalizaton(stack_state);
+  heap->SetEmbedderStackStateForNextFinalization(stack_state);
   heap->PreciseCollectAllGarbage(i::Heap::kNoGCFlags,
                                  i::GarbageCollectionReason::kTesting,
                                  kGCCallbackFlagForced);

src/extensions/gc-extension.cc

@@ -82,7 +82,7 @@ void InvokeGC(v8::Isolate* isolate, v8::Isolate::GarbageCollectionType type,
                            kGCCallbackFlagForced);
       break;
     case v8::Isolate::GarbageCollectionType::kFullGarbageCollection:
-      heap->SetEmbedderStackStateForNextFinalizaton(embedder_stack_state);
+      heap->SetEmbedderStackStateForNextFinalization(embedder_stack_state);
       heap->PreciseCollectAllGarbage(i::Heap::kNoGCFlags,
                                      i::GarbageCollectionReason::kTesting,
                                      kGCCallbackFlagForced);

src/heap/heap.cc

@@ -6646,7 +6646,7 @@ bool Heap::PageFlagsAreConsistent(HeapObject object) {
   return true;
 }
 
-void Heap::SetEmbedderStackStateForNextFinalizaton(
+void Heap::SetEmbedderStackStateForNextFinalization(
     EmbedderHeapTracer::EmbedderStackState stack_state) {
   local_embedder_heap_tracer()->SetEmbedderStackStateForNextFinalization(
       stack_state);

src/heap/heap.h

@@ -1127,7 +1127,7 @@ class Heap {
   EmbedderHeapTracer* GetEmbedderHeapTracer() const;
 
   void RegisterExternallyReferencedObject(Address* location);
-  V8_EXPORT_PRIVATE void SetEmbedderStackStateForNextFinalizaton(
+  V8_EXPORT_PRIVATE void SetEmbedderStackStateForNextFinalization(
       EmbedderHeapTracer::EmbedderStackState stack_state);
 
   EmbedderHeapTracer::TraceFlags flags_for_embedder_tracer() const;

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

@@ -25,37 +25,12 @@ namespace internal {
 
 namespace {
 
-class UnifiedHeapSnapshotTest : public TestWithHeapInternals {
+class UnifiedHeapSnapshotTest : public UnifiedHeapTest {
  public:
-  UnifiedHeapSnapshotTest()
-      : saved_incremental_marking_wrappers_(FLAG_incremental_marking_wrappers) {
-    FLAG_incremental_marking_wrappers = false;
-    cppgc::InitializeProcess(V8::GetCurrentPlatform()->GetPageAllocator());
-    cpp_heap_ = std::make_unique<CppHeap>(
-        v8_isolate(), std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>());
-    heap()->SetEmbedderHeapTracer(&cpp_heap());
-  }
-
-  ~UnifiedHeapSnapshotTest() override {
-    heap()->SetEmbedderHeapTracer(nullptr);
-    FLAG_incremental_marking_wrappers = saved_incremental_marking_wrappers_;
-    cppgc::ShutdownProcess();
-  }
-
-  CppHeap& cpp_heap() const { return *cpp_heap_.get(); }
-
-  cppgc::AllocationHandle& allocation_handle() const {
-    return cpp_heap().object_allocator();
-  }
-
   const v8::HeapSnapshot* TakeHeapSnapshot() {
     v8::HeapProfiler* heap_profiler = v8_isolate()->GetHeapProfiler();
     return heap_profiler->TakeHeapSnapshot();
   }
-
- private:
-  std::unique_ptr<CppHeap> cpp_heap_;
-  bool saved_incremental_marking_wrappers_;
 };
 
 bool IsValidSnapshot(const v8::HeapSnapshot* snapshot, int depth = 3) {
@@ -316,7 +291,7 @@ TEST_F(UnifiedHeapSnapshotTest, JSReferenceForcesVisibleObject) {
   v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
   v8::Context::Scope context_scope(context);
   v8::Local<v8::Object> api_object =
-      ConstructTraceableJSApiObject(context, gc_w_js_ref.Get(), "LeafJSObject");
+      WrapperHelper::CreateWrapper(context, gc_w_js_ref.Get(), "LeafJSObject");
   gc_w_js_ref->SetV8Object(v8_isolate(), api_object);
   const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
   EXPECT_TRUE(IsValidSnapshot(snapshot));
@@ -339,14 +314,14 @@ TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) {
   v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
   v8::Context::Scope context_scope(context);
   v8::Local<v8::Object> wrapper_object =
-      ConstructTraceableJSApiObject(context, gc_w_js_ref.Get(), "MergedObject");
+      WrapperHelper::CreateWrapper(context, gc_w_js_ref.Get(), "MergedObject");
   gc_w_js_ref->SetV8Object(v8_isolate(), wrapper_object);
   gc_w_js_ref->SetWrapperClassId(1);  // Any class id will do.
   // Chain another object to `wrapper_object`. Since `wrapper_object` should be
   // merged into `GCedWithJSRef`, the additional object must show up as direct
   // child from `GCedWithJSRef`.
   v8::Local<v8::Object> next_object =
-      ConstructTraceableJSApiObject(context, nullptr, "NextObject");
+      WrapperHelper::CreateWrapper(context, nullptr, "NextObject");
   wrapper_object
       ->Set(context,
             v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), "link")

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

@@ -16,51 +16,6 @@ namespace internal {
 
 namespace {
 
-void ResetWrappableConnection(v8::Local<v8::Object> api_object) {
-  api_object->SetAlignedPointerInInternalField(0, nullptr);
-  api_object->SetAlignedPointerInInternalField(1, nullptr);
-}
-
-class UnifiedHeapTest : public TestWithHeapInternals {
- public:
-  UnifiedHeapTest()
-      : saved_incremental_marking_wrappers_(FLAG_incremental_marking_wrappers) {
-    FLAG_incremental_marking_wrappers = false;
-    cppgc::InitializeProcess(V8::GetCurrentPlatform()->GetPageAllocator());
-    cpp_heap_ = std::make_unique<CppHeap>(
-        v8_isolate(), std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>());
-    heap()->SetEmbedderHeapTracer(&cpp_heap());
-  }
-
-  ~UnifiedHeapTest() {
-    heap()->SetEmbedderHeapTracer(nullptr);
-    FLAG_incremental_marking_wrappers = saved_incremental_marking_wrappers_;
-    cppgc::ShutdownProcess();
-  }
-
-  void CollectGarbageWithEmbedderStack() {
-    heap()->SetEmbedderStackStateForNextFinalizaton(
-        EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
-    CollectGarbage(OLD_SPACE);
-  }
-
-  void CollectGarbageWithoutEmbedderStack() {
-    heap()->SetEmbedderStackStateForNextFinalizaton(
-        EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
-    CollectGarbage(OLD_SPACE);
-  }
-
-  CppHeap& cpp_heap() const { return *cpp_heap_.get(); }
-
-  cppgc::AllocationHandle& allocation_handle() {
-    return cpp_heap().object_allocator();
-  }
-
- private:
-  std::unique_ptr<CppHeap> cpp_heap_;
-  bool saved_incremental_marking_wrappers_;
-};
-
 class Wrappable final : public cppgc::GarbageCollected<Wrappable> {
  public:
   static size_t destructor_callcount;
@@ -80,13 +35,13 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
   v8::HandleScope scope(v8_isolate());
   v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
   v8::Context::Scope context_scope(context);
-  v8::Local<v8::Object> api_object = ConstructTraceableJSApiObject(
+  v8::Local<v8::Object> api_object = WrapperHelper::CreateWrapper(
       context, cppgc::MakeGarbageCollected<Wrappable>(allocation_handle()));
   EXPECT_FALSE(api_object.IsEmpty());
   EXPECT_EQ(0u, Wrappable::destructor_callcount);
   CollectGarbageWithoutEmbedderStack();
   EXPECT_EQ(0u, Wrappable::destructor_callcount);
-  ResetWrappableConnection(api_object);
+  WrapperHelper::ResetWrappableConnection(api_object);
   CollectGarbageWithoutEmbedderStack();
   // Calling CollectGarbage twice to force the first GC to finish sweeping.
   CollectGarbageWithoutEmbedderStack();

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

@@ -4,14 +4,51 @@
 
 #include "test/unittests/heap/unified-heap-utils.h"
 
+#include "include/cppgc/platform.h"
 #include "src/api/api-inl.h"
+#include "src/heap/cppgc-js/cpp-heap.h"
 #include "src/objects/objects-inl.h"
 
 namespace v8 {
 namespace internal {
 
-v8::Local<v8::Object> ConstructTraceableJSApiObject(
-    v8::Local<v8::Context> context, void* object, const char* class_name) {
+UnifiedHeapTest::UnifiedHeapTest()
+    : saved_incremental_marking_wrappers_(FLAG_incremental_marking_wrappers) {
+  FLAG_incremental_marking_wrappers = false;
+  cppgc::InitializeProcess(V8::GetCurrentPlatform()->GetPageAllocator());
+  cpp_heap_ = std::make_unique<CppHeap>(
+      v8_isolate(), std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>());
+  heap()->SetEmbedderHeapTracer(&cpp_heap());
+}
+
+UnifiedHeapTest::~UnifiedHeapTest() {
+  heap()->SetEmbedderHeapTracer(nullptr);
+  FLAG_incremental_marking_wrappers = saved_incremental_marking_wrappers_;
+  cppgc::ShutdownProcess();
+}
+
+void UnifiedHeapTest::CollectGarbageWithEmbedderStack() {
+  heap()->SetEmbedderStackStateForNextFinalization(
+      EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
+  CollectGarbage(OLD_SPACE);
+}
+
+void UnifiedHeapTest::CollectGarbageWithoutEmbedderStack() {
+  heap()->SetEmbedderStackStateForNextFinalization(
+      EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+  CollectGarbage(OLD_SPACE);
+}
+
+CppHeap& UnifiedHeapTest::cpp_heap() const { return *cpp_heap_.get(); }
+
+cppgc::AllocationHandle& UnifiedHeapTest::allocation_handle() {
+  return cpp_heap().object_allocator();
+}
+
+// static
+v8::Local<v8::Object> WrapperHelper::CreateWrapper(
+    v8::Local<v8::Context> context, void* wrappable_object,
+    const char* class_name) {
   v8::EscapableHandleScope scope(context->GetIsolate());
   v8::Local<v8::FunctionTemplate> function_t =
       v8::FunctionTemplate::New(context->GetIsolate());
@@ -26,13 +63,19 @@ v8::Local<v8::Object> ConstructTraceableJSApiObject(
       function_t->GetFunction(context).ToLocalChecked();
   v8::Local<v8::Object> instance =
       function->NewInstance(context).ToLocalChecked();
-  instance->SetAlignedPointerInInternalField(0, object);
-  instance->SetAlignedPointerInInternalField(1, object);
+  instance->SetAlignedPointerInInternalField(0, wrappable_object);
+  instance->SetAlignedPointerInInternalField(1, wrappable_object);
   CHECK(!instance.IsEmpty());
   i::Handle<i::JSReceiver> js_obj = v8::Utils::OpenHandle(*instance);
   CHECK_EQ(i::JS_API_OBJECT_TYPE, js_obj->map().instance_type());
   return scope.Escape(instance);
 }
 
+// static
+void WrapperHelper::ResetWrappableConnection(v8::Local<v8::Object> api_object) {
+  api_object->SetAlignedPointerInInternalField(0, nullptr);
+  api_object->SetAlignedPointerInInternalField(1, nullptr);
+}
+
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/unified-heap-utils.h

@@ -5,16 +5,44 @@
 #ifndef V8_UNITTESTS_HEAP_UNIFIED_HEAP_UTILS_H_
 #define V8_UNITTESTS_HEAP_UNIFIED_HEAP_UTILS_H_
 
+#include "include/cppgc/heap.h"
 #include "include/v8.h"
+#include "test/unittests/heap/heap-utils.h"
 
 namespace v8 {
 namespace internal {
 
-// Sets up a V8 API object so that it points back to a C++ object. The setup
-// used is recognized by the GC and references will be followed for liveness
-// analysis (marking) as well as tooling (snapshot).
-v8::Local<v8::Object> ConstructTraceableJSApiObject(
-    v8::Local<v8::Context> context, void* object, const char* class_name = "");
+class CppHeap;
+
+class UnifiedHeapTest : public TestWithHeapInternals {
+ public:
+  UnifiedHeapTest();
+  ~UnifiedHeapTest() override;
+
+  void CollectGarbageWithEmbedderStack();
+  void CollectGarbageWithoutEmbedderStack();
+
+  CppHeap& cpp_heap() const;
+  cppgc::AllocationHandle& allocation_handle();
+
+ private:
+  std::unique_ptr<CppHeap> cpp_heap_;
+  bool saved_incremental_marking_wrappers_;
+};
+
+class WrapperHelper {
+ public:
+  // Sets up a V8 API object so that it points back to a C++ object. The setup
+  // used is recognized by the GC and references will be followed for liveness
+  // analysis (marking) as well as tooling (snapshot).
+  static v8::Local<v8::Object> CreateWrapper(v8::Local<v8::Context> context,
+                                             void* wrappable_object,
+                                             const char* class_name = "");
+
+  // Resets the connection of a wrapper (JS) to its wrappable (C++), meaning
+  // that the wrappable object is not longer kept alive by the wrapper object.
+  static void ResetWrappableConnection(v8::Local<v8::Object> api_object);
+};
 
 }  // namespace internal
 }  // namespace v8