v8binding: Initializes WindowProxy iff it's uninitialized.

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

 /*
  * Copyright (C) 2008, 2009, 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 27 matching lines @@
 #include "core/frame/DOMWindow.h"
 #include "core/frame/Frame.h"
 #include "v8/include/v8.h"
 #include "wtf/Assertions.h"
 
 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) {
   visitor->trace(m_frame);
 }
 
 WindowProxy::WindowProxy(v8::Isolate* isolate,
                          Frame& frame,
                          RefPtr<DOMWrapperWorld> world)
     : m_isolate(isolate),
       m_frame(frame),
 
       m_world(std::move(world)),
-      m_lifecycle(Lifecycle::ContextUninitialized) {}
+      m_lifecycle(Lifecycle::ContextIsUninitialized) {}
 
 void WindowProxy::clearForClose() {
   disposeContext(DoNotDetachGlobal);
 }
 
 void WindowProxy::clearForNavigation() {
   disposeContext(DetachGlobal);
 }
 
 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);
   }
   return v8::Local<v8::Object>();
 }
 
 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().
   DLOG_IF(FATAL, m_isGlobalObjectAttached)
       << "Context not detached by calling clearForNavigation()";
 
   v8::Local<v8::Object> global = m_globalProxy.newLocal(m_isolate);
   m_globalProxy.clear();
   return global;
 }
@@ skipping 40 matching lines @@
 // has a security token which is the domain. The outer window cannot
 // have its own properties. window.foo = 'x' is delegated to the
 // inner window.
 //
 // When a frame navigates to a new page, the inner window is cut off
 // the outer window, and the outer window identify is preserved for
 // the frame. However, a new inner window is created for the new page.
 // 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();
   }
 }
 
 v8::Local<v8::Object> WindowProxy::associateWithWrapper(
     DOMWindow* window,
     const WrapperTypeInfo* wrapperTypeInfo,
     v8::Local<v8::Object> wrapper) {
   if (m_world->domDataStore().set(m_isolate, window, wrapperTypeInfo,
                                   wrapper)) {
     wrapperTypeInfo->wrapperCreated();
     V8DOMWrapper::setNativeInfo(m_isolate, wrapper, wrapperTypeInfo, window);
     DCHECK(V8DOMWrapper::hasInternalFieldsSet(wrapper));
   }
   SECURITY_CHECK(toScriptWrappable(wrapper) == window);
   return wrapper;
 }
 
 }  // namespace blink