Attachment 335621 Details for Bug 183479 – Patch

[patch] Patch

bug-183479-20180312151233.patch (text/plain), 6.35 KB, created by Ali Juma on 2018-03-12 12:12:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ali Juma

Created: 2018-03-12 12:12:34 PDT

Size: 6.35 KB

patch

obsolete

>Subversion Revision: 229530
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 29cb10b039eb8327cff536840693140835ba471c..3199438822195c2fb00a0d5c9da1a97455ae60e2 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-03-12  Ali Juma  <ajuma@chromium.org>
>+
>+        http/tests/workers/service/service-worker-download.https.html times out with async policy delegates
>+        https://bugs.webkit.org/show_bug.cgi?id=183479
>+
>+        Reviewed by Youenn Fablet.
>+
>+        Ensure that ServiceWorkerFetchClient::m_isCheckingResponse is set before code that depends on it
>+        executes. This bit was set by code that's posted to the runloop using 'callOnMainThread' in
>+        ServiceWorkerFetchClient::didReceiveResponse. But when didReceiveResponse is executing, tasks for
>+        handling didReceiveData, didFail, or didFinish may already have been posted to the runloop, and in
>+        that case would execute before m_isCheckingResponse gets set, and then incorrectly fail to 
>+        early-out. Fix this by directly setting m_isCheckingResponse in didReceiveResponse.
>+
>+        * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
>+        (WebKit::ServiceWorkerClientFetch::start):
>+        (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
>+
> 2018-03-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Fix the internal iOS build after r229512
>diff --git a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp b/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp
>index 2808f87827ac1dc5334537e037704e8b045d9215..eaf715fa0eb7e18a5db377a55bcc846ba9320437 100644
>--- a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp
>+++ b/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp
>@@ -68,6 +68,9 @@ void ServiceWorkerClientFetch::start()
> 
>     auto referrer = request.httpReferrer();
> 
>+    m_didFail = false;
>+    m_didFinish = false;
>+
>     // We are intercepting fetch calls after going through the HTTP layer, which may add some specific headers.
>     cleanHTTPRequestHeadersForAccessControl(request, options.httpHeadersToKeep);
> 
>@@ -100,11 +103,15 @@ std::optional<ResourceError> ServiceWorkerClientFetch::validateResponse(const Re
> 
> void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
> {
>+    m_isCheckingResponse = true;
>     callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
>-        if (!m_loader)
>+        if (!m_loader) {
>+            m_isCheckingResponse = false;
>             return;
>+        }
> 
>         if (auto error = validateResponse(response)) {
>+            m_isCheckingResponse = false;
>             m_loader->didFail(error.value());
>             ASSERT(!m_loader);
>             if (auto callback = WTFMove(m_callback))
>@@ -114,6 +121,8 @@ void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
>         response.setSource(ResourceResponse::Source::ServiceWorker);
> 
>         if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
>+            m_isCheckingResponse = false;
>+            continueLoadingAfterCheckingResponse();
>             m_redirectionStatus = RedirectionStatus::Receiving;
>             m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
>                 if (request.isNull() || !m_callback)
>@@ -142,7 +151,6 @@ void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
>         if (response.url().isNull())
>             response.setURL(m_loader->request().url());
> 
>-        m_isCheckingResponse = true;
>         m_loader->didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis)] {
>             m_isCheckingResponse = false;
>             continueLoadingAfterCheckingResponse();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fb17a58d54866556101f0719fc9bc6ebcb0dc445..5d6d1c447b01ccdeb8e8822f5e38557f0141ddc5 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-03-12  Ali Juma  <ajuma@chromium.org>
>+
>+        http/tests/workers/service/service-worker-download.https.html times out with async policy delegates
>+        https://bugs.webkit.org/show_bug.cgi?id=183479
>+
>+        Reviewed by Youenn Fablet.
>+
>+        Add layout test coverage.
>+
>+        * http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt: Added.
>+        * http/tests/workers/service/service-worker-download-async-delegates.https.html: Added.
>+
> 2018-03-12  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Implement CSS Animations and CSS Transitions as Web Animations
>diff --git a/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt b/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..814299cef117796455a7ccd91ba2bfb59b8bbb56
>--- /dev/null
>+++ b/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt
>@@ -0,0 +1,4 @@
>+Download started.
>+Downloading URL with suggested filename "download-binary.php"
>+Download completed.
>+
>diff --git a/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html b/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..0393eb95dd02387b8a85919f319426d39e0e64cf
>--- /dev/null
>+++ b/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html
>@@ -0,0 +1,24 @@
>+<html>
>+<head>
>+<script src="resources/sw-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    testRunner.setShouldLogDownloadCallbacks(true);
>+    testRunner.waitUntilDownloadFinished();
>+    testRunner.setShouldDownloadUndisplayableMIMETypes(true);
>+    if (testRunner.setShouldDecideResponsePolicyAfterDelay)
>+        testRunner.setShouldDecideResponsePolicyAfterDelay(true);
>+}
>+
>+async function doTest()
>+{
>+    await interceptedFrame("resources/service-worker-download-worker.js", "/workers/service/resources/");
>+    window.location = "/workers/service/resources/download-binary.php";
>+}
>+doTest();
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 183479: 335460 | 335478 | 335593 | 335597 | 335598 | 335602 | 335621