[bindings] Refactor wrapper-tracing tests

- Move temporary visitor scope to test infrastructure
- Factor out integration tests that are independent from visitors

Bug: chromium:843903
Change-Id: I3850cc48fe1b9186691722f3241e9de27cd0a29e
Reviewed-on: https://chromium-review.googlesource.com/1172294
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582572}

third_party/blink/renderer/bindings/bindings.gni

@@ -198,6 +198,7 @@ bindings_unittest_files =
           "core/v8/script_promise_test.cc",
           "core/v8/script_streamer_test.cc",
           "core/v8/script_wrappable_marking_visitor_test.cc",
+          "core/v8/script_wrappable_v8_gc_integration_test.cc",
           "core/v8/script_wrappable_visitor_test.cc",
           "core/v8/to_v8_test.cc",
           "core/v8/trace_wrapper_member_test.cc",

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

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.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/core/testing/death_aware_script_wrappable.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+namespace v8_gc_integration_test {
+
+void PreciselyCollectGarbage() {
+  ThreadState::Current()->CollectAllGarbage();
+}
+
+void RunV8Scavenger(v8::Isolate* isolate) {
+  V8GCController::CollectGarbage(isolate, true);
+}
+
+void RunV8FullGc(v8::Isolate* isolate) {
+  V8GCController::CollectGarbage(isolate, false);
+}
+
+template <typename T>
+class ObjectObserver {
+ public:
+  void Observe(T* object) { holder_ = object; }
+  bool ObjectDied() const { return nullptr == holder_.Get(); }
+
+ private:
+  WeakPersistent<T> holder_;
+};
+
+}  // namespace v8_gc_integration_test
+
+// =============================================================================
+// Tests that ScriptWrappable and its wrapper survive or are reclaimed in
+// certain garbage collection scenarios.
+// =============================================================================
+
+TEST(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringFullGc) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.GetIsolate();
+
+  v8::Persistent<v8::Value> holder;
+  v8_gc_integration_test::ObjectObserver<DeathAwareScriptWrappable> observer;
+  {
+    v8::HandleScope handle_scope(isolate);
+    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
+    observer.Observe(object);
+
+    holder.Reset(isolate, ToV8(object, scope.GetContext()->Global(), isolate));
+  }
+
+  v8_gc_integration_test::RunV8FullGc(isolate);
+  v8_gc_integration_test::PreciselyCollectGarbage();
+  EXPECT_FALSE(observer.ObjectDied());
+  holder.Reset();
+}
+
+TEST(ScriptWrappableV8GCIntegrationTest, V8ReportsLiveObjectsDuringScavenger) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.GetIsolate();
+
+  v8_gc_integration_test::ObjectObserver<DeathAwareScriptWrappable> observer;
+  {
+    v8::HandleScope handle_scope(isolate);
+    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
+    observer.Observe(object);
+
+    v8::Local<v8::Value> wrapper =
+        ToV8(object, scope.GetContext()->Global(), isolate);
+    EXPECT_TRUE(wrapper->IsObject());
+    v8::Local<v8::Object> wrapper_object =
+        wrapper->ToObject(scope.GetContext()).ToLocalChecked();
+    // V8 collects wrappers with unmodified maps (as they can be recreated
+    // without losing any data if needed). We need to create some property on
+    // wrapper so V8 will not see it as unmodified.
+    EXPECT_TRUE(
+        wrapper_object->CreateDataProperty(scope.GetContext(), 1, wrapper)
+            .IsJust());
+  }
+
+  // Scavenger should not collect JavaScript wrappers that are modified, even if
+  // they are otherwise unreachable.
+  v8_gc_integration_test::RunV8Scavenger(isolate);
+  v8_gc_integration_test::PreciselyCollectGarbage();
+
+  EXPECT_FALSE(observer.ObjectDied());
+}
+
+TEST(ScriptWrappableV8GCIntegrationTest,
+     OilpanDoesntCollectObjectsReachableFromV8) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.GetIsolate();
+
+  v8::Persistent<v8::Value> holder;
+  v8_gc_integration_test::ObjectObserver<DeathAwareScriptWrappable> observer;
+  {
+    v8::HandleScope handle_scope(isolate);
+    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));
+  }
+
+  v8_gc_integration_test::RunV8Scavenger(isolate);
+  v8_gc_integration_test::RunV8FullGc(isolate);
+  v8_gc_integration_test::PreciselyCollectGarbage();
+
+  EXPECT_FALSE(observer.ObjectDied());
+  holder.Reset();
+}
+
+TEST(ScriptWrappableV8GCIntegrationTest,
+     OilpanCollectObjectsNotReachableFromV8) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.GetIsolate();
+
+  v8_gc_integration_test::ObjectObserver<DeathAwareScriptWrappable> observer;
+  {
+    v8::HandleScope handle_scope(isolate);
+    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::Create();
+    observer.Observe(object);
+
+    // Creates new V8 wrapper and associates it with global scope
+    ToV8(object, scope.GetContext()->Global(), isolate);
+  }
+
+  v8_gc_integration_test::RunV8Scavenger(isolate);
+  v8_gc_integration_test::RunV8FullGc(isolate);
+  v8_gc_integration_test::PreciselyCollectGarbage();
+
+  EXPECT_TRUE(observer.ObjectDied());
+}
+
+}  // namespace blink

