Title: [229177] trunk
Revision
229177
Author
cdu...@apple.com
Date
2018-03-02 09:41:07 -0800 (Fri, 02 Mar 2018)

Log Message

Converting a load to a download does not work with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183254
<rdar://problem/38035334>

Reviewed by Youenn Fablet.

Source/WebCore:

Update DocumentLoader::responseReceived() to call didReceiveResponsePolicy()
on the mainResourceLoader *after* calling continueAfterContentPolicy(),
not *before*. This makes sure that the WebResourceLoader sends the
NetworkResourceLoader::ContinueDidReceiveResponse IPC back to the Network
Process *after* the policy decision has been processed, which restores the
pre-r228852 order.

Test: fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::responseReceived):

Tools:

Add layout test infrastructure for responding to the decidePolicyForNavigationResponse
delegate asynchronously.

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
(WTR::InjectedBundlePage::decidePolicyForResponse):
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setShouldDecideResponsePolicyAfterDelay):
* WebKitTestRunner/InjectedBundle/TestRunner.h:
(WTR::TestRunner::shouldDecideResponsePolicyAfterDelay const):
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):
(WTR::TestController::decidePolicyForNavigationResponse):
* WebKitTestRunner/TestController.h:
(WTR::TestController::setShouldDecideResponsePolicyAfterDelay):
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):

LayoutTests:

Add layout test coverage.

* fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt: Added.
* fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (229176 => 229177)


--- trunk/LayoutTests/ChangeLog	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/LayoutTests/ChangeLog	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1,3 +1,16 @@
+2018-03-02  Chris Dumez  <cdu...@apple.com>
+
+        Converting a load to a download does not work with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183254
+        <rdar://problem/38035334>
+
+        Reviewed by Youenn Fablet.
+
+        Add layout test coverage.
+
+        * fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt: Added.
+        * fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html: Added.
+
 2018-03-02  Claudio Saavedra  <csaave...@igalia.com>
 
         [GTK] Unreviewed gardening

Added: trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt (0 => 229177)


--- trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt	2018-03-02 17:41:07 UTC (rev 229177)
@@ -0,0 +1,6 @@
+Download started.
+Downloading URL with suggested filename "unknown"
+Download completed.
+The download should succeed.
+
+File backed blob URL

Added: trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html (0 => 229177)


--- trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html	2018-03-02 17:41:07 UTC (rev 229177)
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.setShouldLogDownloadCallbacks(true);
+  testRunner.waitUntilDownloadFinished();
+  testRunner.setShouldDownloadUndisplayableMIMETypes(true);
+  if (testRunner.setShouldDecideResponsePolicyAfterDelay)
+    testRunner.setShouldDecideResponsePolicyAfterDelay(true);
+}
+</script>
+</head>
+<body>
+<p>The download should succeed.</p>
+<a id="blob-url">File backed blob URL</a>
+<script>
+function click(elmt)
+{
+    if (!window.eventSender) {
+        alert('Click the link to run the test.');
+        return;
+    }
+    eventSender.mouseMoveTo(elmt.offsetLeft + 5, elmt.offsetTop + 5);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+}
+
+function runTest()
+{
+    file = internals.createFile("../../../resources/Ahem.otf");
+    var link = document.getElementById("blob-url");
+    link.href = ""
+    click(link);
+}
+runTest();
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/ios-wk1/TestExpectations (229176 => 229177)


--- trunk/LayoutTests/platform/ios-wk1/TestExpectations	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/LayoutTests/platform/ios-wk1/TestExpectations	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1334,6 +1334,7 @@
 
 # testRunner.setShouldDownloadUndisplayableMIMETypes() is not supported on WK1.
 fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html [ Skip ]
+fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html [ Skip ]
 
 webkit.org/b/137572 scrollbars/scrollbar-iframe-click-does-not-blur-content.html [ Failure ]
 

Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (229176 => 229177)


