v8binding: Makes ToV8(window) work without a frame.

third_party/WebKit/Source/core/frame/DOMWindow.h

Index: third_party/WebKit/Source/core/frame/DOMWindow.h
diff --git a/third_party/WebKit/Source/core/frame/DOMWindow.h b/third_party/WebKit/Source/core/frame/DOMWindow.h
index 8099170901faa6df18ec4a66e4db9290d6d6c181..ec56e4224e8791e42442675a40b2848efa3fccff 100644
--- a/third_party/WebKit/Source/core/frame/DOMWindow.h
+++ b/third_party/WebKit/Source/core/frame/DOMWindow.h
@@ -18,10 +18,11 @@ namespace blink {
 
 class Document;
 class InputDeviceCapabilitiesConstants;
-class Location;
 class LocalDOMWindow;
+class Location;
 class MessageEvent;
 class SerializedScriptValue;
+class WindowProxyManager;
 
 class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
                               public DOMWindowBase64 {
@@ -108,6 +109,8 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
 
   bool isSecureContext() const;
 
+  v8::Local<v8::Object> globalProxy(DOMWrapperWorld&);
+
   InputDeviceCapabilitiesConstants* getInputDeviceCapabilities();
 
  protected:
@@ -121,6 +124,34 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
 
  private:
   Member<Frame> m_frame;
+  // Unlike |m_frame|, |m_windowProxyManager| is available even after the
+  // window's frame gets detached from the DOM, until the end of the lifetime
+  // of this object.

[haraken, 2017/04/06 17:25:27]

I don't fully understand why DOMWindow needs to hold m_windowProxyManager. If
the window's frame is detached, DOMWindow::globalProxy() is expected to return
an empty handle. Then wouldn't it be enough to hold m_frame and return a valid
global proxy only when m_frame is not null?

[dcheng, 2017/04/07 06:23:12]

Right now it does, but maybe it shouldn't. According to the spec in
https://html.spec.whatwg.org/multipage/browsers.html#dom-self: The window,
frames, and self IDL attributes, on getting, must all return this Window
object's browsing context's WindowProxy object.

The spec is not clear if a detached Window still has a browsing context.

(I guess you could argue that the window doesn't have a browsing context once
it's removed; however, this is inconsistent from the behavior of Firefox. Edge
seems to throw an error in this case)

[Yuki, 2017/04/07 08:40:19]

My understanding is that we agreed that we need to support a browsing context in
a detached iframe mainly because author script can have references to it and
there is no way to avoid it.

Having said that, we still have an option to return null for win.self where
|win| is a saved reference.  However, as dcheng@ wrote, the spec requires that
|self| always returns non-null WindowProxy (of the spec, not
blink::WindowProxy).
I asked this point before:
https://github.com/whatwg/html/issues/1279

And at that time, IIUC, the answer is that we should maintain the browsing
context as long as author script has a reference to it.  Note that, in the spec,
"nested browsing context" is a different concept from "browsing context".  IIUC,
"nested browsing context" is a kind of slot to store a reference to a browsing
context or null.

Also there is WPT for detached cases.

In short, we need to support win.self, etc. in a detached iframe.

+  //
+  // Note that |m_windowProxyManager| is always available despite of WeakMember
+  // because this window object is unreachable when |m_windowProxyManager| gets
+  // collected.
+  // - |m_windowProxyManager| is unreachable.
+  // - All WindowProxies in |m_windowProxyManager| are unreachable.
+  // - One of the WindowProxies is the WindowProxy for this window object, and
+  //   it's unreachable.
+  // - So, this window object is unreachable, too.
+  // Thus, if this window object is reachable, |m_windowProxyManager| is still
+  // available.

[dcheng, 2017/04/07 18:48:48]

Thinking about this more, I wonder if this is guaranteed.

If we have something like this:

parentWindow = window.parent;
parentWindow.savedClosure = function () { parentWindow.console.log(document); };

And then we remove the child iframe from the DOM and wait for GC, what would
keep WindowProxyManager live? The frame is gone in that case, right? In that
case, what stops WindowProxyManager from being collected?

[Yuki, 2017/04/10 09:43:32]

You're right.  Let me change the approach overall.

+  //
+  // The reason why we cannot make |m_windowProxyManager| non-weak Member is
+  // because WindowProxy has |m_globalProxy| as a ScopedPersistent.  We need

[haraken, 2017/04/06 17:25:27]

Can we break the cycle by replacing the ScopedPersistent with
TraceWrapperV8Reference?

[Yuki, 2017/04/07 08:40:19]

I don't fully understand this point and I'm nore sure if it's possible or not at
this moment.

I thought that WindowProxy::m_globalProxy is the root reference.  If so,
TraceWrapperV8Reference wouldn't work.  IIUC, ScriptState::m_context is a weak
reference, so it cannot be the root reference.

I think that, if we don't have a root reference, we cannot keep a browsing
context alive.

[haraken, 2017/04/07 10:08:58]

Ah, makes sense.

However, I don't fully understand why using a Member causes a leak. When a frame
navigates to a new window, the reference from m_globalProxy to the old global
object will be broken. Then:

1) A V8 GC collects the old global object (because it is unreachable from
m_globalProxy).
2) An oilpan GC collects the corresponding C++ DOMWindow, WindowProxyManager etc
(even if we use a Member here).

[Yuki, 2017/04/07 10:30:44]

We have another case: iframe is detacehd from the DOM.

In this case, m_globalProxy still points to the same global object.

+  // the following steps in order to collect the window object.
+  // 1. Blink GC collects |m_windowProxyManager| and its WindowProxies.
+  // 2. WindowProxy's |m_globalProxy| (v8::Persistent) is destroyed.
+  //    Note that |m_globalProxy| had a strong reference to the global proxy.
+  // 3. V8 GC collects the global proxy, global object, and context.
+  //    Note that the global proxy and global object had strong references to
+  //    this DOMWindow object.
+  // 4. Blink GC collects this window object.
+  // Non-weak Member makes a cycle between Blink GC and V8 GC and currently
+  // the two GC systems cannot detect it as collectable.
+  const WeakMember<WindowProxyManager> m_windowProxyManager;
   Member<InputDeviceCapabilitiesConstants> m_inputCapabilities;
   mutable Member<Location> m_location;