Commit 84f37f06 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

Worker: Stop calling 'delete this' in EmbeddedSharedWorkerStub::WorkerScriptLoadFailed()

This CL stops calling 'delete this' in WorkerScriptLoadFailed(), and instead
makes sure WorkerContextDestroyed() is called in any case.

This cleanup is necessary for off-the-main-thread shared worker script fetch
because when the feature is enabled, we cannot destroy the worker at
WorkerScriptLoadFailed(), and need to wait for completion of worker thread
termination.

Bug: 924041
Change-Id: Ia4b744f1d7302efbc53fdc09e42e414a7b768865
Reviewed-on: https://chromium-review.googlesource.com/c/1441735Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626970}
parent 67bee393
...@@ -208,7 +208,6 @@ void EmbeddedSharedWorkerStub::WorkerScriptLoaded() { ...@@ -208,7 +208,6 @@ void EmbeddedSharedWorkerStub::WorkerScriptLoaded() {
void EmbeddedSharedWorkerStub::WorkerScriptLoadFailed() { void EmbeddedSharedWorkerStub::WorkerScriptLoadFailed() {
host_->OnScriptLoadFailed(); host_->OnScriptLoadFailed();
pending_channels_.clear(); pending_channels_.clear();
delete this;
} }
void EmbeddedSharedWorkerStub::WorkerScriptEvaluated(bool success) { void EmbeddedSharedWorkerStub::WorkerScriptEvaluated(bool success) {
......
...@@ -49,10 +49,7 @@ struct NavigationResponseOverrideParameters; ...@@ -49,10 +49,7 @@ struct NavigationResponseOverrideParameters;
// A stub class to receive IPC from browser process and talk to // A stub class to receive IPC from browser process and talk to
// blink::WebSharedWorker. Implements blink::WebSharedWorkerClient. // blink::WebSharedWorker. Implements blink::WebSharedWorkerClient.
// This class is self-destructed (no one explicitly owns this). It deletes // This class is self-destructed (no one explicitly owns this). It deletes
// itself when either one of following methods is called by // itself when WorkerContextDestroyed() is called by blink::WebSharedWorker.
// blink::WebSharedWorker:
// - WorkerScriptLoadFailed() or
// - WorkerContextDestroyed()
// //
// This class owns blink::WebSharedWorker. // This class owns blink::WebSharedWorker.
class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient, class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
......
...@@ -98,6 +98,9 @@ void WebSharedWorkerImpl::TerminateWorkerThread() { ...@@ -98,6 +98,9 @@ void WebSharedWorkerImpl::TerminateWorkerThread() {
asked_to_terminate_ = true; asked_to_terminate_ = true;
if (shadow_page_ && !shadow_page_->WasInitialized()) { if (shadow_page_ && !shadow_page_->WasInitialized()) {
client_->WorkerScriptLoadFailed(); client_->WorkerScriptLoadFailed();
// The worker thread hasn't been started yet. Immediately notify the client
// of worker termination.
client_->WorkerContextDestroyed();
// |this| is deleted at this point. // |this| is deleted at this point.
return; return;
} }
...@@ -105,6 +108,9 @@ void WebSharedWorkerImpl::TerminateWorkerThread() { ...@@ -105,6 +108,9 @@ void WebSharedWorkerImpl::TerminateWorkerThread() {
main_script_loader_->Cancel(); main_script_loader_->Cancel();
main_script_loader_ = nullptr; main_script_loader_ = nullptr;
client_->WorkerScriptLoadFailed(); client_->WorkerScriptLoadFailed();
// The worker thread hasn't been started yet. Immediately notify the client
// of worker termination.
client_->WorkerContextDestroyed();
// |this| is deleted at this point. // |this| is deleted at this point.
return; return;
} }
...@@ -166,6 +172,13 @@ void WebSharedWorkerImpl::DidFetchScript() { ...@@ -166,6 +172,13 @@ void WebSharedWorkerImpl::DidFetchScript() {
client_->WorkerScriptLoaded(); client_->WorkerScriptLoaded();
} }
void WebSharedWorkerImpl::DidFailToFetchClassicScript() {
DCHECK(IsMainThread());
client_->WorkerScriptLoadFailed();
TerminateWorkerThread();
// DidTerminateWorkerThread() will be called asynchronously.
}
void WebSharedWorkerImpl::DidEvaluateClassicScript(bool success) { void WebSharedWorkerImpl::DidEvaluateClassicScript(bool success) {
DCHECK(IsMainThread()); DCHECK(IsMainThread());
client_->WorkerScriptEvaluated(success); client_->WorkerScriptEvaluated(success);
...@@ -258,7 +271,11 @@ void WebSharedWorkerImpl::OnScriptLoaderFinished() { ...@@ -258,7 +271,11 @@ void WebSharedWorkerImpl::OnScriptLoaderFinished() {
return; return;
if (main_script_loader_->Failed()) { if (main_script_loader_->Failed()) {
main_script_loader_->Cancel(); main_script_loader_->Cancel();
main_script_loader_ = nullptr;
client_->WorkerScriptLoadFailed(); client_->WorkerScriptLoadFailed();
// The worker thread hasn't been started yet. Immediately notify the client
// of worker termination.
client_->WorkerContextDestroyed();
// |this| is deleted at this point. // |this| is deleted at this point.
return; return;
} }
......
...@@ -110,6 +110,7 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker, ...@@ -110,6 +110,7 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker,
// Callback methods for SharedWorkerReportingProxy. // Callback methods for SharedWorkerReportingProxy.
void CountFeature(WebFeature); void CountFeature(WebFeature);
void DidFetchScript(); void DidFetchScript();
void DidFailToFetchClassicScript();
void DidEvaluateClassicScript(bool success); void DidEvaluateClassicScript(bool success);
void DidCloseWorkerGlobalScope(); void DidCloseWorkerGlobalScope();
void DidTerminateWorkerThread(); void DidTerminateWorkerThread();
......
...@@ -75,9 +75,15 @@ void SharedWorkerReportingProxy::DidFetchScript() { ...@@ -75,9 +75,15 @@ void SharedWorkerReportingProxy::DidFetchScript() {
} }
void SharedWorkerReportingProxy::DidFailToFetchClassicScript() { void SharedWorkerReportingProxy::DidFailToFetchClassicScript() {
// TODO(nhiroki): Add a runtime flag check for off-the-main-thread shared
// worker script fetch. This function should be called only when the flag is
// enabled (https://crbug.com/924041).
DCHECK(!IsMainThread()); DCHECK(!IsMainThread());
// TODO(nhiroki): Dispatch an error event at the SharedWorker object in the PostCrossThreadTask(
// parent document. *parent_execution_context_task_runners_->Get(TaskType::kInternalDefault),
FROM_HERE,
CrossThreadBind(&WebSharedWorkerImpl::DidFailToFetchClassicScript,
CrossThreadUnretained(worker_)));
} }
void SharedWorkerReportingProxy::DidFailToFetchModuleScript() { void SharedWorkerReportingProxy::DidFailToFetchModuleScript() {
......
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