v8binding: Initializes WindowProxy iff it's uninitialized.

third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp

Index: third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
index 761b4a5c50260087de64d90bd90070998e813310..2247de0f1770fd78d34b804ffd98c6e4ca842da2 100644
--- a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
@@ -45,7 +45,7 @@ namespace blink {
 WindowProxy::~WindowProxy() {
   // clearForClose() or clearForNavigation() must be invoked before destruction
   // starts.
-  DCHECK(m_lifecycle != Lifecycle::ContextInitialized);
+  DCHECK(m_lifecycle != Lifecycle::ContextIsInitialized);
 }
 
 DEFINE_TRACE(WindowProxy) {
@@ -59,7 +59,7 @@ WindowProxy::WindowProxy(v8::Isolate* isolate,
       m_frame(frame),
 
       m_world(std::move(world)),
-      m_lifecycle(Lifecycle::ContextUninitialized) {}
+      m_lifecycle(Lifecycle::ContextIsUninitialized) {}
 
 void WindowProxy::clearForClose() {
   disposeContext(DoNotDetachGlobal);
@@ -70,7 +70,7 @@ void WindowProxy::clearForNavigation() {
 }
 
 v8::Local<v8::Object> WindowProxy::globalIfNotDetached() {
-  if (m_lifecycle == Lifecycle::ContextInitialized) {
+  if (m_lifecycle == Lifecycle::ContextIsInitialized) {
     DLOG_IF(FATAL, !m_isGlobalObjectAttached)
         << "Context is initialized but global object is detached!";
     return m_globalProxy.newLocal(m_isolate);
@@ -79,7 +79,7 @@ v8::Local<v8::Object> WindowProxy::globalIfNotDetached() {
 }
 
 v8::Local<v8::Object> WindowProxy::releaseGlobal() {
-  DCHECK(m_lifecycle != Lifecycle::ContextInitialized);
+  DCHECK(m_lifecycle != Lifecycle::ContextIsInitialized);
 
   // Make sure the global object was detached from the proxy by calling
   // clearForNavigation().
@@ -140,9 +140,8 @@ void WindowProxy::setGlobal(v8::Local<v8::Object> global) {
 // If there are JS code holds a closure to the old inner window,
 // it won't be able to reach the outer window via its global object.
 void WindowProxy::initializeIfNeeded() {
-  // TODO(haraken): It is wrong to re-initialize an already detached window
-  // proxy. This must be 'if(m_lifecycle == Lifecycle::ContextUninitialized)'.
-  if (m_lifecycle != Lifecycle::ContextInitialized) {
+  if (m_lifecycle == Lifecycle::ContextIsUninitialized ||
+      m_lifecycle == Lifecycle::GlobalObjectIsDetached) {

[dcheng, 2017/03/30 21:03:01]

I wonder how often we actually take advantage of the GlobalObjectIsDetached
path. In theory, I think that LocalDOMWindow::installNewDocument() will always
reinitialize, so maybe we can just DCHECK(m_lifecycle !=
Lifecycle::GlobalObjectIsDetached).

Maybe something to investigate in a followup.

[Yuki, 2017/03/31 07:32:09]

Yes, I'll address this point separately.

As I quickly checked, when swapping frames, the following happens.

WebFrame::swap() {
  // This made m_lifecycle = GlobalObjectIsDetached.
  windowProxyManager->clearForNavigation();

  windowProxyManager->releaseGlobals();

  windowProxyManager->setGlobals();
}

WindowProxy::setGlobal(global) {
  m_globalProxy.set(global);

  initializeIfNeeded();  // [1] Reinitialization HERE!
}

It seems we're entering to initializeIfNeeded with m_lifecycle =
GlobalObjectIsDetached.

If you guys agree, I will make it:

WindowProxy::setGlobal(global) {
  m_globalProxy.set(global);
  DCHECK_EQ(m_lifecycle, GloablObjectIsDetached);
  initialize();
}

then, we may be able to remove GlobalObjectIsDetached case from
initializeIfNeeded().

     initialize();
   }
 }