From f300a01a639ea7291636f8d031b9650389974c57 Mon Sep 17 00:00:00 2001 From: Michael Lippautz Date: Wed, 27 Oct 2021 15:10:43 +0200 Subject: [PATCH] unittests: Provide Context in TestWithHeapInternals Change-Id: I54e658325dfbfb425c41cab2fd7b32253b380e37 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3247038 Commit-Queue: Michael Lippautz Commit-Queue: Omer Katz Auto-Submit: Michael Lippautz Reviewed-by: Omer Katz Cr-Commit-Position: refs/heads/main@{#77577} --- test/unittests/heap/heap-utils.h | 13 +- .../heap/traced-reference-unittest.cc | 230 ++++++++---------- test/unittests/heap/unified-heap-unittest.cc | 18 +- 3 files changed, 107 insertions(+), 154 deletions(-) diff --git a/test/unittests/heap/heap-utils.h b/test/unittests/heap/heap-utils.h index 2cd123c827..1839fcf563 100644 --- a/test/unittests/heap/heap-utils.h +++ b/test/unittests/heap/heap-utils.h @@ -37,12 +37,13 @@ class WithHeapInternals : public TMixin, HeapInternalsBase { } }; -using TestWithHeapInternals = // - WithHeapInternals< // - WithInternalIsolateMixin< // - WithIsolateScopeMixin< // - WithIsolateMixin< // - ::testing::Test>>>>; +using TestWithHeapInternals = // + WithHeapInternals< // + WithContextMixin< // + WithInternalIsolateMixin< // + WithIsolateScopeMixin< // + WithIsolateMixin< // + ::testing::Test>>>>>; } // namespace internal } // namespace v8 diff --git a/test/unittests/heap/traced-reference-unittest.cc b/test/unittests/heap/traced-reference-unittest.cc index b47262ec57..cbcb5fb758 100644 --- a/test/unittests/heap/traced-reference-unittest.cc +++ b/test/unittests/heap/traced-reference-unittest.cc @@ -10,155 +10,119 @@ namespace v8 { namespace internal { -using TracedReferenceTest = TestWithIsolate; +using TracedReferenceTest = TestWithContext; TEST_F(TracedReferenceTest, ResetFromLocal) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); v8::TracedReference ref; - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - EXPECT_TRUE(ref.IsEmpty()); - EXPECT_NE(ref, local); - ref.Reset(v8_isolate(), local); - EXPECT_FALSE(ref.IsEmpty()); - EXPECT_EQ(ref, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + EXPECT_TRUE(ref.IsEmpty()); + EXPECT_NE(ref, local); + ref.Reset(v8_isolate(), local); + EXPECT_FALSE(ref.IsEmpty()); + EXPECT_EQ(ref, local); } TEST_F(TracedReferenceTest, ConstructFromLocal) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref(v8_isolate(), local); - EXPECT_FALSE(ref.IsEmpty()); - EXPECT_EQ(ref, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref(v8_isolate(), local); + EXPECT_FALSE(ref.IsEmpty()); + EXPECT_EQ(ref, local); } TEST_F(TracedReferenceTest, Reset) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref(v8_isolate(), local); - EXPECT_FALSE(ref.IsEmpty()); - EXPECT_EQ(ref, local); - ref.Reset(); - EXPECT_TRUE(ref.IsEmpty()); - EXPECT_NE(ref, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref(v8_isolate(), local); + EXPECT_FALSE(ref.IsEmpty()); + EXPECT_EQ(ref, local); + ref.Reset(); + EXPECT_TRUE(ref.IsEmpty()); + EXPECT_NE(ref, local); } TEST_F(TracedReferenceTest, Copy) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref(v8_isolate(), local); - v8::TracedReference ref_copy1(ref); - v8::TracedReference ref_copy2 = ref; - EXPECT_EQ(ref, local); - EXPECT_EQ(ref_copy1, local); - EXPECT_EQ(ref_copy2, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref(v8_isolate(), local); + v8::TracedReference ref_copy1(ref); + v8::TracedReference ref_copy2 = ref; + EXPECT_EQ(ref, local); + EXPECT_EQ(ref_copy1, local); + EXPECT_EQ(ref_copy2, local); } TEST_F(TracedReferenceTest, CopyHeterogenous) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref(v8_isolate(), local); - v8::TracedReference ref_copy1(ref); - v8::TracedReference ref_copy2 = ref; - EXPECT_EQ(ref, local); - EXPECT_EQ(ref_copy1, local); - EXPECT_EQ(ref_copy2, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref(v8_isolate(), local); + v8::TracedReference ref_copy1(ref); + v8::TracedReference ref_copy2 = ref; + EXPECT_EQ(ref, local); + EXPECT_EQ(ref_copy1, local); + EXPECT_EQ(ref_copy2, local); } TEST_F(TracedReferenceTest, Move) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref(v8_isolate(), local); - v8::TracedReference ref_moved1(std::move(ref)); - v8::TracedReference ref_moved2 = std::move(ref_moved1); - EXPECT_TRUE(ref.IsEmpty()); - EXPECT_TRUE(ref_moved1.IsEmpty()); - EXPECT_EQ(ref_moved2, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref(v8_isolate(), local); + v8::TracedReference ref_moved1(std::move(ref)); + v8::TracedReference ref_moved2 = std::move(ref_moved1); + EXPECT_TRUE(ref.IsEmpty()); + EXPECT_TRUE(ref_moved1.IsEmpty()); + EXPECT_EQ(ref_moved2, local); } TEST_F(TracedReferenceTest, MoveHeterogenous) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref1(v8_isolate(), local); - v8::TracedReference ref_moved1(std::move(ref1)); - v8::TracedReference ref2(v8_isolate(), local); - v8::TracedReference ref_moved2 = std::move(ref2); - EXPECT_TRUE(ref1.IsEmpty()); - EXPECT_EQ(ref_moved1, local); - EXPECT_TRUE(ref2.IsEmpty()); - EXPECT_EQ(ref_moved2, local); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref1(v8_isolate(), local); + v8::TracedReference ref_moved1(std::move(ref1)); + v8::TracedReference ref2(v8_isolate(), local); + v8::TracedReference ref_moved2 = std::move(ref2); + EXPECT_TRUE(ref1.IsEmpty()); + EXPECT_EQ(ref_moved1, local); + EXPECT_TRUE(ref2.IsEmpty()); + EXPECT_EQ(ref_moved2, local); } TEST_F(TracedReferenceTest, Equality) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local1 = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref1(v8_isolate(), local1); - v8::TracedReference ref2(v8_isolate(), local1); - EXPECT_EQ(ref1, ref2); - EXPECT_EQ(ref2, ref1); - v8::Local local2 = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref3(v8_isolate(), local2); - EXPECT_NE(ref2, ref3); - EXPECT_NE(ref3, ref2); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local1 = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref1(v8_isolate(), local1); + v8::TracedReference ref2(v8_isolate(), local1); + EXPECT_EQ(ref1, ref2); + EXPECT_EQ(ref2, ref1); + v8::Local local2 = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref3(v8_isolate(), local2); + EXPECT_NE(ref2, ref3); + EXPECT_NE(ref3, ref2); } TEST_F(TracedReferenceTest, EqualityHeterogenous) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local1 = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref1(v8_isolate(), local1); - v8::TracedReference ref2(v8_isolate(), local1); - EXPECT_EQ(ref1, ref2); - EXPECT_EQ(ref2, ref1); - v8::Local local2 = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference ref3(v8_isolate(), local2); - EXPECT_NE(ref2, ref3); - EXPECT_NE(ref3, ref2); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local1 = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref1(v8_isolate(), local1); + v8::TracedReference ref2(v8_isolate(), local1); + EXPECT_EQ(ref1, ref2); + EXPECT_EQ(ref2, ref1); + v8::Local local2 = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference ref3(v8_isolate(), local2); + EXPECT_NE(ref2, ref3); + EXPECT_NE(ref3, ref2); } namespace { @@ -185,19 +149,15 @@ class JSVisitorForTesting final : public JSVisitor { } // namespace TEST_F(TracedReferenceTest, TracedReferenceTrace) { - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); - { - v8::HandleScope handles(v8_isolate()); - v8::Local local = - v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); - v8::TracedReference js_member(v8_isolate(), local); - JSVisitorForTesting visitor(local); - // Cast to cppgc::Visitor to ensure that we dispatch through the base - // visitor and use traits. - static_cast(visitor).Trace(js_member); - EXPECT_EQ(1u, visitor.visit_count()); - } + v8::HandleScope handles(v8_isolate()); + v8::Local local = + v8::Local::New(v8_isolate(), v8::Object::New(v8_isolate())); + v8::TracedReference js_member(v8_isolate(), local); + JSVisitorForTesting visitor(local); + // Cast to cppgc::Visitor to ensure that we dispatch through the base + // visitor and use traits. + static_cast(visitor).Trace(js_member); + EXPECT_EQ(1u, visitor.visit_count()); } } // namespace internal diff --git a/test/unittests/heap/unified-heap-unittest.cc b/test/unittests/heap/unified-heap-unittest.cc index 5f9bbc3e42..0abdcdcf25 100644 --- a/test/unittests/heap/unified-heap-unittest.cc +++ b/test/unittests/heap/unified-heap-unittest.cc @@ -60,11 +60,9 @@ TEST_F(UnifiedHeapTest, OnlyGC) { CollectGarbageWithEmbedderStack(); } TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) { v8::HandleScope scope(v8_isolate()); - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); uint16_t wrappable_type = WrapperHelper::kTracedEmbedderId; v8::Local api_object = WrapperHelper::CreateWrapper( - context, &wrappable_type, + context(), &wrappable_type, cppgc::MakeGarbageCollected(allocation_handle())); Wrappable::destructor_callcount = 0; EXPECT_FALSE(api_object.IsEmpty()); @@ -78,12 +76,11 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) { TEST_F(UnifiedHeapTest, WriteBarrierV8ToCppReference) { if (!FLAG_incremental_marking) return; + v8::HandleScope scope(v8_isolate()); - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); void* wrappable = cppgc::MakeGarbageCollected(allocation_handle()); v8::Local api_object = - WrapperHelper::CreateWrapper(context, nullptr, nullptr); + WrapperHelper::CreateWrapper(context(), nullptr, nullptr); Wrappable::destructor_callcount = 0; WrapperHelper::ResetWrappableConnection(api_object); SimulateIncrementalMarking(); @@ -105,9 +102,8 @@ TEST_F(UnifiedHeapTest, WriteBarrierV8ToCppReference) { TEST_F(UnifiedHeapTest, WriteBarrierCppToV8Reference) { if (!FLAG_incremental_marking) return; + v8::HandleScope scope(v8_isolate()); - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); cppgc::Persistent wrappable = cppgc::MakeGarbageCollected(allocation_handle()); Wrappable::destructor_callcount = 0; @@ -119,7 +115,7 @@ TEST_F(UnifiedHeapTest, WriteBarrierCppToV8Reference) { // setter for C++ to JS references. v8::HandleScope nested_scope(v8_isolate()); v8::Local api_object = - WrapperHelper::CreateWrapper(context, nullptr, nullptr); + WrapperHelper::CreateWrapper(context(), nullptr, nullptr); // Setting only one field to avoid treating this as wrappable backref, see // `LocalEmbedderHeapTracer::ExtractWrapperInfo`. api_object->SetAlignedPointerInInternalField(1, kMagicAddress); @@ -148,8 +144,6 @@ class Unreferenced : public cppgc::GarbageCollected { TEST_F(UnifiedHeapTest, FreeUnreferencedDuringNoGcScope) { v8::HandleScope handle_scope(v8_isolate()); - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); auto* unreferenced = cppgc::MakeGarbageCollected( allocation_handle(), cppgc::AdditionalBytes(cppgc::internal::api_constants::kMB)); @@ -178,8 +172,6 @@ TEST_F(UnifiedHeapTest, FreeUnreferencedDuringNoGcScope) { #if !V8_OS_FUCHSIA TEST_F(UnifiedHeapTest, TracedReferenceRetainsFromStack) { v8::HandleScope handle_scope(v8_isolate()); - v8::Local context = v8::Context::New(v8_isolate()); - v8::Context::Scope context_scope(context); TracedReference holder; { v8::HandleScope inner_handle_scope(v8_isolate());