Worker: Wait for termination by WorkerThreadLifecycleObserver instead of WaitableEvent

Worker: Wait for termination by WorkerThreadLifecycleObserver instead of WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
Committed: https://crrev.com/8900a3ebc6d4be184e38ca43c3142304d37ab044
Cr-Commit-Position: refs/heads/master@{#413672}

[nhiroki, 2016-08-22 12:08:43]

The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-22 12:08:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nhiroki, 2016-08-22 12:23:03]

Description was changed from

==========
(WIP) Worker: Remove m_terminationEvent from WorkerThread

BUG=
==========

to

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

[nhiroki, 2016-08-22 12:23:17]

Patchset #1 (id:1) has been deleted

[nhiroki, 2016-08-22 12:23:25]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2016-08-22 12:23:46]

Description was changed from

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

to

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

[nhiroki, 2016-08-22 12:26:13]

nhiroki@chromium.org changed reviewers:
+ yhirano@chromium.org

[nhiroki, 2016-08-22 12:26:14]

PTAL, thanks!

[commit-bot: I haz the power, 2016-08-22 13:32:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-22 13:32:47]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-08-23 02:23:51]

lgtm

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp (right):

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:35: class
WorkerThreadLifecycleObserverForTest : public
GarbageCollectedFinalized<WorkerThreadLifecycleObserverForTest>, public
WorkerThreadLifecycleObserver {
+final

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:40: :
WorkerThreadLifecycleObserver(context) {}
DCHECK(!wasContextDestroyedBeforeObserverCreation());

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:216:
std::unique_ptr<WaitableEvent> waitableEvent(new
WaitableEvent(WaitableEvent::ResetPolicy::Manual,
WaitableEvent::InitialState::NonSignaled));
Why do we need unique_ptr here? Isn't it enough to instantiate WaitableEvent
directly?

[nhiroki, 2016-08-23 03:27:25]

Thank you for reviewing.

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp (right):

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:35: class
WorkerThreadLifecycleObserverForTest : public
GarbageCollectedFinalized<WorkerThreadLifecycleObserverForTest>, public
WorkerThreadLifecycleObserver {
On 2016/08/23 02:23:51, yhirano wrote:
> +final

Done.

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:40: :
WorkerThreadLifecycleObserver(context) {}
On 2016/08/23 02:23:51, yhirano wrote:
> DCHECK(!wasContextDestroyedBeforeObserverCreation());

Done.

https://codereview.chromium.org/2262023002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:216:
std::unique_ptr<WaitableEvent> waitableEvent(new
WaitableEvent(WaitableEvent::ResetPolicy::Manual,
WaitableEvent::InitialState::NonSignaled));
On 2016/08/23 02:23:51, yhirano wrote:
> Why do we need unique_ptr here? Isn't it enough to instantiate WaitableEvent
> directly?

It's enough. The latest patchset just instantiates it.

[nhiroki, 2016-08-23 03:27:29]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2016-08-23 03:27:29]

The patchset sent to the CQ was uploaded after l-g-t-m from yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2262023002/#ps60001
(title: "address review comments")

[commit-bot: I haz the power, 2016-08-23 03:27:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 05:23:26]

Description was changed from

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

to

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

[commit-bot: I haz the power, 2016-08-23 05:23:27]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-08-23 05:27:58]

Description was changed from

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153
==========

to

==========
Worker: Wait for termination by WorkerThreadLifecycleObserver instead of
WaitableEvent

WaitableEvent |WorkerThread::m_terminationEvent| is used to wait for worker
thread termination in a unit test. This should be replaced with saner
termination observation mechanism, that is, WorkerThreadLifecycleObserver.

BUG=639153

Committed: https://crrev.com/8900a3ebc6d4be184e38ca43c3142304d37ab044
Cr-Commit-Position: refs/heads/master@{#413672}
==========

[commit-bot: I haz the power, 2016-08-23 05:28:00]

Patchset 2 (id:??) landed as
https://crrev.com/8900a3ebc6d4be184e38ca43c3142304d37ab044
Cr-Commit-Position: refs/heads/master@{#413672}