[heap][test] Refactor tests using heap internals

This CL refactors most of the cctests and unittests (22 out of 31) that
directly invoke heap GC, so that the corresponding internal heap methods
are called from a few specific places in boilerplate code. This will
facilitate impending changes to the interface of GC-related internal
heap methods.

Bug: v8:13257
Change-Id: Ia6773a7952501b0792b279b799171519620497d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3869264Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82927}

test/cctest/cctest.cc

@@ -219,6 +219,11 @@ void CcTest::PreciseCollectAllGarbage(i::Isolate* isolate) {
                                         i::GarbageCollectionReason::kTesting);
 }
 
+void CcTest::CollectSharedGarbage(i::Isolate* isolate) {
+  i::Isolate* iso = isolate ? isolate : i_isolate();
+  iso->heap()->CollectSharedGarbage(i::GarbageCollectionReason::kTesting);
+}
+
 i::Handle<i::String> CcTest::MakeString(const char* str) {
   i::Isolate* isolate = CcTest::i_isolate();
   i::Factory* factory = isolate->factory();

test/cctest/cctest.h

@@ -175,6 +175,7 @@ class CcTest {
   static void CollectAllGarbage(i::Isolate* isolate = nullptr);
   static void CollectAllAvailableGarbage(i::Isolate* isolate = nullptr);
   static void PreciseCollectAllGarbage(i::Isolate* isolate = nullptr);
+  static void CollectSharedGarbage(i::Isolate* isolate = nullptr);
 
   static i::Handle<i::String> MakeString(const char* str);
   static i::Handle<i::String> MakeName(const char* str, int suffix);

test/cctest/heap/test-heap.cc

@@ -3549,20 +3549,17 @@ void ReleaseStackTraceDataTest(v8::Isolate* isolate, const char* source,
     v8::Local<v8::Context> ctx = isolate->GetCurrentContext();
     v8::Local<v8::String> source_string =
         v8::String::NewExternalOneByte(isolate, resource).ToLocalChecked();
-    i_isolate->heap()->CollectAllAvailableGarbage(
-        i::GarbageCollectionReason::kTesting);
+    CcTest::CollectAllAvailableGarbage(i_isolate);
     v8::Script::Compile(ctx, source_string)
         .ToLocalChecked()
         ->Run(ctx)
         .ToLocalChecked();
     CHECK(!resource->IsDisposed());
   }
-  // i_isolate->heap()->CollectAllAvailableGarbage();
   CHECK(!resource->IsDisposed());
 
   CompileRun(accessor);
-  i_isolate->heap()->CollectAllAvailableGarbage(
-      i::GarbageCollectionReason::kTesting);
+  CcTest::CollectAllAvailableGarbage(i_isolate);
 
   // External source has been released.
   CHECK(resource->IsDisposed());
@@ -6649,7 +6646,7 @@ UNINITIALIZED_TEST(OutOfMemoryIneffectiveGC) {
   isolate->SetOOMErrorHandler(OOMCallback);
   Factory* factory = i_isolate->factory();
   Heap* heap = i_isolate->heap();
-  heap->CollectAllGarbage(Heap::kNoGCFlags, GarbageCollectionReason::kTesting);
+  CcTest::CollectAllGarbage(i_isolate);
   {
     HandleScope scope(i_isolate);
     while (heap->OldGenerationSizeOfObjects() <

test/cctest/test-api.cc

@@ -13684,8 +13684,7 @@ UNINITIALIZED_TEST(SetJitCodeEventHandler) {
   i::Heap* heap = i_isolate->heap();
 
   // Start with a clean slate.
-  heap->CollectAllAvailableGarbage(i::GarbageCollectionReason::kTesting);
-
+  CcTest::CollectAllAvailableGarbage(i_isolate);
   {
     v8::HandleScope scope(isolate);
     v8::base::HashMap code;
@@ -13729,7 +13728,7 @@ UNINITIALIZED_TEST(SetJitCodeEventHandler) {
     }
 
     // Force code movement.
-    heap->CollectAllAvailableGarbage(i::GarbageCollectionReason::kTesting);
+    CcTest::CollectAllAvailableGarbage(i_isolate);
 
     isolate->SetJitCodeEventHandler(v8::kJitCodeEventDefault, nullptr);
 

test/cctest/test-serialize.cc

@@ -208,8 +208,7 @@ static StartupBlobs Serialize(v8::Isolate* isolate) {
   }
 
   Isolate* i_isolate = reinterpret_cast<Isolate*>(isolate);
-  i_isolate->heap()->CollectAllAvailableGarbage(
-      i::GarbageCollectionReason::kTesting);
+  CcTest::CollectAllAvailableGarbage(i_isolate);
 
   SafepointScope safepoint(i_isolate->heap());
   HandleScope scope(i_isolate);
@@ -392,7 +391,7 @@ static void SerializeContext(base::Vector<const byte>* startup_blob_out,
 
     // If we don't do this then we end up with a stray root pointing at the
     // context even after we have disposed of env.
-    heap->CollectAllAvailableGarbage(i::GarbageCollectionReason::kTesting);
+    CcTest::CollectAllAvailableGarbage(isolate);
 
     {
       v8::HandleScope handle_scope(v8_isolate);
@@ -561,8 +560,7 @@ static void SerializeCustomContext(
     }
     // If we don't do this then we end up with a stray root pointing at the
     // context even after we have disposed of env.
-    isolate->heap()->CollectAllAvailableGarbage(
-        i::GarbageCollectionReason::kTesting);
+    CcTest::CollectAllAvailableGarbage(isolate);
 
     {
       v8::HandleScope handle_scope(v8_isolate);

test/cctest/test-shared-strings.cc

@@ -686,11 +686,9 @@ UNINITIALIZED_TEST(PromotionMarkCompact) {
 
     // 1st GC moves `one_byte_seq` to old space and 2nd GC evacuates it within
     // old space.
-    heap->CollectAllGarbage(Heap::kNoGCFlags,
-                            GarbageCollectionReason::kTesting);
+    CcTest::CollectAllGarbage(i_isolate);
     heap::ForceEvacuationCandidate(i::Page::FromHeapObject(*one_byte_seq));
-    heap->CollectAllGarbage(Heap::kNoGCFlags,
-                            GarbageCollectionReason::kTesting);
+    CcTest::CollectAllGarbage(i_isolate);
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
@@ -729,7 +727,7 @@ UNINITIALIZED_TEST(PromotionScavenge) {
     CHECK(heap->InSpace(*one_byte_seq, NEW_SPACE));
 
     for (int i = 0; i < 2; i++) {
-      heap->CollectGarbage(NEW_SPACE, GarbageCollectionReason::kTesting);
+      CcTest::CollectGarbage(NEW_SPACE, i_isolate);
     }
 
     // In-place-internalizable strings are promoted into the shared heap when
@@ -779,7 +777,7 @@ UNINITIALIZED_TEST(PromotionScavengeOldToShared) {
         RememberedSet<OLD_TO_NEW>::Contains(old_object_chunk, slot.address()));
 
     for (int i = 0; i < 2; i++) {
-      heap->CollectGarbage(NEW_SPACE, GarbageCollectionReason::kTesting);
+      CcTest::CollectGarbage(NEW_SPACE, i_isolate);
     }
 
     // In-place-internalizable strings are promoted into the shared heap when
@@ -830,7 +828,7 @@ UNINITIALIZED_TEST(PromotionMarkCompactNewToShared) {
     CHECK(
         RememberedSet<OLD_TO_NEW>::Contains(old_object_chunk, slot.address()));
 
-    heap->CollectGarbage(OLD_SPACE, GarbageCollectionReason::kTesting);
+    CcTest::CollectGarbage(OLD_SPACE, i_isolate);
 
     // In-place-internalizable strings are promoted into the shared heap when
     // sharing.
@@ -938,7 +936,7 @@ UNINITIALIZED_TEST(PagePromotionRecordingOldToShared) {
 
     young_object->set(0, *shared_string);
 
-    heap->CollectGarbage(OLD_SPACE, GarbageCollectionReason::kTesting);
+    CcTest::CollectGarbage(OLD_SPACE, i_isolate);
 
     // Object should get promoted using page promotion, so address should remain
     // the same.
@@ -984,7 +982,7 @@ UNINITIALIZED_TEST(SharedStringsTransitionDuringGC) {
     }
 
     // Trigger garbage collection on the shared isolate.
-    i_isolate->heap()->CollectSharedGarbage(GarbageCollectionReason::kTesting);
+    CcTest::CollectSharedGarbage(i_isolate);
 
     // Check that GC cleared the forwarding table.
     CHECK_EQ(i_isolate->string_forwarding_table()->size(), 0);

test/cctest/wasm/test-wasm-serialization.cc

@@ -96,8 +96,7 @@ class WasmSerializationTest {
   void CollectGarbage() {
     // Try hard to collect all garbage and will therefore also invoke all weak
     // callbacks of actually unreachable persistent handles.
-    CcTest::i_isolate()->heap()->CollectAllAvailableGarbage(
-        GarbageCollectionReason::kTesting);
+    CcTest::CollectAllAvailableGarbage();
   }
 
  private:

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

@@ -379,8 +379,7 @@ TEST_F(EmbedderTracingTest, V8RegisterEmbedderReference) {
   v8::Local<v8::Object> api_object = ConstructTraceableJSApiObject(
       context, first_and_second_field, first_and_second_field);
   ASSERT_FALSE(api_object.IsEmpty());
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      GarbageCollectionReason::kTesting);
+  CollectGarbage(i::OLD_SPACE);
   EXPECT_TRUE(tracer.IsRegisteredFromV8(first_and_second_field));
 }
 
@@ -402,8 +401,7 @@ TEST_F(EmbedderTracingTest, EmbedderRegisteringV8Reference) {
     handle->Reset(v8_isolate(), o);
   }
   tracer.AddReferenceForTracing(handle.get());
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      GarbageCollectionReason::kTesting);
+  CollectGarbage(i::OLD_SPACE);
   EXPECT_FALSE(handle->IsEmpty());
 }
 
@@ -431,8 +429,7 @@ TEST_F(EmbedderTracingTest, TracingInEphemerons) {
     int32_t hash = js_key->GetOrCreateHash(i_isolate()).value();
     JSWeakCollection::Set(weak_map, js_key, js_api_object, hash);
   }
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      GarbageCollectionReason::kTesting);
+  CollectGarbage(i::OLD_SPACE);
   EXPECT_TRUE(tracer.IsRegisteredFromV8(first_and_second_field));
 }
 
@@ -442,8 +439,7 @@ TEST_F(EmbedderTracingTest, FinalizeTracingIsNoopWhenNotMarking) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(v8_isolate(), &tracer);
 
   // Finalize a potentially running garbage collection.
-  i_isolate()->heap()->CollectGarbage(OLD_SPACE,
-                                      GarbageCollectionReason::kTesting);
+  CollectGarbage(OLD_SPACE);
   EXPECT_TRUE(i_isolate()->heap()->incremental_marking()->IsStopped());
 
   int gc_counter = i_isolate()->heap()->gc_count();
@@ -460,7 +456,7 @@ TEST_F(EmbedderTracingTest, FinalizeTracingWhenMarking) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(v8_isolate(), &tracer);
 
   // Finalize a potentially running garbage collection.
-  heap->CollectGarbage(OLD_SPACE, GarbageCollectionReason::kTesting);
+  CollectGarbage(OLD_SPACE);
   if (heap->mark_compact_collector()->sweeping_in_progress()) {
     heap->mark_compact_collector()->EnsureSweepingCompleted(
         MarkCompactCollector::SweepingForcedFinalizationMode::kV8Only);

test/unittests/heap/page-promotion-unittest.cc

@@ -51,7 +51,7 @@ TEST_F(PagePromotionTest, PagePromotion_NewToOld) {
 
     // Ensure that the new space is empty so that the page to be promoted
     // does not contain the age mark.
-    heap->CollectGarbage(OLD_SPACE, i::GarbageCollectionReason::kTesting);
+    CollectGarbage(OLD_SPACE);
 
     std::vector<Handle<FixedArray>> handles;
     SimulateFullSpace(heap->new_space(), &handles);

test/unittests/objects/managed-unittest.cc

@@ -175,16 +175,14 @@ TEST_F(ManagedTest, CollectAcrossIsolates) {
           Managed<DeleteCounter>::FromSharedPtr(i_isolate2, 0, handle1->get());
       USE(handle2);
     }
-    i_isolate2->heap()->CollectAllAvailableGarbage(
-        i::GarbageCollectionReason::kTesting);
+    CollectAllAvailableGarbage(i_isolate2);
     CHECK_EQ(0, deleted);
     isolate2->Exit();
     isolate2->Dispose();
     CHECK_EQ(0, deleted);
   }
   // Should be deleted after the first isolate is destroyed.
-  i_isolate1->heap()->CollectAllAvailableGarbage(
-      i::GarbageCollectionReason::kTesting);
+  CollectAllAvailableGarbage(i_isolate1);
   CHECK_EQ(1, deleted);
   isolate1->Exit();
   isolate1->Dispose();

test/unittests/parser/scanner-streams-unittest.cc

@@ -786,12 +786,10 @@ TEST_F(ScannerStreamsTest, RelocatingCharacterStream) {
   i::String raw = *two_byte_string;
   // 1st GC moves `two_byte_string` to old space and 2nd GC evacuates it within
   // old space.
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      i::GarbageCollectionReason::kUnknown);
+  CollectGarbage(i::OLD_SPACE);
   i::Page::FromHeapObject(*two_byte_string)
       ->SetFlag(i::MemoryChunk::FORCE_EVACUATION_CANDIDATE_FOR_TESTING);
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      i::GarbageCollectionReason::kUnknown);
+  CollectGarbage(i::OLD_SPACE);
   // GC moved the string.
   CHECK_NE(raw, *two_byte_string);
   CHECK_EQ('c', two_byte_string_stream->Advance());
@@ -829,12 +827,10 @@ TEST_F(ScannerStreamsTest, RelocatingUnbufferedCharacterStream) {
   i::String raw = *two_byte_string;
   // 1st GC moves `two_byte_string` to old space and 2nd GC evacuates it within
   // old space.
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      i::GarbageCollectionReason::kUnknown);
+  CollectGarbage(i::OLD_SPACE);
   i::Page::FromHeapObject(*two_byte_string)
       ->SetFlag(i::MemoryChunk::FORCE_EVACUATION_CANDIDATE_FOR_TESTING);
-  i_isolate()->heap()->CollectGarbage(i::OLD_SPACE,
-                                      i::GarbageCollectionReason::kUnknown);
+  CollectGarbage(i::OLD_SPACE);
   // GC moved the string and buffer was updated to the correct location.
   CHECK_NE(raw, *two_byte_string);
 

test/unittests/test-utils.h

@@ -159,24 +159,30 @@ class WithIsolateScopeMixin : public TMixin {
         .ToLocalChecked();
   }
 
-  void CollectGarbage(i::AllocationSpace space) {
-    i_isolate()->heap()->CollectGarbage(space,
-                                        i::GarbageCollectionReason::kTesting);
+  void CollectGarbage(i::AllocationSpace space, i::Isolate* isolate = nullptr) {
+    i::Isolate* iso = isolate ? isolate : i_isolate();
+    iso->heap()->CollectGarbage(space, i::GarbageCollectionReason::kTesting,
+                                kNoGCCallbackFlags);
   }
 
-  void CollectAllGarbage() {
-    i_isolate()->heap()->CollectAllGarbage(
-        i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting);
+  void CollectAllGarbage(i::Isolate* isolate = nullptr) {
+    i::Isolate* iso = isolate ? isolate : i_isolate();
+    iso->heap()->CollectAllGarbage(i::Heap::kNoGCFlags,
+                                   i::GarbageCollectionReason::kTesting,
+                                   kNoGCCallbackFlags);
   }
 
-  void CollectAllAvailableGarbage() {
-    i_isolate()->heap()->CollectAllAvailableGarbage(
+  void CollectAllAvailableGarbage(i::Isolate* isolate = nullptr) {
+    i::Isolate* iso = isolate ? isolate : i_isolate();
+    iso->heap()->CollectAllAvailableGarbage(
         i::GarbageCollectionReason::kTesting);
   }
 
-  void PreciseCollectAllGarbage() {
-    i_isolate()->heap()->PreciseCollectAllGarbage(
-        i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting);
+  void PreciseCollectAllGarbage(i::Isolate* isolate = nullptr) {
+    i::Isolate* iso = isolate ? isolate : i_isolate();
+    iso->heap()->PreciseCollectAllGarbage(i::Heap::kNoGCFlags,
+                                          i::GarbageCollectionReason::kTesting,
+                                          kNoGCCallbackFlags);
   }
 
   v8::Local<v8::String> NewString(const char* string) {

test/unittests/utils/identity-map-unittest.cc

@@ -695,8 +695,7 @@ TEST_F(IdentityMapTest, ExplicitGC) {
   }
 
   // Do an explicit, real GC.
-  isolate()->heap()->CollectGarbage(i::NEW_SPACE,
-                                    i::GarbageCollectionReason::kTesting);
+  CollectGarbage(i::NEW_SPACE);
 
   // Check that searching for the numbers finds the same values.
   for (size_t i = 0; i < arraysize(num_keys); i++) {
@@ -816,8 +815,7 @@ TEST_F(IdentityMapTest, GCShortCutting) {
 
     // Do an explicit, real GC, this should short-cut the thin string to point
     // to the internalized string (this is not implemented for MinorMC).
-    isolate()->heap()->CollectGarbage(i::NEW_SPACE,
-                                      i::GarbageCollectionReason::kTesting);
+    CollectGarbage(i::NEW_SPACE);
     DCHECK_IMPLIES(!FLAG_minor_mc && !FLAG_optimize_for_size,
                    *thin_string == *internalized_string);