--- trunk/LayoutTests/platform/ios-wk2/TestExpectations	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1129,6 +1129,7 @@
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-download-unset.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-download.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html [ Skip ]
+webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-backslash.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-doublequote.html [ Skip ]

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (229176 => 229177)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2018-03-02 17:41:07 UTC (rev 229177)
@@ -303,6 +303,7 @@
 
 # testRunner.setShouldDownloadUndisplayableMIMETypes() is not supported on WK1.
 fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html [ Skip ]
+fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html [ Skip ]
 
 webkit.org/b/156629 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html [ Pass Failure ]
 

Modified: trunk/LayoutTests/platform/win/TestExpectations (229176 => 229177)


--- trunk/LayoutTests/platform/win/TestExpectations	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/LayoutTests/platform/win/TestExpectations	2018-03-02 17:41:07 UTC (rev 229177)
@@ -441,6 +441,7 @@
 
 # testRunner.setShouldDownloadUndisplayableMIMETypes() is not supported on WK1.
 fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html [ Skip ]
+fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html [ Skip ]
 
 # TODO Expose title direction in WebKit API (Chromium Only)
 webkit.org/b/58845 fast/dom/title-directionality.html [ Skip ]

Modified: trunk/Source/WebCore/ChangeLog (229176 => 229177)


--- trunk/Source/WebCore/ChangeLog	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Source/WebCore/ChangeLog	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1,3 +1,23 @@
+2018-03-02  Chris Dumez  <cdu...@apple.com>
+
+        Converting a load to a download does not work with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183254
+        <rdar://problem/38035334>
+
+        Reviewed by Youenn Fablet.
+
+        Update DocumentLoader::responseReceived() to call didReceiveResponsePolicy()
+        on the mainResourceLoader *after* calling continueAfterContentPolicy(),
+        not *before*. This makes sure that the WebResourceLoader sends the
+        NetworkResourceLoader::ContinueDidReceiveResponse IPC back to the Network
+        Process *after* the policy decision has been processed, which restores the
+        pre-r228852 order.
+
+        Test: fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::responseReceived):
+
 2018-03-02  Youenn Fablet  <you...@apple.com>
 
         Some RealtimeMediaSource methods do not need to be marked as virtual

Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (229176 => 229177)


--- trunk/Source/WebCore/loader/DocumentLoader.cpp	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp	2018-03-02 17:41:07 UTC (rev 229177)
@@ -783,13 +783,13 @@
     }
 #endif
 
-    if (auto* mainResourceLoader = this->mainResourceLoader())
+    RefPtr<SubresourceLoader> mainResourceLoader = this->mainResourceLoader();
+    if (mainResourceLoader)
         mainResourceLoader->markInAsyncResponsePolicyCheck();
-    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this)](PolicyAction policy) {
-        if (auto* mainResourceLoader = this->mainResourceLoader())
+    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader)](PolicyAction policy) {
+        continueAfterContentPolicy(policy);
+        if (mainResourceLoader)
             mainResourceLoader->didReceiveResponsePolicy();
-
-        continueAfterContentPolicy(policy);
     });
 }
 

Modified: trunk/Tools/ChangeLog (229176 => 229177)


--- trunk/Tools/ChangeLog	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/ChangeLog	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1,3 +1,29 @@
+2018-03-02  Chris Dumez  <cdu...@apple.com>
+
+        Converting a load to a download does not work with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183254
+        <rdar://problem/38035334>
+
+        Reviewed by Youenn Fablet.
+
+        Add layout test infrastructure for responding to the decidePolicyForNavigationResponse
+        delegate asynchronously.
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
+        (WTR::InjectedBundlePage::decidePolicyForResponse):
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setShouldDecideResponsePolicyAfterDelay):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        (WTR::TestRunner::shouldDecideResponsePolicyAfterDelay const):
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        (WTR::TestController::decidePolicyForNavigationResponse):
+        * WebKitTestRunner/TestController.h:
+        (WTR::TestController::setShouldDecideResponsePolicyAfterDelay):
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
+
 2018-03-01  Youenn Fablet  <you...@apple.com>
 
         Add API test to validate setting of service worker and cache storage directories

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2018-03-02 17:41:07 UTC (rev 229177)
@@ -90,6 +90,7 @@
     void setAsynchronousSpellCheckingEnabled(boolean value);
     void setPrinting();
     void setShouldDecideNavigationPolicyAfterDelay(boolean value);
