New issue
Advanced search Search tips

Issue 755054 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Failing to load installed scripts should terminate the worker thread

Project Member Reported by nhiroki@chromium.org, Aug 14 2017

Issue description

When WorkerThread fails to load installed scrips in InitializeOnWorkerThread(), it sets |should_terminate| and calls PrepareForShutdownOnWorkerThread(). However, this is not sufficient because PrepareForShutdownOnWorkerThread() doesn't terminate the thread but just prepare for the termination. To actually terminate it, WorkerThread should call Terminate() from the main thread.
 
Status: Started (was: Assigned)
WIP CL: https://chromium-review.googlesource.com/c/612923
Cc: -shimazu@chromium.org nhiroki@chromium.org
Owner: shimazu@chromium.org
Status: Assigned (was: Started)
Let me hand over this to shimazu-san.
Cc: -nhiroki@chromium.org shimazu@chromium.org
Components: Blink>Workers
Owner: nhiroki@chromium.org
Status: Started (was: Assigned)
crrev.com/c/701854 will solve this issue
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4404c2f93e2d2ccacf15b07c834bff8051ddb63

commit e4404c2f93e2d2ccacf15b07c834bff8051ddb63
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Oct 05 07:08:46 2017

ServiceWorker: Move InstalledScriptsManager access from WorkerThread to SWGlobalScope

For cleanup, this CL moves InstalledScriptsManager access from
WorkerThread::InitializeOnWorkerThread() to
ServiceWorkerGlobalScope::EvaluateClassicScript() because only ServiceWorker
needs to access the manager and placing it in EvaluateClassicScript() looks more
natural like importScripts().

<Motivation>

My final goal is to remove script evaluation code from WorkerThread and leave it
up to each owner of WorkerThread so that WorkerThead is no longer concerned with
how to evaluate scripts (i.e., "classic", "module", lazy eval for worklets).

For example, for DedicatedWorker, DedicatedWorkerMessagingProxy may post a task
to call WorkerGlobalScope::EvaluateClassicScript() or
WorkerGlobalScope::ImportModuleScript() (to be added later) based on
"WorkerType"[1] after initializing DedicatedWorkerThread.

[1] https://html.spec.whatwg.org/multipage/workers.html#workertype

Bug: 680046,  753350 ,  755054 
Change-Id: I546cbda16f09da66c3f94c319e498055e1f9d05b
Reviewed-on: https://chromium-review.googlesource.com/701854
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506670}
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/core/workers/WorkerGlobalScope.h
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.h
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerThread.cpp
[modify] https://crrev.com/e4404c2f93e2d2ccacf15b07c834bff8051ddb63/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerThread.h

Labels: -M-62 M-63
Status: Fixed (was: Started)

Sign in to add a comment