REGRESSION (r219013): Compute source frame info for frameless document
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jul 2017 22:27:52 +0000 (22:27 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jul 2017 22:27:52 +0000 (22:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174385
<rdar://problem/33217736>

Reviewed by Brady Eidson.

Source/WebKit:

Fixes an issue where we would crash in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
when computing the frame info for a now-frameless document. One way this can happen is when the frame
that contains the document that initiated the navigation is removed from the page.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction): Check that we have a valid page ID before
looking up the WebPage object corresponding to it.
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): Compute a FrameInfoData
object from the source document directly as opposed to using WebFrame::info() as the latter requires
that we have a valid WebCore frame and the source document may not have a frame.

LayoutTests:

* http/tests/navigation/resources/window-open-redirect-and-remove-opener.html: Added.
* http/tests/navigation/window-open-redirect-and-remove-opener-expected.txt: Added.
* http/tests/navigation/window-open-redirect-and-remove-opener.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219528 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/navigation/resources/window-open-redirect-and-remove-opener.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

index 91ee1d1..1417085 100644 (file)
@@ -1,3 +1,15 @@
+2017-07-14  Daniel Bates  <dabates@apple.com>
+
+        REGRESSION (r219013): Compute source frame info for frameless document
+        https://bugs.webkit.org/show_bug.cgi?id=174385
+        <rdar://problem/33217736>
+
+        Reviewed by Brady Eidson.
+
+        * http/tests/navigation/resources/window-open-redirect-and-remove-opener.html: Added.
+        * http/tests/navigation/window-open-redirect-and-remove-opener-expected.txt: Added.
+        * http/tests/navigation/window-open-redirect-and-remove-opener.html: Added.
+
 2017-07-14  Matt Lewis  <jlewis3@apple.com>
 
         Fixed expectations after Mac expectation change.
diff --git a/LayoutTests/http/tests/navigation/resources/window-open-redirect-and-remove-opener.html b/LayoutTests/http/tests/navigation/resources/window-open-redirect-and-remove-opener.html
new file mode 100644 (file)
index 0000000..2829e7d
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+<p>PASS did not crash.</p>
+<script>
+if (window.testRunner)
+    testRunner.notifyDone();
+</script>
+</head>
+</html>
diff --git a/LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener-expected.txt b/LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener-expected.txt
new file mode 100644 (file)
index 0000000..b5947dd
--- /dev/null
@@ -0,0 +1,8 @@
+Tests that we do not crash when removing the opener after using window.open() to load a resource that redirects.
+
+
+--------
+Frame: 'B'
+--------
+PASS did not crash.
diff --git a/LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener.html b/LayoutTests/http/tests/navigation/window-open-redirect-and-remove-opener.html
new file mode 100644 (file)
index 0000000..72f2b9f
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.dumpChildFramesAsText();
+    testRunner.waitUntilDone();
+}
+
+function removeIframeA()
+{
+    document.body.removeChild(document.querySelector("iframe[name=A]"));
+}
+</script>
+</head>
+<body>
+<p>Tests that we do not crash when removing the opener after using window.open() to load a resource that redirects.</p>
+<iframe name="A" srcdoc='
+<script>
+window.open("http://127.0.0.1:8000/cache/resources/cache-control-redirect.php?url=http://127.0.0.1:8000/navigation/resources/window-open-redirect-and-remove-opener.html", "B");
+parent.removeIframeA();
+</script>
+'></iframe>
+<iframe name="B"></iframe>
+</body>
+</html>
index c88d5fc..6a6ab3a 100644 (file)
@@ -1,3 +1,23 @@
+2017-07-14  Daniel Bates  <dabates@apple.com>
+
+        REGRESSION (r219013): Compute source frame info for frameless document
+        https://bugs.webkit.org/show_bug.cgi?id=174385
+        <rdar://problem/33217736>
+
+        Reviewed by Brady Eidson.
+
+        Fixes an issue where we would crash in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
+        when computing the frame info for a now-frameless document. One way this can happen is when the frame
+        that contains the document that initiated the navigation is removed from the page.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction): Check that we have a valid page ID before
+        looking up the WebPage object corresponding to it.
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): Compute a FrameInfoData
+        object from the source document directly as opposed to using WebFrame::info() as the latter requires
+        that we have a valid WebCore frame and the source document may not have a frame.
+
 2017-07-14  Jer Noble  <jer.noble@apple.com>
 
         Allow clients to override their own hardware media requirements where no fallback media exists.
index a64a1c6..524c152 100644 (file)
@@ -3643,7 +3643,7 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
         if (!fromAPI && originatingFrame == frame)
             sourceFrameInfo = destinationFrameInfo;
         else if (!fromAPI)
-            sourceFrameInfo = API::FrameInfo::create(originatingFrameInfoData, m_process->webPage(originatingPageID));
+            sourceFrameInfo = API::FrameInfo::create(originatingFrameInfoData, originatingPageID ? m_process->webPage(originatingPageID) : nullptr);
 
         auto userInitiatedActivity = m_process->userInitiatedActivity(navigationActionData.userGestureTokenIdentifier);
         bool shouldOpenAppLinks = !m_shouldSuppressAppLinksInNextNavigationPolicyDecision && (!destinationFrameInfo || destinationFrameInfo->isMainFrame()) && !hostsAreEqual(URL(ParsedURLString, m_mainFrame->url()), request.url()) && navigationActionData.navigationType != WebCore::NavigationType::BackForward;
index 47e8f36..2cfcfce 100644 (file)
@@ -777,7 +777,15 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     DownloadID downloadID;
 
     ASSERT(navigationAction.sourceDocument());
-    RefPtr<WebFrame> originatingFrame = WebFrame::fromCoreFrame(*navigationAction.sourceDocument()->frame());
+    const Document& sourceDocument = *navigationAction.sourceDocument();
+    RefPtr<WebFrame> originatingFrame = sourceDocument.frame() ? WebFrame::fromCoreFrame(*sourceDocument.frame()) : nullptr;
+
+    FrameInfoData originatingFrameInfoData;
+    originatingFrameInfoData.isMainFrame = navigationAction.initiatedByMainFrame() == InitiatedByMainFrame::Yes;
+    originatingFrameInfoData.request = ResourceRequest(sourceDocument.url());
+    originatingFrameInfoData.securityOrigin = SecurityOriginData::fromSecurityOrigin(sourceDocument.securityOrigin());
+    if (originatingFrame)
+        originatingFrameInfoData.frameID = originatingFrame->frameID();
 
     NavigationActionData navigationActionData;
     navigationActionData.navigationType = action->navigationType();
@@ -803,7 +811,10 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
     // Notify the UIProcess.
     Ref<WebFrame> protect(*m_frame);
     WebsitePolicies websitePolicies;
-    if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrame ? originatingFrame->info() : FrameInfoData(), originatingFrame && originatingFrame->page() ? originatingFrame->page()->pageID() : 0, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationAction::Reply(newNavigationID, policyAction, downloadID, websitePolicies))) {
+    // FIXME: Determine the originating page independently from the originating frame as it may exist even if
+    // the originating frame does not exist. This can happen if the originating frame was removed from the page.
+    // See <https://bugs.webkit.org/show_bug.cgi?id=174531>.
+    if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingFrame && originatingFrame->page() ? originatingFrame->page()->pageID() : 0, navigationAction.resourceRequest(), request, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())), Messages::WebPageProxy::DecidePolicyForNavigationAction::Reply(newNavigationID, policyAction, downloadID, websitePolicies))) {
         m_frame->didReceivePolicyDecision(listenerID, PolicyIgnore, 0, { });
         return;
     }