+    void setShouldDecideResponsePolicyAfterDelay(boolean value);
     void setNavigationGesturesEnabled(boolean value);
     void setIgnoresViewportScaleLimits(boolean value);
     void setShouldDownloadUndisplayableMIMETypes(boolean value);

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1367,7 +1367,8 @@
 
 WKBundlePagePolicyAction InjectedBundlePage::decidePolicyForResponse(WKBundlePageRef page, WKBundleFrameRef, WKURLResponseRef response, WKURLRequestRef, WKTypeRef*)
 {
-    if (InjectedBundle::singleton().testRunner()->isPolicyDelegateEnabled() && WKURLResponseIsAttachment(response)) {
+    auto& injectedBundle = InjectedBundle::singleton();
+    if (injectedBundle.testRunner()->isPolicyDelegateEnabled() && WKURLResponseIsAttachment(response)) {
         StringBuilder stringBuilder;
         WKRetainPtr<WKStringRef> filename = adoptWK(WKURLResponseCopySuggestedFilename(response));
         stringBuilder.appendLiteral("Policy delegate: resource is an attachment, suggested file name \'");
@@ -1376,6 +1377,9 @@
         InjectedBundle::singleton().outputText(stringBuilder.toString());
     }
 
+    if (injectedBundle.testRunner()->shouldDecideResponsePolicyAfterDelay())
+        return WKBundlePagePolicyActionPassThrough;
+
     WKRetainPtr<WKStringRef> mimeType = adoptWK(WKURLResponseCopyMIMEType(response));
     return WKBundlePageCanShowMIMEType(page, mimeType.get()) ? WKBundlePagePolicyActionUse : WKBundlePagePolicyActionPassThrough;
 }

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2018-03-02 17:41:07 UTC (rev 229177)
@@ -1111,6 +1111,14 @@
     WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), messageBody.get());
 }
 
+void TestRunner::setShouldDecideResponsePolicyAfterDelay(bool value)
+{
+    m_shouldDecideResponsePolicyAfterDelay = value;
+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("SetShouldDecideResponsePolicyAfterDelay"));
+    WKRetainPtr<WKBooleanRef> messageBody(AdoptWK, WKBooleanCreate(value));
+    WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), messageBody.get());
+}
+
 void TestRunner::setNavigationGesturesEnabled(bool value)
 {
     WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("SetNavigationGesturesEnabled"));

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2018-03-02 17:41:07 UTC (rev 229177)
@@ -329,6 +329,8 @@
 
     bool shouldDecideNavigationPolicyAfterDelay() const { return m_shouldDecideNavigationPolicyAfterDelay; }
     void setShouldDecideNavigationPolicyAfterDelay(bool);
+    bool shouldDecideResponsePolicyAfterDelay() const { return m_shouldDecideResponsePolicyAfterDelay; }
+    void setShouldDecideResponsePolicyAfterDelay(bool);
     void setNavigationGesturesEnabled(bool);
     void setIgnoresViewportScaleLimits(bool);
     void setShouldDownloadUndisplayableMIMETypes(bool);
@@ -475,6 +477,7 @@
     double m_databaseMaxQuota;
 
     bool m_shouldDecideNavigationPolicyAfterDelay { false };
+    bool m_shouldDecideResponsePolicyAfterDelay { false };
     bool m_shouldFinishAfterDownload { false };
 
     bool m_userStyleSheetEnabled;

Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/TestController.cpp	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp	2018-03-02 17:41:07 UTC (rev 229177)
@@ -854,6 +854,7 @@
     platformResetStateToConsistentValues();
 
     m_shouldDecideNavigationPolicyAfterDelay = false;
