Diff
Modified: trunk/LayoutTests/ChangeLog (231713 => 231714)
--- trunk/LayoutTests/ChangeLog 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/ChangeLog 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1,3 +1,22 @@
+2018-05-11 Chris Dumez <[email protected]>
+
+ REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
+ https://bugs.webkit.org/show_bug.cgi?id=185531
+ <rdar://problem/39909589>
+
+ Reviewed by Geoffrey Garen.
+
+ * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt: Added.
+ * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html: Added.
+ Add layout test coverage.
+
+ * platform/ios-wk1/TestExpectations:
+ * platform/ios-wk2/TestExpectations:
+ * platform/mac-wk1/TestExpectations:
+ * platform/win/TestExpectations:
+ * platform/wincairo/TestExpectations:
+ Skip new test on platforms that do not support the download attribute.
+
2018-05-11 Antti Koivisto <[email protected]>
LinkLoader fails to remove CachedResourceClient in some cases
Added: trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt (0 => 231714)
--- trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt 2018-05-11 19:22:34 UTC (rev 231714)
@@ -0,0 +1,7 @@
+CONSOLE MESSAGE: line 37: PASS: URL was revoked
+Download started.
+Downloading URL with suggested filename "foo.txt"
+Download completed.
+The suggested filename above should be "foo.txt" and the download should succeed.
+
+Memory backed blob URL
Added: trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html (0 => 231714)
--- trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html 2018-05-11 19:22:34 UTC (rev 231714)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setShouldLogDownloadCallbacks(true);
+ testRunner.waitUntilDownloadFinished();
+}
+</script>
+</head>
+<body>
+<p>The suggested filename above should be "foo.txt" and the download should succeed.</p>
+<a id="blob-url" download="foo">Memory backed blob URL</a>
+<script>
+function runTest()
+{
+ file = new File(["foo"], "foo.txt", {
+ type: "text/plain"
+ });
+ var link = document.getElementById("blob-url");
+ link.href = ""
+ link.click();
+ // Revoke the URL right away.
+ window.URL.revokeObjectURL(link.href);
+
+ // Make sure the URL was revoked.
+ xhr = new XMLHttpRequest();
+ xhr.open("GET", link.href, false);
+ try {
+ xhr.send(null);
+ } catch (e) {
+ }
+ if (xhr.status == 200)
+ console.log("FAIL: URL was not revoked");
+ else
+ console.log("PASS: URL was revoked");
+}
+runTest();
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios-wk1/TestExpectations (231713 => 231714)
--- trunk/LayoutTests/platform/ios-wk1/TestExpectations 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/platform/ios-wk1/TestExpectations 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1367,6 +1367,7 @@
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-doublequote.html [ Skip ]
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
+webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html [ Skip ]
webkit.org/b/156069 http/tests/download/area-download.html [ Skip ]
webkit.org/b/156069 http/tests/security/anchor-download-allow-blob.html [ Skip ]
webkit.org/b/156069 http/tests/security/anchor-download-allow-data.html [ Skip ]
Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (231713 => 231714)
--- trunk/LayoutTests/platform/ios-wk2/TestExpectations 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1157,6 +1157,7 @@
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html [ Skip ]
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-nodownload-set.html [ Skip ]
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-nodownload.html [ Skip ]
webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-download-synthetic-click.html [ Skip ]
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (231713 => 231714)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-05-11 19:22:34 UTC (rev 231714)
@@ -301,6 +301,7 @@
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html [ Skip ]
webkit.org/b/156069 http/tests/download/anchor-download-attribute-content-disposition.html [ Skip ]
webkit.org/b/156069 http/tests/download/anchor-download-no-extension.html [ Skip ]
webkit.org/b/156069 http/tests/download/anchor-download-redirect.html [ Skip ]
Modified: trunk/LayoutTests/platform/win/TestExpectations (231713 => 231714)
--- trunk/LayoutTests/platform/win/TestExpectations 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/platform/win/TestExpectations 2018-05-11 19:22:34 UTC (rev 231714)
@@ -446,6 +446,7 @@
fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html [ Skip ]
http/tests/download/anchor-download-attribute-content-disposition.html [ Skip ]
http/tests/download/anchor-download-no-extension.html [ Skip ]
http/tests/download/area-download.html [ Skip ]
Modified: trunk/LayoutTests/platform/wincairo/TestExpectations (231713 => 231714)
--- trunk/LayoutTests/platform/wincairo/TestExpectations 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/LayoutTests/platform/wincairo/TestExpectations 2018-05-11 19:22:34 UTC (rev 231714)
@@ -66,6 +66,7 @@
fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-nodownload-set.html [ Skip ]
# ENABLE_INPUT_TYPE_* are disabled.
Modified: trunk/Source/WTF/ChangeLog (231713 => 231714)
--- trunk/Source/WTF/ChangeLog 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WTF/ChangeLog 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1,3 +1,15 @@
+2018-05-11 Chris Dumez <[email protected]>
+
+ REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
+ https://bugs.webkit.org/show_bug.cgi?id=185531
+ <rdar://problem/39909589>
+
+ Reviewed by Geoffrey Garen.
+
+ Add a default constructor for CompletionHandlerCallingScope, for convenience.
+
+ * wtf/CompletionHandler.h:
+
2018-05-11 Saam Barati <[email protected]>
Don't allocate value profiles when the JIT is disabled
Modified: trunk/Source/WTF/wtf/CompletionHandler.h (231713 => 231714)
--- trunk/Source/WTF/wtf/CompletionHandler.h 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WTF/wtf/CompletionHandler.h 2018-05-11 19:22:34 UTC (rev 231714)
@@ -66,6 +66,8 @@
class CompletionHandlerCallingScope {
public:
+ CompletionHandlerCallingScope() = default;
+
CompletionHandlerCallingScope(CompletionHandler<void()>&& completionHandler)
: m_completionHandler(WTFMove(completionHandler))
{ }
Modified: trunk/Source/WebCore/ChangeLog (231713 => 231714)
--- trunk/Source/WebCore/ChangeLog 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WebCore/ChangeLog 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1,3 +1,29 @@
+2018-05-11 Chris Dumez <[email protected]>
+
+ REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
+ https://bugs.webkit.org/show_bug.cgi?id=185531
+ <rdar://problem/39909589>
+
+ Reviewed by Geoffrey Garen.
+
+ Whenever we start an asynchronous navigation policy decision for a blob URL, create a temporary
+ blob URL pointing to the same data, and update the request's URL. This way, if the page's JS revokes
+ the URL during the policy decision, the load will still succeed.
+
+ Test: fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::load):
+ (WebCore::FrameLoader::loadPostRequest):
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::extendBlobURLLifetimeIfNecessary const):
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
2018-05-11 Antti Koivisto <[email protected]>
LinkLoader fails to remove CachedResourceClient in some cases
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (231713 => 231714)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2018-05-11 19:22:34 UTC (rev 231714)
@@ -666,7 +666,7 @@
return;
}
- frameLoader()->policyChecker().checkNavigationPolicy(ResourceRequest(newRequest), didReceiveRedirectResponse, WTFMove(navigationPolicyCompletionHandler));
+ frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), didReceiveRedirectResponse, WTFMove(navigationPolicyCompletionHandler));
}
bool DocumentLoader::tryLoadingRequestFromApplicationCache()
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (231713 => 231714)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-05-11 19:22:34 UTC (rev 231714)
@@ -1336,7 +1336,7 @@
if (!targetFrame && !frameName.isEmpty()) {
action = "" frameLoadRequest));
- policyChecker().checkNewWindowPolicy(WTFMove(action), request, formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
});
@@ -1356,7 +1356,7 @@
oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
policyChecker().stopCheck();
policyChecker().setLoadType(newLoadType);
- policyChecker().checkNavigationPolicy(ResourceRequest(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
+ policyChecker().checkNavigationPolicy(WTFMove(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
}, PolicyDecisionMode::Synchronous);
return;
@@ -1412,7 +1412,7 @@
if (request.shouldCheckNewWindowPolicy()) {
NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
- policyChecker().checkNewWindowPolicy(WTFMove(action), request.resourceRequest(), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
});
@@ -2858,7 +2858,7 @@
return;
}
- policyChecker().checkNewWindowPolicy(WTFMove(action), workingResourceRequest, WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
});
Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (231713 => 231714)
--- trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-05-11 19:22:34 UTC (rev 231714)
@@ -31,6 +31,8 @@
#include "config.h"
#include "PolicyChecker.h"
+#include "BlobRegistry.h"
+#include "BlobURL.h"
#include "ContentFilter.h"
#include "ContentSecurityPolicy.h"
#include "DOMWindow.h"
@@ -81,6 +83,20 @@
checkNavigationPolicy(WTFMove(newRequest), didReceiveRedirectResponse, m_frame.loader().activeDocumentLoader(), nullptr, WTFMove(function));
}
+CompletionHandlerCallingScope PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request) const
+{
+ if (!request.url().protocolIsBlob())
+ return { };
+
+ // Create a new temporary blobURL in case this one gets revoked during the asynchronous navigation policy decision.
+ URL temporaryBlobURL = BlobURL::createPublicURL(&m_frame.document()->securityOrigin());
+ blobRegistry().registerBlobURL(temporaryBlobURL, request.url());
+ request.setURL(temporaryBlobURL);
+ return CompletionHandler<void()>([temporaryBlobURL = WTFMove(temporaryBlobURL)] {
+ blobRegistry().unregisterBlobURL(temporaryBlobURL);
+ });
+}
+
void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didReceiveRedirectResponse, DocumentLoader* loader, FormState* formState, NavigationPolicyDecisionFunction&& function, PolicyDecisionMode policyDecisionMode)
{
NavigationAction action = ""
@@ -147,10 +163,11 @@
m_frame.loader().clearProvisionalLoadForPolicyCheck();
+ auto blobURLLifetimeExtension = policyDecisionMode == PolicyDecisionMode::Asynchronous ? extendBlobURLLifetimeIfNecessary(request) : CompletionHandlerCallingScope { };
+
m_delegateIsDecidingNavigationPolicy = true;
String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
- ResourceRequest requestCopy = request;
- m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename)](PolicyAction policyAction) mutable {
+ m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = ResourceRequest(request), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
m_delegateIsDecidingNavigationPolicy = false;
switch (policyAction) {
@@ -173,7 +190,7 @@
});
}
-void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)
+void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, ResourceRequest&& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)
{
if (m_frame.document() && m_frame.document()->isSandboxed(SandboxPopups))
return function({ }, nullptr, { }, { }, ShouldContinue::No);
@@ -181,7 +198,9 @@
if (!DOMWindow::allowPopUp(m_frame))
return function({ }, nullptr, { }, { }, ShouldContinue::No);
- m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function)](PolicyAction policyAction) mutable {
+ auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request);
+
+ m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function), blobURLLifetimeExtension = WTFMove(blobURLLifetimeExtension)](PolicyAction policyAction) mutable {
switch (policyAction) {
case PolicyAction::Download:
frame->loader().client().startDownload(request);
Modified: trunk/Source/WebCore/loader/PolicyChecker.h (231713 => 231714)
--- trunk/Source/WebCore/loader/PolicyChecker.h 2018-05-11 19:05:36 UTC (rev 231713)
+++ trunk/Source/WebCore/loader/PolicyChecker.h 2018-05-11 19:22:34 UTC (rev 231714)
@@ -39,6 +39,7 @@
namespace WTF {
template<typename> class CompletionHandler;
+class CompletionHandlerCallingScope;
}
namespace WebCore {
@@ -69,7 +70,7 @@
void checkNavigationPolicy(ResourceRequest&&, bool didReceiveRedirectResponse, DocumentLoader*, FormState*, NavigationPolicyDecisionFunction&&, PolicyDecisionMode = PolicyDecisionMode::Asynchronous);
void checkNavigationPolicy(ResourceRequest&&, bool didReceiveRedirectResponse, NavigationPolicyDecisionFunction&&);
- void checkNewWindowPolicy(NavigationAction&&, const ResourceRequest&, FormState*, const String& frameName, NewWindowPolicyDecisionFunction&&);
+ void checkNewWindowPolicy(NavigationAction&&, ResourceRequest&&, FormState*, const String& frameName, NewWindowPolicyDecisionFunction&&);
void stopCheck();
@@ -87,6 +88,7 @@
private:
void handleUnimplementablePolicy(const ResourceError&);
+ WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&) const;
Frame& m_frame;
