Trace `WindowProxy::global_proxy_`

Originally, this field was a ScopedPersistent, making it a strong GC
root from Blink to v8::Context. When the context was disposed (e.g. when
detaching the frame or purging v8), it was important to convert it to a
weak GC root with SetPhantom(). Otherwise, even if WindowProxy itself
was no longer reachable via Blink C++ objects, the strong V8 reference
could create a reference cycle preventing GC of both Blink and v8
objects.

   WindowProxy -> global_proxy_ -> v8::Context -> JS object
       -> chain of Blink C++ objects -> WindowProxy

With unified tracing, global_proxy_ can simply be a traced v8 reference.
Once WindowProxy is no longer reachable from a GC root (typically via
WebFrame (retained via a SelfKeepAlive) -> Frame -> WindowProxy),
WindowProxy and the associated v8::Context become collectible with no
need to explitily break a cycle.

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

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

@@ -69,12 +69,6 @@
 
 namespace blink {
 
-namespace {
-
-constexpr char kGlobalProxyLabel[] = "WindowProxy::global_proxy_";
-
-}  // namespace
-
 void LocalWindowProxy::Trace(Visitor* visitor) const {
   visitor->Trace(script_state_);
   WindowProxy::Trace(visitor);
@@ -94,8 +88,6 @@ void LocalWindowProxy::DisposeContext(Lifecycle next_status,
   if (lifecycle_ == Lifecycle::kV8MemoryIsForciblyPurged) {
     DCHECK(next_status == Lifecycle::kGlobalObjectIsDetached ||
            next_status == Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged);
-    if (next_status == Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged)
-      global_proxy_.SetPhantom();
     lifecycle_ = next_status;
     return;
   }
@@ -114,7 +106,7 @@ void LocalWindowProxy::DisposeContext(Lifecycle next_status,
       next_status == Lifecycle::kGlobalObjectIsDetached) {
     // Clean up state on the global proxy, which will be reused.
     if (!global_proxy_.IsEmpty()) {
-      CHECK(global_proxy_ == context->Global());
+      CHECK(global_proxy_.Get() == context->Global());
       CHECK_EQ(ToScriptWrappable(context->Global()),
                ToScriptWrappable(
                    context->Global()->GetPrototype().As<v8::Object>()));
@@ -135,12 +127,6 @@ void LocalWindowProxy::DisposeContext(Lifecycle next_status,
   V8GCForContextDispose::Instance().NotifyContextDisposed(
       GetFrame()->IsMainFrame(), frame_reuse_status);
 
-  if (next_status == Lifecycle::kFrameIsDetached) {
-    // The context's frame is detached from the DOM, so there shouldn't be a
-    // strong reference to the context.
-    global_proxy_.SetPhantom();
-  }
-
   DCHECK_EQ(lifecycle_, Lifecycle::kContextIsInitialized);
   lifecycle_ = next_status;
 }
@@ -159,7 +145,6 @@ void LocalWindowProxy::Initialize() {
   v8::Local<v8::Context> context = script_state_->GetContext();
   if (global_proxy_.IsEmpty()) {
     global_proxy_.Set(GetIsolate(), context->Global());
-    global_proxy_.Get().AnnotateStrongRetainer(kGlobalProxyLabel);
     CHECK(!global_proxy_.IsEmpty());
   }
 
@@ -319,7 +304,7 @@ void LocalWindowProxy::SetupWindowPrototypeChain() {
 
   // The global proxy object.  Note this is not the global object.
   v8::Local<v8::Object> global_proxy = context->Global();
-  CHECK(global_proxy_ == global_proxy);
+  CHECK(global_proxy_.Get() == global_proxy);
   V8DOMWrapper::SetNativeInfo(GetIsolate(), global_proxy, wrapper_type_info,
                               window);
   // Mark the handle to be traced by Oilpan, since the global proxy has a

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

@@ -62,8 +62,6 @@ void RemoteWindowProxy::DisposeContext(Lifecycle next_status,
   if (lifecycle_ == Lifecycle::kV8MemoryIsForciblyPurged) {
     DCHECK(next_status == Lifecycle::kGlobalObjectIsDetached ||
            next_status == Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged);
-    if (next_status == Lifecycle::kFrameIsDetachedAndV8MemoryIsPurged)
-      global_proxy_.SetPhantom();
     lifecycle_ = next_status;
     return;
   }
@@ -83,12 +81,6 @@ void RemoteWindowProxy::DisposeContext(Lifecycle next_status,
 #endif
   }
 
-  if (next_status == Lifecycle::kFrameIsDetached) {
-    // The context's frame is detached from the DOM, so there shouldn't be a
-    // strong reference to the context.
-    global_proxy_.SetPhantom();
-  }
-
   DCHECK_EQ(lifecycle_, Lifecycle::kContextIsInitialized);
   lifecycle_ = next_status;
 }

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

@@ -50,6 +50,7 @@ WindowProxy::~WindowProxy() {
 
 void WindowProxy::Trace(Visitor* visitor) const {
   visitor->Trace(frame_);
+  visitor->Trace(global_proxy_);
 }
 
 WindowProxy::WindowProxy(v8::Isolate* isolate,

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

@@ -34,7 +34,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"
-#include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
+#include "third_party/blink/renderer/platform/bindings/trace_wrapper_v8_reference.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "v8/include/v8.h"
 
@@ -277,11 +277,20 @@ class WindowProxy : public GarbageCollected<WindowProxy> {
  protected:
   // TODO(dcheng): Consider making these private and using getters.
   const scoped_refptr<DOMWrapperWorld> world_;
-  // |global_proxy_| is the root reference from Blink to v8::Context (a strong
-  // reference to the global proxy makes the entire context alive).  In order to
-  // discard the v8::Context, |global_proxy_| needs to be a weak reference or
-  // to be destroyed.
-  ScopedPersistent<v8::Object> global_proxy_;
+  // Note: this used to be a ScopedPersistent, making `global_proxy_` a strong
+  // GC root from Blink to v8::Context. When the context was disposed (e.g. when
+  // detaching the frame or purging v8), it was important to call `SetPhantom()`
+  // to convert `global_proxy_` to a weak GC root. Otherwise, even if
+  // `WindowProxy` was no longer reachable via Blink GC, the strong reference
+  // could create a reference cycle preventing GC of both Blink and v8 objects:
+  //
+  //   WindowProxy -> global_proxy_ -> v8::Context -> JS object
+  //       -> chain of Blink C++ objects -> WindowProxy
+  //
+  // With unified tracing, `global_proxy_` can simply be a traced v8 reference.
+  // Once `WindowProxy` is no longer reachable from a GC root, `global_proxy_`
+  // will also become collectible with no need to explicitly break a cycle.
+  TraceWrapperV8Reference<v8::Object> global_proxy_;
   Lifecycle lifecycle_;
 };