+    m_shouldDecideResponsePolicyAfterDelay = false;
 
     setNavigationGesturesEnabled(false);
     
@@ -2228,17 +2229,28 @@
 
 void TestController::decidePolicyForNavigationResponse(WKNavigationResponseRef navigationResponse, WKFramePolicyListenerRef listener)
 {
-    // Even though Response was already checked by WKBundlePagePolicyClient, the check did not include plugins
-    // so we have to re-check again.
-    if (WKNavigationResponseCanShowMIMEType(navigationResponse)) {
-        WKFramePolicyListenerUse(listener);
-        return;
-    }
+    WKRetainPtr<WKNavigationResponseRef> retainedNavigationResponse { navigationResponse };
+    WKRetainPtr<WKFramePolicyListenerRef> retainedListener { listener };
 
-    if (m_shouldDownloadUndisplayableMIMETypes)
-        WKFramePolicyListenerDownload(listener);
+    bool shouldDownloadUndisplayableMIMETypes = m_shouldDownloadUndisplayableMIMETypes;
+    auto decisionFunction = [shouldDownloadUndisplayableMIMETypes, retainedNavigationResponse, retainedListener]() {
+        // Even though Response was already checked by WKBundlePagePolicyClient, the check did not include plugins
+        // so we have to re-check again.
+        if (WKNavigationResponseCanShowMIMEType(retainedNavigationResponse.get())) {
+            WKFramePolicyListenerUse(retainedListener.get());
+            return;
+        }
+
+        if (shouldDownloadUndisplayableMIMETypes)
+            WKFramePolicyListenerDownload(retainedListener.get());
+        else
+            WKFramePolicyListenerIgnore(retainedListener.get());
+    };
+
+    if (m_shouldDecideResponsePolicyAfterDelay)
+        RunLoop::main().dispatch(WTFMove(decisionFunction));
     else
-        WKFramePolicyListenerIgnore(listener);
+        decisionFunction();
 }
 
 void TestController::didNavigateWithNavigationData(WKContextRef, WKPageRef, WKNavigationDataRef navigationData, WKFrameRef frame, const void* clientInfo)

Modified: trunk/Tools/WebKitTestRunner/TestController.h (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/TestController.h	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/TestController.h	2018-03-02 17:41:07 UTC (rev 229177)
@@ -148,6 +148,7 @@
     bool isCurrentInvocation(TestInvocation* invocation) const { return invocation == m_currentInvocation.get(); }
 
     void setShouldDecideNavigationPolicyAfterDelay(bool value) { m_shouldDecideNavigationPolicyAfterDelay = value; }
+    void setShouldDecideResponsePolicyAfterDelay(bool value) { m_shouldDecideResponsePolicyAfterDelay = value; }
 
     void setNavigationGesturesEnabled(bool value);
     void setIgnoresViewportScaleLimits(bool);
@@ -415,6 +416,7 @@
     bool m_shouldShowTouches { false };
     
     bool m_shouldDecideNavigationPolicyAfterDelay { false };
+    bool m_shouldDecideResponsePolicyAfterDelay { false };
 
     WKRetainPtr<WKArrayRef> m_openPanelFileURLs;
 

Modified: trunk/Tools/WebKitTestRunner/TestInvocation.cpp (229176 => 229177)


--- trunk/Tools/WebKitTestRunner/TestInvocation.cpp	2018-03-02 17:39:50 UTC (rev 229176)
+++ trunk/Tools/WebKitTestRunner/TestInvocation.cpp	2018-03-02 17:41:07 UTC (rev 229177)
@@ -705,6 +705,13 @@
         return;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "SetShouldDecideResponsePolicyAfterDelay")) {
+        ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
+        WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);
+        TestController::singleton().setShouldDecideResponsePolicyAfterDelay(WKBooleanGetValue(value));
+        return;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "SetNavigationGesturesEnabled")) {
         ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
         WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to