Attachment 340150 Details for Bug 185531 – Patch

[patch] Patch

bug-185531-20180510170718.patch (text/plain), 10.35 KB, created by Chris Dumez on 2018-05-10 17:07:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-10 17:07:19 PDT

Size: 10.35 KB

patch

obsolete

>Subversion Revision: 231654
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8c7ef67f825183403d835e517da46ea83a49b4d0..34f71f24b2aac46de4a10fbd3647f9d91642bf29 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-05-10  Chris Dumez  <cdumez@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        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 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/PolicyChecker.cpp:
>+        (WebCore::PolicyChecker::checkNavigationPolicy):
>+
> 2018-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         'Cross-Origin-Options header implementation follow-up
>diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
>index fc2419498384d22bdf6d7a7d926ec53c6b04ad58..a20c34c821f0c02ce35f8aff5cb28dbb65fcf3da 100644
>--- a/Source/WebCore/loader/PolicyChecker.cpp
>+++ b/Source/WebCore/loader/PolicyChecker.cpp
>@@ -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"
>@@ -44,6 +46,7 @@
> #include "HTMLFormElement.h"
> #include "HTMLFrameOwnerElement.h"
> #include "HTMLPlugInElement.h"
>+#include <wtf/CallbackAggregator.h>
> #include <wtf/CompletionHandler.h>
> 
> #if USE(QUICK_LOOK)
>@@ -147,10 +150,21 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
> 
>     m_frame.loader().clearProvisionalLoadForPolicyCheck();
> 
>+    RefPtr<CallbackAggregator> blobURLLifetimeExtension;
>+    if (request.url().protocolIsBlob() && policyDecisionMode == PolicyDecisionMode::Asynchronous) {
>+        // 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);
>+        blobURLLifetimeExtension = CallbackAggregator::create([temporaryBlobURL] {
>+            blobRegistry().unregisterBlobURL(temporaryBlobURL);
>+        });
>+    }
>+
>     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 = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension](PolicyAction policyAction) mutable {
>         m_delegateIsDecidingNavigationPolicy = false;
> 
>         switch (policyAction) {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dd871f09cc6f5c49066135edcd7fdf9a65ee7eaa..6287122dc8c39d9cc8fb69a8560546ee5a8145dc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-05-10  Chris Dumez  <cdumez@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt: Added.
>+        * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html: Added.
>+
> 2018-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         'Cross-Origin-Options header implementation follow-up
>diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d2b6224077a4f5ef1b6b69632c4bc245e9aef742
>--- /dev/null
>+++ b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt
>@@ -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
>diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..70723c4b16e3e2eb2998e19421654e20abf87b57
>--- /dev/null
>+++ b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
>@@ -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 = window.URL.createObjectURL(file);
>+    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>
>diff --git a/LayoutTests/platform/ios-wk1/TestExpectations b/LayoutTests/platform/ios-wk1/TestExpectations
>index 0b4a3ca75dfaf633dfe69ce6900ff3221244d253..99b52816de00c43fd2960c8b3c3d8d07a3eb9e1b 100644
>--- a/LayoutTests/platform/ios-wk1/TestExpectations
>+++ b/LayoutTests/platform/ios-wk1/TestExpectations
>@@ -1366,6 +1366,7 @@ webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-include
> 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 ]
>diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
>index df0f144c9b130c01a16dd411d159c014aff942a1..784767028938b12f05c3773d96d96a70bf776fee 100644
>--- a/LayoutTests/platform/mac-wk1/TestExpectations
>+++ b/LayoutTests/platform/mac-wk1/TestExpectations
>@@ -300,6 +300,7 @@ webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-include
> 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 ]
>diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
>index d83d77df329d385416223a7ce21248cad6492abd..3c11c03621298b0597f720d2bea16038834cce32 100644
>--- a/LayoutTests/platform/win/TestExpectations
>+++ b/LayoutTests/platform/win/TestExpectations
>@@ -446,6 +446,7 @@ fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-doublequote.html [
> 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 ]
>diff --git a/LayoutTests/platform/wincairo/TestExpectations b/LayoutTests/platform/wincairo/TestExpectations
>index dfbb498a48d095f58c68bd2b38cda7dffc1947c1..93b69bb7a85530e6a55eb97f37fddfdd34e91d42 100644
>--- a/LayoutTests/platform/wincairo/TestExpectations
>+++ b/LayoutTests/platform/wincairo/TestExpectations
>@@ -66,6 +66,7 @@ fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-doublequote.html [
> 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.

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185531: 340141 | 340148 | 340150 | 340156 | 340199