Title: [229558] trunk
- Revision
- 229558
- Author
- aj...@chromium.org
- Date
- 2018-03-12 15:58:54 -0700 (Mon, 12 Mar 2018)
Log Message
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.
Source/WebKit:
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):
LayoutTests:
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229557 => 229558)
--- trunk/LayoutTests/ChangeLog 2018-03-12 22:24:37 UTC (rev 229557)
+++ trunk/LayoutTests/ChangeLog 2018-03-12 22:58:54 UTC (rev 229558)
@@ -1,3 +1,15 @@
+2018-03-12 Ali Juma <aj...@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 Chris Dumez <cdu...@apple.com>
http/tests/security/frame-loading-via-document-write-async-delegates.html fails with async delegates
Added: trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt (0 => 229558)
--- trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt 2018-03-12 22:58:54 UTC (rev 229558)
@@ -0,0 +1,4 @@
+Download started.
+Downloading URL with suggested filename "download-binary.php"
+Download completed.
+
Added: trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html (0 => 229558)
--- trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-download-async-delegates.https.html 2018-03-12 22:58:54 UTC (rev 229558)
@@ -0,0 +1,24 @@
+<html>
+<head>
+<script src=""
+</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>
Modified: trunk/Source/WebKit/ChangeLog (229557 => 229558)
--- trunk/Source/WebKit/ChangeLog 2018-03-12 22:24:37 UTC (rev 229557)
+++ trunk/Source/WebKit/ChangeLog 2018-03-12 22:58:54 UTC (rev 229558)
@@ -1,3 +1,21 @@
+2018-03-12 Ali Juma <aj...@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-12 Wenson Hsieh <wenson_hs...@apple.com>
REGRESSION(r211643): Dismissing WKActionSheet should not also dismiss its presenting view controller
Modified: trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp (229557 => 229558)
--- trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2018-03-12 22:24:37 UTC (rev 229557)
+++ trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2018-03-12 22:58:54 UTC (rev 229558)
@@ -68,6 +68,9 @@
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 @@
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 @@
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 @@
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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
