Commit 8b8ce25d authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

Worker: Terminate dedicated worker thread when failing to fetch worker script

When off-the-main-thread dedicated worker script loading fails, a dedicated
worker needs to terminate its worker thread. However, the current code does not
do it. This CL fixes it by calling close() in that case.

Off-the-main-thread dedicated worker script loading is used when
PlzDedicatedWorker is enabled and/or ModuleDedicatedWorker is enabled.

Bug: 900429
Change-Id: Iff6c1981ab8e43f9d294a1b9c3e4dc56b3a38650
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1843800
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarKenichi Ishibashi <bashi@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703604}
parent be1faefe
...@@ -280,9 +280,17 @@ void DedicatedWorkerGlobalScope::DidFetchClassicScript( ...@@ -280,9 +280,17 @@ void DedicatedWorkerGlobalScope::DidFetchClassicScript(
// Step 12. "If the algorithm asynchronously completes with null, then:" // Step 12. "If the algorithm asynchronously completes with null, then:"
if (classic_script_loader->Failed()) { if (classic_script_loader->Failed()) {
// Step 12.1. "Queue a task to fire an event named error at worker." // Step 12.1. "Queue a task to fire an event named error at worker."
// DidFailToFetchClassicScript() will asynchronously fire the event.
ReportingProxy().DidFailToFetchClassicScript();
// Step 12.2. "Run the environment discarding steps for inside settings." // Step 12.2. "Run the environment discarding steps for inside settings."
// Do nothing because the HTML spec doesn't define these steps for web
// workers.
// Schedule worker termination.
close();
// Step 12.3. "Return." // Step 12.3. "Return."
ReportingProxy().DidFailToFetchClassicScript();
return; return;
} }
ReportingProxy().DidFetchScript(); ReportingProxy().DidFetchScript();
......
...@@ -26,10 +26,20 @@ void WorkerModuleTreeClient::NotifyModuleTreeLoadFinished( ...@@ -26,10 +26,20 @@ void WorkerModuleTreeClient::NotifyModuleTreeLoadFinished(
blink::WorkerReportingProxy& worker_reporting_proxy = blink::WorkerReportingProxy& worker_reporting_proxy =
worker_global_scope->ReportingProxy(); worker_global_scope->ReportingProxy();
// Step 12. "If the algorithm asynchronously completes with null, then:"
if (!module_script) { if (!module_script) {
// Step 12: "If the algorithm asynchronously completes with null, queue // Step 12.1. "Queue a task to fire an event named error at worker."
// a task to fire an event named error at worker, and return." // DidFailToFetchModuleScript() will asynchronously fire the event.
worker_reporting_proxy.DidFailToFetchModuleScript(); worker_reporting_proxy.DidFailToFetchModuleScript();
// Step 12.2. "Run the environment discarding steps for inside settings."
// Do nothing because the HTML spec doesn't define these steps for web
// workers.
// Schedule worker termination.
worker_global_scope->close();
// Step 12.3. "Return."
return; return;
} }
worker_reporting_proxy.DidFetchScript(); worker_reporting_proxy.DidFetchScript();
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>Test worker termination on script fetch failure</title>
<body></body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
// This test should not be upstreamed to WPT because of using Internals API.
function run_test(worker_url, worker_options, fetch_type) {
promise_test(async t => {
assert_equals(window.internals.workerThreadCount, 0);
const worker = new Worker(worker_url, worker_options);
// A failure of worker script fetch should be reported to the onerror
// handler.
await new Promise(resolve => worker.onerror = resolve);
// Also, it should terminate the worker thread.
await new Promise(resolve => {
const timer = setInterval(() => {
if (window.internals.workerThreadCount === 0) {
clearInterval(timer);
resolve();
}
}, 10);
});
}, `A failure on ${fetch_type} fetch should terminate the worker`);
}
// Loading non-existent top-level classic worker script should terminate the
// worker.
run_test('resources/non-existent-script.js',
{ type: 'classic' },
'top-level classic worker script');
// Loading non-existent top-level module worker script should terminate the
// worker.
run_test('resources/non-existent-script.js',
{ type: 'module' },
'top-level module worker script');
// Static import to non-existent module worker script should terminate the
// worker.
run_test('resources/worker-static-import-nonexistent-script.js',
{ type: 'module' },
'static import');
</script>
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