Commit 39f3a8e9 authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

service worker: Terminate when loading the installed script failed.

Before this CL, we called DidEvaluateClassicScript() with false,
but this doesn't actually terminate the thread and is counted as a
successful startup by the browser. It's only terminated in when
the script is being installed, by ServiceWorkerRegisterJob.

Since M69, we'd get a process kill because the timing milestones
wouldn't make sense, as script evaluate start would be null (zero).

This CL makes startup fail, sends OnStopped to the browser process,
and terminates the worker thread.

Bug: 881100
Change-Id: I452986d8a51344a7ba465fd5c91861bf2443e15e
Reviewed-on: https://chromium-review.googlesource.com/1208997
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589125}
parent 32261aff
...@@ -811,6 +811,19 @@ void ServiceWorkerContextClient::WorkerContextFailedToStart() { ...@@ -811,6 +811,19 @@ void ServiceWorkerContextClient::WorkerContextFailedToStart() {
embedded_worker_client_->WorkerContextDestroyed(); embedded_worker_client_->WorkerContextDestroyed();
} }
void ServiceWorkerContextClient::FailedToLoadInstalledScript() {
DCHECK(worker_task_runner_->RunsTasksInCurrentSequence());
TRACE_EVENT_NESTABLE_ASYNC_END1("ServiceWorker", "LOAD_SCRIPT", this,
"Status", "FailedToLoadInstalledScript");
// Cleanly send an OnStopped() message instead of just breaking the
// Mojo connection on termination, for consistency with the other
// startup failure paths.
(*instance_host_)->OnStopped();
// The caller is responsible for terminating the thread which
// eventually destroys |this|.
}
void ServiceWorkerContextClient::WorkerScriptLoaded() { void ServiceWorkerContextClient::WorkerScriptLoaded() {
if (!is_starting_installed_worker_) if (!is_starting_installed_worker_)
(*instance_host_)->OnScriptLoaded(); (*instance_host_)->OnScriptLoaded();
......
...@@ -129,6 +129,7 @@ class CONTENT_EXPORT ServiceWorkerContextClient ...@@ -129,6 +129,7 @@ class CONTENT_EXPORT ServiceWorkerContextClient
void ClearCachedMetadata(const blink::WebURL&) override; void ClearCachedMetadata(const blink::WebURL&) override;
void WorkerReadyForInspection() override; void WorkerReadyForInspection() override;
void WorkerContextFailedToStart() override; void WorkerContextFailedToStart() override;
void FailedToLoadInstalledScript() override;
void WorkerScriptLoaded() override; void WorkerScriptLoaded() override;
void WorkerContextStarted( void WorkerContextStarted(
blink::WebServiceWorkerContextProxy* proxy) override; blink::WebServiceWorkerContextProxy* proxy) override;
......
...@@ -102,6 +102,10 @@ class WebServiceWorkerContextClient { ...@@ -102,6 +102,10 @@ class WebServiceWorkerContextClient {
// completed. Called on the main thread. // completed. Called on the main thread.
virtual void WorkerContextFailedToStart() {} virtual void WorkerContextFailedToStart() {}
// The worker started but it could not execute because loading the
// installed script failed.
virtual void FailedToLoadInstalledScript() {}
// The worker script successfully loaded. Called on the main thread when the // The worker script successfully loaded. Called on the main thread when the
// script is served from ResourceLoader or on the worker thread when the // script is served from ResourceLoader or on the worker thread when the
// script is served via WebServiceWorkerInstalledScriptsManager. // script is served via WebServiceWorkerInstalledScriptsManager.
......
...@@ -136,8 +136,7 @@ void ServiceWorkerGlobalScope::EvaluateClassicScript( ...@@ -136,8 +136,7 @@ void ServiceWorkerGlobalScope::EvaluateClassicScript(
InstalledScriptsManager::ScriptStatus status = InstalledScriptsManager::ScriptStatus status =
installed_scripts_manager->GetScriptData(script_url, &script_data); installed_scripts_manager->GetScriptData(script_url, &script_data);
if (status == InstalledScriptsManager::ScriptStatus::kFailed) { if (status == InstalledScriptsManager::ScriptStatus::kFailed) {
// This eventually terminates the worker thread. close();
ReportingProxy().DidEvaluateClassicScript(false);
return; return;
} }
......
...@@ -668,9 +668,30 @@ void ServiceWorkerGlobalScopeProxy::DidEvaluateClassicScript(bool success) { ...@@ -668,9 +668,30 @@ void ServiceWorkerGlobalScopeProxy::DidEvaluateClassicScript(bool success) {
} }
void ServiceWorkerGlobalScopeProxy::DidCloseWorkerGlobalScope() { void ServiceWorkerGlobalScopeProxy::DidCloseWorkerGlobalScope() {
// This should never be called because close() is not defined in DCHECK(WorkerGlobalScope()->IsContextThread());
// ServiceWorkerGlobalScope. // close() is not web-exposed. This is called when ServiceWorkerGlobalScope
NOTREACHED(); // internally requests close() due to failure on startup when installed
// scripts couldn't be read.
//
// This may look like a roundabout way to terminate the thread, but close()
// seems like the standard way to initiate termination from inside the thread.
// Tell ServiceWorkerContextClient about the failure. The generic
// WorkerContextFailedToStart() wouldn't make sense because
// WorkerContextStarted() was already called.
Client().FailedToLoadInstalledScript();
// ServiceWorkerGlobalScope expects us to terminate the thread, so request
// that here.
PostCrossThreadTask(
*parent_execution_context_task_runners_->Get(TaskType::kInternalDefault),
FROM_HERE,
CrossThreadBind(&WebEmbeddedWorkerImpl::TerminateWorkerContext,
CrossThreadUnretained(embedded_worker_)));
// NOTE: WorkerThread calls WillDestroyWorkerGlobalScope() synchronously after
// this function returns, since it calls DidCloseWorkerGlobalScope() then
// PrepareForShutdownOnWorkerThread().
} }
void ServiceWorkerGlobalScopeProxy::WillDestroyWorkerGlobalScope() { void ServiceWorkerGlobalScopeProxy::WillDestroyWorkerGlobalScope() {
...@@ -682,7 +703,7 @@ void ServiceWorkerGlobalScopeProxy::WillDestroyWorkerGlobalScope() { ...@@ -682,7 +703,7 @@ void ServiceWorkerGlobalScopeProxy::WillDestroyWorkerGlobalScope() {
} }
void ServiceWorkerGlobalScopeProxy::DidTerminateWorkerThread() { void ServiceWorkerGlobalScopeProxy::DidTerminateWorkerThread() {
// This should be called after WillDestroyWorkerGlobalScope(). // This must be called after WillDestroyWorkerGlobalScope().
DCHECK(!worker_global_scope_); DCHECK(!worker_global_scope_);
Client().WorkerContextDestroyed(); Client().WorkerContextDestroyed();
} }
...@@ -709,6 +730,7 @@ void ServiceWorkerGlobalScopeProxy::Detach() { ...@@ -709,6 +730,7 @@ void ServiceWorkerGlobalScopeProxy::Detach() {
} }
void ServiceWorkerGlobalScopeProxy::TerminateWorkerContext() { void ServiceWorkerGlobalScopeProxy::TerminateWorkerContext() {
DCHECK(IsMainThread());
embedded_worker_->TerminateWorkerContext(); embedded_worker_->TerminateWorkerContext();
} }
......
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