Converting a load to a download does not work with async policy delegates
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Mar 2018 17:41:07 +0000 (17:41 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Mar 2018 17:41:07 +0000 (17:41 +0000)
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.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html [new file with mode: 0644]
LayoutTests/platform/ios-wk1/TestExpectations
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestInvocation.cpp

index 3034c6f..256f6a0 100644 (file)
@@ -1,3 +1,16 @@
+2018-03-02  Chris Dumez  <cdumez@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  <csaavedra@igalia.com>
 
         [GTK] Unreviewed gardening
diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate-expected.txt
new file mode 100644 (file)
index 0000000..11140fd
--- /dev/null
@@ -0,0 +1,6 @@
+Download started.
+Downloading URL with suggested filename "unknown"
+Download completed.
+The download should succeed.
+
+File backed blob URL
diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html
new file mode 100644 (file)
index 0000000..a2fb556
--- /dev/null
@@ -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 = window.URL.createObjectURL(file);
+    click(link);
+}
+runTest();
+</script>
+</body>
+</html>
index 3867199..8ba2530 100644 (file)
@@ -1334,6 +1334,7 @@ webkit.org/b/156069 http/tests/download/anchor-download-no-value.html [ Skip ]
 
 # 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 ]
 
index 99175ac..0c33de6 100644 (file)
@@ -1129,6 +1129,7 @@ webkit.org/b/155948 transitions/cancel-transition.html [ Pass Failure ]
 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 ]
index 65881bc..925c18f 100644 (file)
@@ -303,6 +303,7 @@ imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/hist
 
 # 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 ]
 
index 5ddd17e..1413e27 100644 (file)
@@ -441,6 +441,7 @@ http/tests/download/anchor-download-no-value.html [ Skip ]
 
 # 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 ]
index ac2a203..801e7d1 100644 (file)
@@ -1,3 +1,23 @@
+2018-03-02  Chris Dumez  <cdumez@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  <youenn@apple.com>
 
         Some RealtimeMediaSource methods do not need to be marked as virtual
index 6fc9c16..c072d2c 100644 (file)
@@ -783,13 +783,13 @@ void DocumentLoader::responseReceived(const ResourceResponse& response)
     }
 #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())
-            mainResourceLoader->didReceiveResponsePolicy();
-
+    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader)](PolicyAction policy) {
         continueAfterContentPolicy(policy);
+        if (mainResourceLoader)
+            mainResourceLoader->didReceiveResponsePolicy();
     });
 }
 
index 0c1970c..1714daf 100644 (file)
@@ -1,3 +1,29 @@
+2018-03-02  Chris Dumez  <cdumez@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  <youenn@apple.com>
 
         Add API test to validate setting of service worker and cache storage directories
index b694aca..235191a 100644 (file)
@@ -90,6 +90,7 @@ interface TestRunner {
     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);
index 1af3d87..469da9e 100644 (file)
@@ -1367,7 +1367,8 @@ WKBundlePagePolicyAction InjectedBundlePage::decidePolicyForNewWindowAction(WKBu
 
 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 @@ WKBundlePagePolicyAction InjectedBundlePage::decidePolicyForResponse(WKBundlePag
         InjectedBundle::singleton().outputText(stringBuilder.toString());
     }
 
+    if (injectedBundle.testRunner()->shouldDecideResponsePolicyAfterDelay())
+        return WKBundlePagePolicyActionPassThrough;
+
     WKRetainPtr<WKStringRef> mimeType = adoptWK(WKURLResponseCopyMIMEType(response));
     return WKBundlePageCanShowMIMEType(page, mimeType.get()) ? WKBundlePagePolicyActionUse : WKBundlePagePolicyActionPassThrough;
 }
index 5edb655..aed6e4c 100644 (file)
@@ -1111,6 +1111,14 @@ void TestRunner::setShouldDecideNavigationPolicyAfterDelay(bool value)
     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"));
index 9e4405f..e6b8e56 100644 (file)
@@ -329,6 +329,8 @@ public:
 
     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 @@ private:
     double m_databaseMaxQuota;
 
     bool m_shouldDecideNavigationPolicyAfterDelay { false };
+    bool m_shouldDecideResponsePolicyAfterDelay { false };
     bool m_shouldFinishAfterDownload { false };
 
     bool m_userStyleSheetEnabled;
index e708e10..9e9db94 100644 (file)
@@ -854,6 +854,7 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options)
     platformResetStateToConsistentValues();
 
     m_shouldDecideNavigationPolicyAfterDelay = false;
+    m_shouldDecideResponsePolicyAfterDelay = false;
 
     setNavigationGesturesEnabled(false);
     
@@ -2228,17 +2229,28 @@ void TestController::decidePolicyForNavigationResponse(WKPageRef, WKNavigationRe
 
 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 };
+
+    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 (m_shouldDownloadUndisplayableMIMETypes)
-        WKFramePolicyListenerDownload(listener);
+        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)
index c8d4734..4001e3c 100644 (file)
@@ -148,6 +148,7 @@ public:
     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 @@ private:
     bool m_shouldShowTouches { false };
     
     bool m_shouldDecideNavigationPolicyAfterDelay { false };
+    bool m_shouldDecideResponsePolicyAfterDelay { false };
 
     WKRetainPtr<WKArrayRef> m_openPanelFileURLs;
 
index 6347d0b..7868b6d 100644 (file)
@@ -705,6 +705,13 @@ void TestInvocation::didReceiveMessageFromInjectedBundle(WKStringRef messageName
         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);