Commit bb9fd1a8 authored by Tsuyoshi Horo's avatar Tsuyoshi Horo Committed by Commit Bot

Set SkipServiceWorker flag for synchronous loads from the main thread.

Before 5e1b52dd, synchronous XHR on worker
was handled by service workers. It is because the |is_sync_load| was false
when the sync request is from worker thread. But after the CL, the
|is_sync_load| flag for the sync request from worker became true, so the request
will not go to the service worker.

This CL will fix this by
 - Set the SkipServiceWorker flag for synchronous loads from the main thread
   in the renderer process. (FetchParameters.cpp)
 - Don't set skip_service_worker even if is_sync_load is true in the browser
   process. (resource_dispatcher_host_impl.cc)

Bug: 706331,827473
Change-Id: I186bc97f3f8d298e0a04942d0ec4b708b3022cc1
Reviewed-on: https://chromium-review.googlesource.com/989376Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547407}
parent aa4f22e9
......@@ -1242,14 +1242,11 @@ void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(
std::move(url_loader_client),
static_cast<ResourceType>(request_data.resource_type));
} else {
// Initialize the service worker handler for the request. We don't use
// ServiceWorker for synchronous loads to avoid renderer deadlocks.
const bool skip_service_worker =
is_sync_load || request_data.skip_service_worker;
// Initialize the service worker handler for the request.
ServiceWorkerRequestHandler::InitializeHandler(
new_request.get(), requester_info->service_worker_context(),
blob_context, child_id, request_data.service_worker_provider_id,
skip_service_worker, request_data.fetch_request_mode,
request_data.skip_service_worker, request_data.fetch_request_mode,
request_data.fetch_credentials_mode, request_data.fetch_redirect_mode,
request_data.fetch_integrity, request_data.keepalive,
static_cast<ResourceType>(request_data.resource_type),
......
<!DOCTYPE html>
<title>Service Worker: Synchronous XHR on Worker is intercepted</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/test-helpers.sub.js"></script>
<script>
'use strict';
promise_test((t) => {
const url = 'resources/fetch-request-xhr-sync-on-worker-worker.js';
const scope = 'resources/fetch-request-xhr-sync-on-worker-scope/';
const non_existent_file = 'non-existent-file.txt';
// In Chromium, the service worker scope matching for workers is based on
// the URL of the parent HTML. So this test creates an iframe which is
// controlled by the service worker first, and creates a worker from the
// iframe.
return service_worker_unregister_and_register(t, url, scope)
.then((registration) => {
t.add_cleanup(() => registration.unregister());
return wait_for_state(t, registration.installing, 'activated');
})
.then(() => { return with_iframe(scope + 'iframe_page'); })
.then((frame) => {
t.add_cleanup(() => frame.remove());
return frame.contentWindow.performSyncXHROnWorker(non_existent_file);
})
.then((result) => {
assert_equals(
result.status,
200,
'HTTP response status code for intercepted request'
);
assert_equals(
result.responseText,
'Response from service worker',
'HTTP response text for intercepted request'
);
});
}, 'Verify SyncXHR on Worker is intercepted');
</script>
'use strict';
self.onfetch = function(event) {
if (event.request.url.indexOf('non-existent-file.txt') !== -1) {
event.respondWith(new Response('Response from service worker'));
} else if (event.request.url.indexOf('/iframe_page') !== -1) {
event.respondWith(new Response(
'<!DOCTYPE html>\n' +
'<script>\n' +
'function performSyncXHROnWorker(url) {\n' +
' return new Promise((resolve) => {\n' +
' var worker =\n' +
' new Worker(\'./worker_script\');\n' +
' worker.addEventListener(\'message\', (msg) => {\n' +
' resolve(msg.data);\n' +
' });\n' +
' worker.postMessage({\n' +
' url: url\n' +
' });\n' +
' });\n' +
'}\n' +
'</script>',
{
headers: [['content-type', 'text/html']]
}));
} else if (event.request.url.indexOf('/worker_script') !== -1) {
event.respondWith(new Response(
'self.onmessage = (msg) => {' +
' const syncXhr = new XMLHttpRequest();' +
' syncXhr.open(\'GET\', msg.data.url, false);' +
' syncXhr.send();' +
' self.postMessage({' +
' status: syncXhr.status,' +
' responseText: syncXhr.responseText' +
' });' +
'}',
{
headers: [['content-type', 'application/javascript']]
}));
}
};
......@@ -125,6 +125,10 @@ void FetchParameters::MakeSynchronous() {
if (resource_request_.TimeoutInterval() == INT_MAX) {
resource_request_.SetTimeoutInterval(10);
}
// Skip ServiceWorker for synchronous loads from the main thread to avoid
// deadlocks.
if (IsMainThread())
resource_request_.SetSkipServiceWorker(true);
options_.synchronous_policy = kRequestSynchronously;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment