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..84d79feee243814bf36a9bca30c2d610335055f5 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(frame.widget(), windowRect);
     host->chrome().show(policy);
 
-    frame.loader().forceSandboxFlags(openerFrame.document()->sandboxFlags());
+    // FIXME: There's currently no way to set sandbox flags on a RemoteFrame and have it propagate
+    // to the real frame in a different process.
+    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();

[Nate Chapin, 2015/04/28 23:25:51]

I can't find any reason this is actually necessary.

-
     // 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()) {
+    // FIXME: 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?
+    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());

[Nate Chapin, 2015/04/28 23:25:51]

The test that was added when this line was added still passes with out it, so I
believe it is vestigial.

-    }
+    if (shouldSendReferrer == MaybeSendReferrer)
+        newFrame->client()->setOpener(&openerFrame);
+
+    // FIXME: 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