Make createWindow (mostly) work with OOPIF

Source/core/page/CreateWindow.cpp

Index: Source/core/page/CreateWindow.cpp
diff --git a/Source/core/page/CreateWindow.cpp b/Source/core/page/CreateWindow.cpp
index c2329edc91b41b680201896fbb7f2f7721548550..8ce3329c9302c5eb417b85da47e03682dea717b8 100644
--- a/Source/core/page/CreateWindow.cpp
+++ b/Source/core/page/CreateWindow.cpp
@@ -28,6 +28,7 @@
 #include "core/page/CreateWindow.h"
 
 #include "core/dom/Document.h"
+#include "core/frame/FrameClient.h"
 #include "core/frame/FrameHost.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
@@ -47,7 +48,7 @@
 
 namespace blink {
 
-static LocalFrame* createWindow(LocalFrame& openerFrame, LocalFrame& lookupFrame, const FrameLoadRequest& request, const WindowFeatures& features, NavigationPolicy policy, ShouldSendReferrer shouldSendReferrer, bool& created)
+static Frame* createWindow(LocalFrame& openerFrame, LocalFrame& lookupFrame, const FrameLoadRequest& request, const WindowFeatures& features, NavigationPolicy policy, ShouldSendReferrer shouldSendReferrer)
 {
     ASSERT(!features.dialog || request.frameName().isEmpty());
     ASSERT(request.resourceRequest().frameType() == WebURLRequest::FrameTypeAuxiliary);
@@ -62,9 +63,7 @@ static LocalFrame* createWindow(LocalFrame& openerFrame, LocalFrame& lookupFrame
                         host->chrome().focus();
                 }
             }
-            created = false;
-            // FIXME: Make this work with RemoteFrames.
-            return frame->isLocalFrame() ? toLocalFrame(frame) : nullptr;
+            return frame;
         }
     }
 
@@ -75,24 +74,20 @@ static LocalFrame* createWindow(LocalFrame& openerFrame, LocalFrame& lookupFrame
         return nullptr;
     }
 
-    if (openerFrame.settings() && !openerFrame.settings()->supportsMultipleWindows()) {
-        created = false;
-        if (!openerFrame.tree().top()->isLocalFrame())
-            return nullptr;
-        return toLocalFrame(openerFrame.tree().top());
-    }
+    if (openerFrame.settings() && !openerFrame.settings()->supportsMultipleWindows())
+        return openerFrame.tree().top();
 
-    Page* oldPage = openerFrame.page();
-    if (!oldPage)
+    FrameHost* oldHost = openerFrame.host();
+    if (!oldHost)
         return nullptr;
 
-    Page* page = oldPage->chrome().client().createWindow(&openerFrame, request, features, policy, shouldSendReferrer);
-    if (!page || !page->mainFrame()->isLocalFrame())
+    Page* page = oldHost->chrome().client().createWindow(&openerFrame, request, features, policy, shouldSendReferrer);
+    if (!page)
         return nullptr;
     FrameHost* host = &page->frameHost();
 
     ASSERT(page->mainFrame());
-    LocalFrame& frame = *page->deprecatedLocalMainFrame();
+    Frame& frame = *page->mainFrame();
 
     if (request.frameName() != "_blank")
         frame.tree().setName(request.frameName());
@@ -115,19 +110,18 @@ static LocalFrame* createWindow(LocalFrame& openerFrame, LocalFrame& lookupFrame
     if (features.heightSet)
         windowRect.setHeight(features.height + (windowRect.height() - viewportSize.height()));
 
-    // Ensure minimum size as well as being within valid screen area.
-    IntRect newWindowRect = LocalDOMWindow::adjustWindowRect(frame, windowRect);
-
-    host->chrome().setWindowRect(newWindowRect);
+    host->chrome().setWindowRect(windowRect);
     host->chrome().show(policy);
 
-    frame.loader().forceSandboxFlags(openerFrame.document()->sandboxFlags());
+    // TODO(japhet): There's currently no way to set sandbox flags on a RemoteFrame and have it propagate
+    // to the real frame in a different process. See crbug.com/483584.
+    if (frame.isLocalFrame())
+        toLocalFrame(&frame)->loader().forceSandboxFlags(openerFrame.document()->sandboxFlags());
 
-    created = true;
     return &frame;
 }
 
