Remove WorkerThread::terminateAndWait() |
|||
Issue descriptionCurrently, in production code, WorkerThread::terminateAndWait() is called only from WebEmbeddedWorkerImpl::~WebEmbeddedWorkerImpl(). It'd be nice if we can remove it or make it test-only function because it sometime causes crash bugs (see issue 487050 for details)
,
Aug 29 2016
WIP CL is here: https://codereview.chromium.org/2284303002/ This seems to be blocked by issue 639244 that abandons graceful renderer process shutdown (see also the CL description)
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15e3d6427580e9418b98aa765a4a2600acec5807 commit 15e3d6427580e9418b98aa765a4a2600acec5807 Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Fri Jun 30 12:31:15 2017 ServiceWorker: Remove WorkerThread::TerminateAndWait() call from WebEmbeddedWorkerImpl This CL removes WorkerThread::TerminateAndWait() call from the dtor of WebEmbeddedWorkerImpl. WorkerThread should already be terminated or should not be started yet at the point as follows: Note that WebEmbeddedWorkerImpl::TerminateWorkerContext() is the entry point to terminate WorkerThread and must always be called before the dtor. Case 1) TerminateWorkerContext() is called while WorkerThread is running. WebEmbeddedWorkerImpl::TerminateWorkerContext WorkerThread::Terminate (thread temination runs) WorkerThread::PerformShutdownOnWorkerThread ServiceWorkerGlobalScopeProxy::DidTerminateWorkerThread ServiceWorkerContextClient::WorkerContextDestroyed EmbeddedWorkerInstanceClientImpl::WorkerContextDestroyed EmbeddedWorkerInstanceClientImpl::WorkerWrapper::~WorkerWrapper -> WebEmbeddedWorkerImpl is destroyed here. Case 2) TerminateWorkerContext() is called before WorkerThread starts. WebEmbeddedWorkerImpl::TerminateWorkerContext ServiceWorkerContextClient::WorkerContextFailedToStart EmbeddedWorkerInstanceClientImpl::WorkerContextDestroyed EmbeddedWorkerInstanceClientImpl::WorkerWrapper::~WorkerWrapper -> WebEmbeddedWorkerImpl is destroyed here. Bug: 641846 Change-Id: I1df721bff395449f1da20c247e1e24063c4dc836 Reviewed-on: https://chromium-review.googlesource.com/554442 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#483684} [modify] https://crrev.com/15e3d6427580e9418b98aa765a4a2600acec5807/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp [modify] https://crrev.com/15e3d6427580e9418b98aa765a4a2600acec5807/third_party/WebKit/Source/modules/serviceworkers/WebEmbeddedWorkerImplTest.cpp
,
Jul 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/426f46f231759c646bf2357be299f87d2c966260 commit 426f46f231759c646bf2357be299f87d2c966260 Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Mon Jul 03 03:09:58 2017 Worker: Remove WorkerThread::TerminateAndWait() WorkerThread::TerminateAndWait() is used only in unit tests and can be replaced with Terminate() and WaitForShutdownForTesting(). Bug: 641846 Change-Id: I763256be20d1aa76c0c7d9448e2b854c7ce9602c Reviewed-on: https://chromium-review.googlesource.com/558324 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#483928} [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/core/workers/ThreadedWorkletTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/core/workers/WorkerThread.h [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScopeTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp [modify] https://crrev.com/426f46f231759c646bf2357be299f87d2c966260/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
,
Jul 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dcd677d1b401a3e9b4d1af21410764ba971b7cb commit 2dcd677d1b401a3e9b4d1af21410764ba971b7cb Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Mon Jul 03 03:55:18 2017 Worker: Avoid duplicate thread state checks This is a cleanup and should not change existing behavior. Bug: 641846 Change-Id: Ieeae827f6ea506f939236c894ef3ecd57e812a06 Reviewed-on: https://chromium-review.googlesource.com/558844 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#483934} [modify] https://crrev.com/2dcd677d1b401a3e9b4d1af21410764ba971b7cb/third_party/WebKit/Source/core/workers/WorkerThread.cpp
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4d76ff5f3e0dd3177c72c5e848ff40349375c41e commit 4d76ff5f3e0dd3177c72c5e848ff40349375c41e Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Wed Jul 05 01:36:55 2017 Worker: Separate synchronous worker termination from production code Synchronous termination is now used only for the leak detector via TerminateAndWaitForAllWorkers(), that is, production code no longer depends on it. To simplify the production code, this CL pushes the code into TerminateAndWaitForAllWorkers(), and renames it to TerminateAllWorkersForTesting(). Bug: 641846 Change-Id: Ia2c5372394cf0ddfdf038c8ceb81a6655d7c564f Reviewed-on: https://chromium-review.googlesource.com/558596 Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#484173} [modify] https://crrev.com/4d76ff5f3e0dd3177c72c5e848ff40349375c41e/third_party/WebKit/Source/core/workers/WorkerThread.cpp [modify] https://crrev.com/4d76ff5f3e0dd3177c72c5e848ff40349375c41e/third_party/WebKit/Source/core/workers/WorkerThread.h [modify] https://crrev.com/4d76ff5f3e0dd3177c72c5e848ff40349375c41e/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp [modify] https://crrev.com/4d76ff5f3e0dd3177c72c5e848ff40349375c41e/third_party/WebKit/Source/modules/exported/WebLeakDetector.cpp
,
Jul 5 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by nhiroki@chromium.org
, Aug 29 2016