bindings: Provide BindingTestSupportingGC

- Provide BindingTestSupportingGC that can be used to call various types
  of GC.
- Avoiding including v8_binding_for_testing.h in production code.
- Fix clients that relied on transitively including V8TestingScope.

Change-Id: I64733749389817bbc3eda3cf90003a587e0bf33f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776362
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691997}

third_party/blink/renderer/bindings/core/v8/script_wrappable_v8_gc_integration_test.cc

@@ -12,66 +12,51 @@
 
 namespace blink {
 
-namespace v8_gc_integration_test {
+namespace {
 
-void PreciselyCollectGarbage() {
-  ThreadState::Current()->CollectAllGarbageForTesting();
-}
-
-// The following directly calls testing GCs in V8 to avoid cluttering a globally
-// visible interface with calls that have to be carefully staged.
-
-void RunV8MinorGC(v8::Isolate* isolate) {
-  CHECK(isolate);
-  isolate->RequestGarbageCollectionForTesting(
-      v8::Isolate::GarbageCollectionType::kMinorGarbageCollection);
-}
-
-void RunV8FullGCWithoutScanningOilpanStack(v8::Isolate* isolate) {
-  CHECK(isolate);
-  V8GCController::CollectAllGarbageForTesting(
-      isolate, v8::EmbedderHeapTracer::EmbedderStackState::kEmpty);
-}
+using ScriptWrappableV8GCIntegrationTest = BindingTestSupportingGC;
 
-}  // namespace v8_gc_integration_test
+}  // namespace
 
 // =============================================================================
 // Tests that ScriptWrappable and its wrapper survive or are reclaimed in
 // certain garbage collection scenarios.
 // =============================================================================
 
-TEST(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringFullGc) {
+TEST_F(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringFullGc) {
   V8TestingScope scope;
-  v8::Isolate* isolate = scope.GetIsolate();
+  SetIsolate(scope.GetIsolate());
 
   v8::Persistent<v8::Value> holder;
   GCObjectLivenessObserver<DeathAwareScriptWrappable> observer;
   {
-    v8::HandleScope handle_scope(isolate);
+    v8::HandleScope handle_scope(GetIsolate());
     DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
     observer.Observe(object);
 
-    holder.Reset(isolate, ToV8(object, scope.GetContext()->Global(), isolate));
+    holder.Reset(GetIsolate(),
+                 ToV8(object, scope.GetContext()->Global(), GetIsolate()));
   }
 
-  v8_gc_integration_test::RunV8MinorGC(isolate);
-  v8_gc_integration_test::PreciselyCollectGarbage();
+  RunV8MinorGC();
+  PreciselyCollectGarbage();
   EXPECT_FALSE(observer.WasCollected());
   holder.Reset();
 }
 
-TEST(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringScavenger) {
+TEST_F(ScriptWrappableV8GCIntegrationTest,
+       V8ReportsLiveObjectsDuringScavenger) {
   V8TestingScope scope;
-  v8::Isolate* isolate = scope.GetIsolate();
+  SetIsolate(scope.GetIsolate());
 
   GCObjectLivenessObserver<DeathAwareScriptWrappable> observer;
   {
-    v8::HandleScope handle_scope(isolate);
+    v8::HandleScope handle_scope(GetIsolate());
     DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
     observer.Observe(object);
 
     v8::Local<v8::Value> wrapper =
-        ToV8(object, scope.GetContext()->Global(), isolate);
+        ToV8(object, scope.GetContext()->Global(), GetIsolate());
     EXPECT_TRUE(wrapper->IsObject());
     v8::Local<v8::Object> wrapper_object =
         wrapper->ToObject(scope.GetContext()).ToLocalChecked();
@@ -85,54 +70,55 @@ TEST(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringScavenger) {
 
   // Scavenger should not collect JavaScript wrappers that are modified, even if
   // they are otherwise unreachable.
-  v8_gc_integration_test::RunV8MinorGC(isolate);
-  v8_gc_integration_test::PreciselyCollectGarbage();
+  RunV8MinorGC();
+  PreciselyCollectGarbage();
 
   EXPECT_FALSE(observer.WasCollected());
 }
 
-TEST(ScriptWrappableV8GCIntegrationTest,
-     OilpanDoesntCollectObjectsReachableFromV8) {
+TEST_F(ScriptWrappableV8GCIntegrationTest,
+       OilpanDoesntCollectObjectsReachableFromV8) {
   V8TestingScope scope;
-  v8::Isolate* isolate = scope.GetIsolate();
+  SetIsolate(scope.GetIsolate());
 
   v8::Persistent<v8::Value> holder;
   GCObjectLivenessObserver<DeathAwareScriptWrappable> observer;
   {
-    v8::HandleScope handle_scope(isolate);
+    v8::HandleScope handle_scope(GetIsolate());
     DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
     observer.Observe(object);
 
     // Creates new V8 wrapper and associates it with global scope
-    holder.Reset(isolate, ToV8(object, scope.GetContext()->Global(), isolate));
+    holder.Reset(GetIsolate(),
+                 ToV8(object, scope.GetContext()->Global(), GetIsolate()));
   }
 
-  v8_gc_integration_test::RunV8MinorGC(isolate);
-  v8_gc_integration_test::RunV8FullGCWithoutScanningOilpanStack(isolate);
-  v8_gc_integration_test::PreciselyCollectGarbage();
+  RunV8MinorGC();
+  RunV8FullGC();
+  PreciselyCollectGarbage();
 
   EXPECT_FALSE(observer.WasCollected());
   holder.Reset();
 }
 
-TEST(ScriptWrappableV8GCIntegrationTest,
-     OilpanCollectObjectsNotReachableFromV8) {
+TEST_F(ScriptWrappableV8GCIntegrationTest,
+       OilpanCollectObjectsNotReachableFromV8) {
   V8TestingScope scope;
-  v8::Isolate* isolate = scope.GetIsolate();
+  SetIsolate(scope.GetIsolate());
 
   GCObjectLivenessObserver<DeathAwareScriptWrappable> observer;
   {
-    v8::HandleScope handle_scope(isolate);
+    v8::HandleScope handle_scope(GetIsolate());
     DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
     observer.Observe(object);
 
     // Creates new V8 wrapper and associates it with global scope
-    ToV8(object, scope.GetContext()->Global(), isolate);
+    ToV8(object, scope.GetContext()->Global(), GetIsolate());
   }
 
-  v8_gc_integration_test::RunV8MinorGC(isolate);
-  v8_gc_integration_test::RunV8FullGCWithoutScanningOilpanStack(isolate);
-  v8_gc_integration_test::PreciselyCollectGarbage();
+  RunV8MinorGC();
+  RunV8FullGC();
+  PreciselyCollectGarbage();
 
   EXPECT_TRUE(observer.WasCollected());
 }

third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h

@@ -10,7 +10,6 @@
 
 #include "third_party/blink/renderer/bindings/core/v8/idl_dictionary_base.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_value.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/core/dom/node.h"
 #include "third_party/blink/renderer/core/typed_arrays/array_buffer_view_helpers.h"
 #include "third_party/blink/renderer/platform/bindings/to_v8.h"
@@ -75,11 +74,6 @@ inline ScriptValue ScriptValue::From(ScriptState* script_state, T&& value) {
   return ScriptValue(script_state, ToV8(std::forward<T>(value), script_state));
 }
 
-template <typename T>
-v8::Local<v8::Value> ToV8(V8TestingScope* scope, T value) {
-  return blink::ToV8(value, scope->GetContext()->Global(), scope->GetIsolate());
-}
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_TO_V8_FOR_CORE_H_

third_party/blink/renderer/bindings/core/v8/trace_wrapper_v8_reference_test.cc

@@ -6,26 +6,13 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_gc_controller.h"
 #include "third_party/blink/renderer/platform/heap/persistent.h"
 
 namespace blink {
 
 namespace {
 
-class TraceWrapperV8ReferenceTest : public testing::Test {
- public:
-  void SetIsolate(v8::Isolate* isolate) { isolate_ = isolate; }
-  v8::Isolate* GetIsolate() const { return isolate_; }
-
-  void CollectGarbage() {
-    V8GCController::CollectAllGarbageForTesting(
-        isolate_, v8::EmbedderHeapTracer::EmbedderStackState::kEmpty);
-  }
-
- private:
-  v8::Isolate* isolate_;
-};
+using TraceWrapperV8ReferenceTest = BindingTestSupportingGC;
 
 class TraceWrapperV8ReferenceHolder
     : public GarbageCollected<TraceWrapperV8ReferenceHolder> {
@@ -78,11 +65,11 @@ TEST_F(TraceWrapperV8ReferenceTest, CtorWithValue) {
 
   CHECK(!holder1->ref()->IsEmpty());
   CHECK(!observer.IsEmpty());
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(!holder1->ref()->IsEmpty());
   CHECK(!observer.IsEmpty());
   holder1->ref()->Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(holder1->ref()->IsEmpty());
   CHECK(observer.IsEmpty());
 }
@@ -103,16 +90,16 @@ TEST_F(TraceWrapperV8ReferenceTest, CopyOverEmpty) {
   CHECK(!holder1->ref()->IsEmpty());
   CHECK(*holder1->ref() == *holder2->ref());
   CHECK(!observer1.IsEmpty());
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(!holder1->ref()->IsEmpty());
   CHECK(*holder1->ref() == *holder2->ref());
   CHECK(!observer1.IsEmpty());
   holder1.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
   holder2.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(observer1.IsEmpty());
 }
 
@@ -136,18 +123,18 @@ TEST_F(TraceWrapperV8ReferenceTest, CopyOverNonEmpty) {
   CHECK(*holder1->ref() == *holder2->ref());
   CHECK(!observer1.IsEmpty());
   CHECK(!observer2.IsEmpty());
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(!holder1->ref()->IsEmpty());
   CHECK(*holder1->ref() == *holder2->ref());
   CHECK(!observer1.IsEmpty());
   // Old object in holder2 already gone.
   CHECK(observer2.IsEmpty());
   holder1.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
   holder2.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(observer1.IsEmpty());
 }
 
@@ -168,13 +155,13 @@ TEST_F(TraceWrapperV8ReferenceTest, MoveOverEmpty) {
   CHECK(holder1->ref()->IsEmpty());
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(holder1->ref()->IsEmpty());
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
   holder1.Clear();
   holder2.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(observer1.IsEmpty());
 }
 
@@ -199,14 +186,14 @@ TEST_F(TraceWrapperV8ReferenceTest, MoveOverNonEmpty) {
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
   CHECK(!observer2.IsEmpty());
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(holder1->ref()->IsEmpty());
   CHECK(!holder2->ref()->IsEmpty());
   CHECK(!observer1.IsEmpty());
   CHECK(observer2.IsEmpty());
   holder1.Clear();
   holder2.Clear();
-  CollectGarbage();
+  RunV8FullGC();
   CHECK(observer1.IsEmpty());
 }
 

third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h

@@ -5,6 +5,8 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_BINDING_FOR_TESTING_H_
 #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_BINDING_FOR_TESTING_H_
 
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_gc_controller.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
@@ -45,6 +47,40 @@ class V8TestingScope {
   DummyExceptionStateForTesting exception_state_;
 };
 
+// Similar to other ToV8 helpers in to_v8_for_core.h.
+template <typename T>
+v8::Local<v8::Value> ToV8(V8TestingScope* scope, T value) {
+  return blink::ToV8(value, scope->GetContext()->Global(), scope->GetIsolate());
+}
+
+// Test supporting different kinds of GCs.
+class BindingTestSupportingGC : public testing::Test {
+ public:
+  void SetIsolate(v8::Isolate* isolate) {
+    CHECK(isolate);
+    CHECK_EQ(isolate, ThreadState::Current()->GetIsolate());
+    isolate_ = isolate;
+  }
+  v8::Isolate* GetIsolate() const { return isolate_; }
+
+  void PreciselyCollectGarbage() {
+    ThreadState::Current()->CollectAllGarbageForTesting();
+  }
+
+  void RunV8MinorGC() {
+    isolate_->RequestGarbageCollectionForTesting(
+        v8::Isolate::GarbageCollectionType::kMinorGarbageCollection);
+  }
+
+  void RunV8FullGC(v8::EmbedderHeapTracer::EmbedderStackState stack_state =
+                       v8::EmbedderHeapTracer::EmbedderStackState::kEmpty) {
+    V8GCController::CollectAllGarbageForTesting(isolate_, stack_state);
+  }
+
+ private:
+  v8::Isolate* isolate_;
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_BINDING_FOR_TESTING_H_

third_party/blink/renderer/core/dom/element.h

@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/core/html_names.h"
 #include "third_party/blink/renderer/core/trustedtypes/trusted_types_util.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/wtf/hash_set.h"
 
 namespace blink {

third_party/blink/renderer/core/frame/web_frame_widget_base.h

@@ -31,6 +31,7 @@ namespace blink {
 class AnimationWorkletMutatorDispatcherImpl;
 class GraphicsLayer;
 class HitTestResult;
+class Page;
 class PageWidgetEventHandler;
 class PaintWorkletPaintDispatcher;
 class WebLocalFrameImpl;

third_party/blink/renderer/core/layout/layout_text_fragment_test.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/layout/layout_text_fragment.h"
 
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/core/dom/first_letter_pseudo_element.h"
 #include "third_party/blink/renderer/core/html/html_head_element.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"

third_party/blink/renderer/core/trustedtypes/trusted_types_util_test.cc

@@ -11,6 +11,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/string_or_trusted_script.h"
 #include "third_party/blink/renderer/bindings/core/v8/string_or_trusted_script_url.h"
 #include "third_party/blink/renderer/bindings/core/v8/usv_string_or_trusted_url.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"

third_party/blink/renderer/modules/indexeddb/idb_factory_test.cc

@@ -16,6 +16,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_database_error.h"
 #include "third_party/blink/renderer/modules/indexeddb/idb_name_and_version.h"

third_party/blink/renderer/modules/payments/payment_test_helper.h

@@ -8,6 +8,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/mojom/payments/payment_request.mojom-blink.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_function.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_dom_exception.h"
 #include "third_party/blink/renderer/modules/payments/payment_details_init.h"
 #include "third_party/blink/renderer/modules/payments/payment_details_update.h"

third_party/blink/renderer/modules/peerconnection/rtc_ice_transport_test.cc

@@ -9,6 +9,7 @@
 
 #include "third_party/blink/renderer/modules/peerconnection/rtc_ice_transport_test.h"
 
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/modules/peerconnection/adapters/test/mock_ice_transport_adapter_cross_thread_factory.h"
 #include "third_party/blink/renderer/modules/peerconnection/adapters/test/mock_p2p_quic_packet_transport.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_ice_candidate.h"

third_party/blink/renderer/modules/peerconnection/rtc_ice_transport_test.h

@@ -13,6 +13,8 @@
 
 namespace blink {
 
+class V8TestingScope;
+
 class MockEventListener final : public NativeEventListener {
  public:
   MOCK_METHOD2(Invoke, void(ExecutionContext*, Event*));

third_party/blink/renderer/modules/peerconnection/rtc_quic_stream_test.cc

@@ -9,6 +9,7 @@
 
 #include "third_party/blink/renderer/modules/peerconnection/rtc_quic_stream.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/modules/peerconnection/adapters/test/mock_p2p_quic_stream.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_quic_stream_event.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_quic_transport_test.h"

third_party/blink/renderer/modules/peerconnection/rtc_quic_transport_test.cc

@@ -11,6 +11,7 @@
 #include "base/containers/span.h"
 #include "third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_array_buffer.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_rtc_quic_transport_stats.h"
 #include "third_party/blink/renderer/core/dom/dom_high_res_time_stamp.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"

third_party/blink/renderer/modules/peerconnection/rtc_quic_transport_test.h

@@ -13,6 +13,8 @@
 
 namespace blink {
 
+class V8TestingScope;
+
 class RTCQuicTransportTest : public RTCIceTransportTest {
  public:
   // Construct a new RTCQuicTransport with the given RTCIceTransport,