-LocalFrame* createWindow(const String& urlString, const AtomicString& frameName, const WindowFeatures& windowFeatures,
+Frame* createWindow(const String& urlString, const AtomicString& frameName, const WindowFeatures& windowFeatures,
     LocalDOMWindow& callingWindow, LocalFrame& firstFrame, LocalFrame& openerFrame)
 {
     LocalFrame* activeFrame = callingWindow.frame();
@@ -150,27 +144,25 @@ LocalFrame* createWindow(const String& urlString, const AtomicString& frameName,
     // so we need to ensure the proper referrer is set now.
     frameRequest.resourceRequest().setHTTPReferrer(SecurityPolicy::generateReferrer(activeFrame->document()->referrerPolicy(), completedURL, activeFrame->document()->outgoingReferrer()));
 
-    bool hasUserGesture = UserGestureIndicator::processingUserGesture();

[dcheng, 2015/05/04 17:38:11]

I think this might be plumbed through into a few places in Chrome. I looked at
where
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
was called, and traced several locations through. One example of where it
appears to get used is
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

[Nate Chapin, 2015/05/04 18:09:13]

FrameLoader::load() will call setHasUserGesture() and should catch all
same-process cases. It's not clear to me what to do about user gestures for
RemoteFrames.

[dcheng, 2015/05/05 21:54:45]

Hmm... I guess as long as it gets set on the ResourceRequest that the embedder
eventually processes, and we correctly persist this on process swaps, we should
be fine.

Do you happen to know if this path is covered by layout tests?

-
     // We pass the opener frame for the lookupFrame in case the active frame is different from
     // the opener frame, and the name references a frame relative to the opener frame.
-    bool created;
-    LocalFrame* newFrame = createWindow(*activeFrame, openerFrame, frameRequest, windowFeatures, NavigationPolicyIgnore, MaybeSendReferrer, created);
+    Frame* newFrame = createWindow(*activeFrame, openerFrame, frameRequest, windowFeatures, NavigationPolicyIgnore, MaybeSendReferrer);
     if (!newFrame)
         return nullptr;
 
-    newFrame->loader().setOpener(&openerFrame);
+    newFrame->client()->setOpener(&openerFrame);
 
-    if (newFrame->localDOMWindow()->isInsecureScriptAccess(callingWindow, completedURL))
+    if (newFrame->domWindow()->isInsecureScriptAccess(callingWindow, completedURL))
         return newFrame;
 
-    if (created) {
-        FrameLoadRequest request(callingWindow.document(), completedURL);
-        request.resourceRequest().setHasUserGesture(hasUserGesture);
-        newFrame->loader().load(request);
-    } else if (!urlString.isEmpty()) {
+    // TODO(dcheng): Special case for window.open("about:blank") to ensure it loads synchronously into
+    // a new window. This is our historical behavior, and it's consistent with the creation of
+    // a new iframe with src="about:blank". Perhaps we could get rid of this if we started reporting
+    // the initial empty document's url as about:blank? See crbug.com/471239.
+    if (newFrame->isLocalFrame() && newFrame->isMainFrame() && !toLocalFrame(newFrame)->loader().stateMachine()->committedFirstRealDocumentLoad() && (completedURL.isEmpty() || completedURL.isAboutBlankURL()))
+        toLocalFrame(newFrame)->loader().load(FrameLoadRequest(callingWindow.document(), completedURL));
+    else
         newFrame->navigate(*callingWindow.document(), completedURL, false);
-    }
     return newFrame;
 }
 
@@ -189,17 +181,17 @@ void createWindowForRequest(const FrameLoadRequest& request, LocalFrame& openerF
         policy = NavigationPolicyNewForegroundTab;
 
     WindowFeatures features;
-    bool created;
-    LocalFrame* newFrame = createWindow(openerFrame, openerFrame, request, features, policy, shouldSendReferrer, created);
+    Frame* newFrame = createWindow(openerFrame, openerFrame, request, features, policy, shouldSendReferrer);
     if (!newFrame)
         return;
-    if (shouldSendReferrer == MaybeSendReferrer) {
-        newFrame->loader().setOpener(&openerFrame);
-        newFrame->document()->setReferrerPolicy(openerFrame.document()->referrerPolicy());
-    }
+    if (shouldSendReferrer == MaybeSendReferrer)
+        newFrame->client()->setOpener(&openerFrame);
+
+    // TODO(japhet): Form submissions on RemoteFrames don't work yet.
     FrameLoadRequest newRequest(0, request.resourceRequest());
     newRequest.setFormState(request.formState());
-    newFrame->loader().load(newRequest);
+    if (newFrame->isLocalFrame())
+        toLocalFrame(newFrame)->loader().load(newRequest);
 }
 
 } // namespace blink