third_party/blink/renderer/platform/bindings/v8_per_isolate_data.cc

@@ -368,16 +368,4 @@ void V8PerIsolateData::AddActiveScriptWrappable(
   active_script_wrappables_->insert(wrappable);
 }
 
-void V8PerIsolateData::TemporaryScriptWrappableVisitorScope::
-    SwapWithV8PerIsolateDataVisitor(
-        std::unique_ptr<ScriptWrappableMarkingVisitor>& visitor) {
-  ScriptWrappableMarkingVisitor* current = CurrentVisitor();
-  if (current)
-    ScriptWrappableMarkingVisitor::PerformCleanup(isolate_);
-
-  V8PerIsolateData::From(isolate_)->script_wrappable_visitor_.swap(
-      saved_visitor_);
-  isolate_->SetEmbedderHeapTracer(CurrentVisitor());
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h

@@ -200,41 +200,20 @@ class PLATFORM_EXPORT V8PerIsolateData {
     return active_script_wrappables_.Get();
   }
 
-  class PLATFORM_EXPORT TemporaryScriptWrappableVisitorScope {
-    WTF_MAKE_NONCOPYABLE(TemporaryScriptWrappableVisitorScope);
-    STACK_ALLOCATED();
-
-   public:
-    TemporaryScriptWrappableVisitorScope(
-        v8::Isolate* isolate,
-        std::unique_ptr<ScriptWrappableMarkingVisitor> visitor)
-        : isolate_(isolate), saved_visitor_(std::move(visitor)) {
-      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
-    }
-    ~TemporaryScriptWrappableVisitorScope() {
-      SwapWithV8PerIsolateDataVisitor(saved_visitor_);
-    }
-
-    inline ScriptWrappableMarkingVisitor* CurrentVisitor() {
-      return V8PerIsolateData::From(isolate_)
-          ->GetScriptWrappableMarkingVisitor();
-    }
-
-   private:
-    void SwapWithV8PerIsolateDataVisitor(
-        std::unique_ptr<ScriptWrappableMarkingVisitor>&);
-
-    v8::Isolate* isolate_;
-    std::unique_ptr<ScriptWrappableMarkingVisitor> saved_visitor_;
-  };
-
   void SetScriptWrappableMarkingVisitor(
       std::unique_ptr<ScriptWrappableMarkingVisitor> visitor) {
     script_wrappable_visitor_ = std::move(visitor);
   }
-  ScriptWrappableMarkingVisitor* GetScriptWrappableMarkingVisitor() {
+
+  ScriptWrappableMarkingVisitor* GetScriptWrappableMarkingVisitor() const {
     return script_wrappable_visitor_.get();
   }
+
+  void SwapScriptWrappableMarkingVisitor(
+      std::unique_ptr<ScriptWrappableMarkingVisitor>& other) {
+    script_wrappable_visitor_.swap(other);
+  }
+
   int IsNearV8HeapLimitHandled() { return handled_near_v8_heap_limit_; }
 
   void HandledNearV8HeapLimit() { handled_near_v8_heap_limit_ = true; }