Worker: Unify worker thread shutdown sequence

Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843
Committed: https://crrev.com/cfa01627d191d2a1a801943994f287b8cf5b1c42
Cr-Commit-Position: refs/heads/master@{#420592}

[nhiroki, 2016-08-25 04:38:54]

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

[commit-bot: I haz the power, 2016-08-25 04:39:26]

Dry run: 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-25 04:46:05]

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

[commit-bot: I haz the power, 2016-08-25 04:46:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nhiroki, 2016-08-25 05:53:48]

Description was changed from

==========
Worker: Unify worker thread shutdown sequence

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

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:
  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Initialize WorkerThread and switch to termination sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence cycles worker's
message loop after stopping posting a task to ensure that drain all posted tasks
from the main thread, so this can avoid that posted tasks run after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-08-25 06:26:19]

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

[nhiroki, 2016-08-25 06:26:25]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-08-25 06:26:42]

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-25 06:47:53]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:
  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Initialize WorkerThread and switch to termination sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence cycles worker's
message loop after stopping posting a task to ensure that drain all posted tasks
from the main thread, so this can avoid that posted tasks run after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to termination sequence on the
     worker thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks to the worker thread, and it drains tasks posted from the main thread(*),
so this can avoid that posted tasks run after shutdown.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread can be
still remaining even after the shutdown tasks run. We can consider this in
2 senarios:

  1) When the termination happens before initialization, an uninitialized worker
     thread does not post a task to itself, so there should not be tasks posted
     from the worker thread in the task queue.
  2) When termination happens after initialization, shutdown sequence drains
     all posted tasks in the task queue by WorkerBackingThread::shutdown.

Therefore, we can ensure that tasks posted from the worker thread never run
after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-08-25 06:49:42]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to termination sequence on the
     worker thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks to the worker thread, and it drains tasks posted from the main thread(*),
so this can avoid that posted tasks run after shutdown.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread can be
still remaining even after the shutdown tasks run. We can consider this in
2 senarios:

  1) When the termination happens before initialization, an uninitialized worker
     thread does not post a task to itself, so there should not be tasks posted
     from the worker thread in the task queue.
  2) When termination happens after initialization, shutdown sequence drains
     all posted tasks in the task queue by WorkerBackingThread::shutdown.

Therefore, we can ensure that tasks posted from the worker thread never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks to the worker thread, and it drains tasks posted from the main thread(*),
so this can avoid that posted tasks run after shutdown.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread can be
still remaining even after the shutdown tasks run. We can consider this in
2 senarios:

  1) When the termination happens before initialization, an uninitialized worker
     thread does not post a task to itself, so there should not be tasks posted
     from the worker thread in the task queue.
  2) When termination happens after initialization, shutdown sequence drains
     all posted tasks in the task queue by WorkerBackingThread::shutdown.

Therefore, we can ensure that tasks posted from the worker thread never run
after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-08-25 06:59:34]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks to the worker thread, and it drains tasks posted from the main thread(*),
so this can avoid that posted tasks run after shutdown.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread can be
still remaining even after the shutdown tasks run. We can consider this in
2 senarios:

  1) When the termination happens before initialization, an uninitialized worker
     thread does not post a task to itself, so there should not be tasks posted
     from the worker thread in the task queue.
  2) When termination happens after initialization, shutdown sequence drains
     all posted tasks in the task queue by WorkerBackingThread::shutdown.

Therefore, we can ensure that tasks posted from the worker thread never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread cannot be
guaranteed to be drained until the shutdown tasks run. This case is covered by
other mechanism. We can consider it in 2 senarios:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Shutdown sequence drains all posted tasks by WorkerBackingThread::shutdown.
     (Note that this is called only when the worker thread is initialized.)

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

[commit-bot: I haz the power, 2016-08-25 07:07:51]

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

[commit-bot: I haz the power, 2016-08-25 07:07:52]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-08-25 07:12:45]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from the worker thread cannot be
guaranteed to be drained until the shutdown tasks run. This case is covered by
other mechanism. We can consider it in 2 senarios:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Shutdown sequence drains all posted tasks by WorkerBackingThread::shutdown.
     (Note that this is called only when the worker thread is initialized.)

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in 2 senarios:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-08-25 07:23:16]

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

[commit-bot: I haz the power, 2016-08-25 07:23:32]

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-25 07:24:03]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2016-08-25 07:25:01]

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

[nhiroki, 2016-08-25 07:25:01]

PTAL, thanks!

[haraken, 2016-08-25 08:04:32]

LGTM. Great CL description.

If it's very rare that a worker thread is requested to terminate before the
worker thread starts running, would it be an option to just unconditionally
initialize the worker thread (even if a termination request has arrived when the
worker thread starts the initialization sequence)?

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

https://codereview.chromium.org/2280523002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:535: if (m_exitCode ==
ExitCode::NotTerminated)

Maybe do we want to move line 535 and 536 up to before line 532?

[nhiroki, 2016-08-25 08:43:27]

Thank you for reviewing.

On 2016/08/25 08:04:32, haraken wrote:
> LGTM. Great CL description.
> 
> If it's very rare that a worker thread is requested to terminate before the
> worker thread starts running, would it be an option to just unconditionally
> initialize the worker thread (even if a termination request has arrived when
the
> worker thread starts the initialization sequence)?

Good idea. That would be feasible and simper. I'll try it in this CL :)

[commit-bot: I haz the power, 2016-08-25 09:07:10]

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

[commit-bot: I haz the power, 2016-08-25 09:07:11]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-08-26 06:10:22]

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

[commit-bot: I haz the power, 2016-08-26 06:10:45]

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-26 06:16:54]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in 2 senarios:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-08-26 06:47:55]

Updated. Can you take another look? Thanks.

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

https://codereview.chromium.org/2280523002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:535: if (m_exitCode ==
ExitCode::NotTerminated)
On 2016/08/25 08:04:31, haraken wrote:
> 
> Maybe do we want to move line 535 and 536 up to before line 532?

WONTFIX: This part is obsolete in the patchset 2.

(I assume that you meant we can merge if-statement at line 537 into line 532)

[haraken, 2016-08-26 07:32:52]

This looks much simpler! LGTM.

[commit-bot: I haz the power, 2016-08-26 07:36:44]

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

[commit-bot: I haz the power, 2016-08-26 07:36:45]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-08-29 06:56:20]

https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:408: bool
shouldScheduleToTerminateExecution = (m_threadState == ThreadState::Running) &&
!m_runningDebuggerTask;
IIUC m_threadState could be NotStarted when the initialization task has been
posted to the worker thread but it has not been executed yet. In that case that
initialization task might block forever as it runs the given script, and hence I
think shouldScheduleToTerminateExecution should be true.

[nhiroki, 2016-08-30 01:27:44]

Patchset #3 (id:80001) has been deleted

[nhiroki, 2016-08-30 01:35:58]

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

[nhiroki, 2016-08-30 01:52:31]

Thank you for your comment. PTAL.

https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:408: bool
shouldScheduleToTerminateExecution = (m_threadState == ThreadState::Running) &&
!m_runningDebuggerTask;
On 2016/08/29 06:56:20, yhirano wrote:
> IIUC m_threadState could be NotStarted when the initialization task has been
> posted to the worker thread but it has not been executed yet. In that case
that
> initialization task might block forever as it runs the given script, and hence
I
> think shouldScheduleToTerminateExecution should be true.

Very good point. I made the latest patchset to address this case not here but in
initializeOnWorkerThread().

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (left):

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:665: // main thread
before shutdown preparation.
I'm now thinking that running a forcible termination task between here and
prepareForShutdownOnWorkerThread() is not harmful, so unconditionally flip over
|m_runningDebuggerTask|. Can you double-check this is true?

This enables to unify prepareForShutdownOnWorkerThread() calls in
initializeOnWorkerThread() and makes codepath simpler.

[commit-bot: I haz the power, 2016-08-30 02:45:06]

Dry run: 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-30 04:45:18]

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

[commit-bot: I haz the power, 2016-08-30 04:45:19]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nhiroki, 2016-08-30 04:57:18]

On 2016/08/30 01:52:31, nhiroki wrote:
> Thank you for your comment. PTAL.
> 
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):
> 
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/workers/WorkerThread.cpp:408: bool
> shouldScheduleToTerminateExecution = (m_threadState == ThreadState::Running)
&&
> !m_runningDebuggerTask;
> On 2016/08/29 06:56:20, yhirano wrote:
> > IIUC m_threadState could be NotStarted when the initialization task has been
> > posted to the worker thread but it has not been executed yet. In that case
> that
> > initialization task might block forever as it runs the given script, and
hence
> I
> > think shouldScheduleToTerminateExecution should be true.
> 
> Very good point. I made the latest patchset to address this case not here but
in
> initializeOnWorkerThread().
> 
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/workers/WorkerThread.cpp (left):
> 
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/workers/WorkerThread.cpp:665: // main thread
> before shutdown preparation.
> I'm now thinking that running a forcible termination task between here and
> prepareForShutdownOnWorkerThread() is not harmful, so unconditionally flip
over
> |m_runningDebuggerTask|. Can you double-check this is true?
> 
> This enables to unify prepareForShutdownOnWorkerThread() calls in
> initializeOnWorkerThread() and makes codepath simpler.

Hmm..., this change is causing test failures because it varies test results
depending on timing. I'll fix it and ask you review this again.

[yhirano, 2016-08-30 05:00:45]

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:511:
prepareForShutdownOnWorkerThread();
Sorry I don't understand why this call is needed. terminateInternal has already
post prepareForShutdownOnWorkerThread, right?

[nhiroki, 2016-09-06 08:35:11]

I just resumed this CL. Sorry for my delayed reply.

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:511:
prepareForShutdownOnWorkerThread();
On 2016/08/30 05:00:45, yhirano wrote:
> Sorry I don't understand why this call is needed. terminateInternal has
already
> post prepareForShutdownOnWorkerThread, right?

This is necessary to stop further worker tasks from running (i.e., set
|m_threadState| to ThreadState::ReadyToShutdown) between this point and a
shutdown task posted from the main thread.

[nhiroki, 2016-09-07 05:38:50]

On 2016/08/30 04:57:18, nhiroki wrote:
> On 2016/08/30 01:52:31, nhiroki wrote:
> > Thank you for your comment. PTAL.
> > 
> >
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/workers/WorkerThread.cpp:408: bool
> > shouldScheduleToTerminateExecution = (m_threadState == ThreadState::Running)
> &&
> > !m_runningDebuggerTask;
> > On 2016/08/29 06:56:20, yhirano wrote:
> > > IIUC m_threadState could be NotStarted when the initialization task has
been
> > > posted to the worker thread but it has not been executed yet. In that case
> > that
> > > initialization task might block forever as it runs the given script, and
> hence
> > I
> > > think shouldScheduleToTerminateExecution should be true.
> > 
> > Very good point. I made the latest patchset to address this case not here
but
> in
> > initializeOnWorkerThread().
> > 
> >
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/workers/WorkerThread.cpp (left):
> > 
> >
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/workers/WorkerThread.cpp:665: // main thread
> > before shutdown preparation.
> > I'm now thinking that running a forcible termination task between here and
> > prepareForShutdownOnWorkerThread() is not harmful, so unconditionally flip
> over
> > |m_runningDebuggerTask|. Can you double-check this is true?
> > 
> > This enables to unify prepareForShutdownOnWorkerThread() calls in
> > initializeOnWorkerThread() and makes codepath simpler.
> 
> Hmm..., this change is causing test failures because it varies test results
> depending on timing. I'll fix it and ask you review this again.

Oops, sorry! I forgot to address this issue... I'll ask you to review this again
after fixing it.

[nhiroki, 2016-09-07 06:48:15]

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

[commit-bot: I haz the power, 2016-09-07 06:48:31]

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

[nhiroki, 2016-09-07 08:43:35]

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

[commit-bot: I haz the power, 2016-09-07 08:43:56]

Dry run: 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-09-07 10:13:11]

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

[commit-bot: I haz the power, 2016-09-07 10:13:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nhiroki, 2016-09-13 01:31:08]

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

[commit-bot: I haz the power, 2016-09-13 01:32:08]

Dry run: 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-09-13 03:29:55]

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

[commit-bot: I haz the power, 2016-09-13 03:29:56]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nhiroki, 2016-09-23 04:41:25]

Patchset #7 (id:180001) has been deleted

[nhiroki, 2016-09-23 04:41:33]

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

[commit-bot: I haz the power, 2016-09-23 04:41:52]

Dry run: 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-09-23 06:16:09]

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

[commit-bot: I haz the power, 2016-09-23 06:16:10]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-09-23 06:51:07]

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

[commit-bot: I haz the power, 2016-09-23 06:51:19]

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

[nhiroki, 2016-09-23 06:54:46]

yhirano@, sorry for taking a long time. The test failures are now fixed. Can you
take another look? Thanks!

On 2016/09/07 05:38:50, nhiroki (ooo until Sep. 30) wrote:
> On 2016/08/30 04:57:18, nhiroki wrote:
> > On 2016/08/30 01:52:31, nhiroki wrote:
> > > Thank you for your comment. PTAL.
> > > 
> > >
> >
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2280523002/diff/60001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/workers/WorkerThread.cpp:408: bool
> > > shouldScheduleToTerminateExecution = (m_threadState ==
ThreadState::Running)
> > &&
> > > !m_runningDebuggerTask;
> > > On 2016/08/29 06:56:20, yhirano wrote:
> > > > IIUC m_threadState could be NotStarted when the initialization task has
> been
> > > > posted to the worker thread but it has not been executed yet. In that
case
> > > that
> > > > initialization task might block forever as it runs the given script, and
> > hence
> > > I
> > > > think shouldScheduleToTerminateExecution should be true.
> > > 
> > > Very good point. I made the latest patchset to address this case not here
> but
> > in
> > > initializeOnWorkerThread().
> > > 
> > >
> >
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/core/workers/WorkerThread.cpp (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/2280523002/diff/100001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/core/workers/WorkerThread.cpp:665: // main
thread
> > > before shutdown preparation.
> > > I'm now thinking that running a forcible termination task between here and
> > > prepareForShutdownOnWorkerThread() is not harmful, so unconditionally flip
> > over
> > > |m_runningDebuggerTask|. Can you double-check this is true?
> > > 
> > > This enables to unify prepareForShutdownOnWorkerThread() calls in
> > > initializeOnWorkerThread() and makes codepath simpler.
> > 
> > Hmm..., this change is causing test failures because it varies test results
> > depending on timing. I'll fix it and ask you review this again.
> 
> Oops, sorry! I forgot to address this issue... I'll ask you to review this
again
> after fixing it.

The following CL fixed the cause of these test failures.
[1] https://codereview.chromium.org/2355723005/

[yhirano, 2016-09-23 06:58:03]

lgtm

[nhiroki, 2016-09-23 07:32:52]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence just quit on the step 4) and defer to
the regular shutdown sequence. The regular shutdown sequence posts shutdown
tasks (ie., performShutdownOnWorkerThread) to the worker thread, and other tasks
also posted from the main thread(*) are guaranteed to be drained until the
shutdown tasks run.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

[nhiroki, 2016-09-23 07:35:35]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 639153
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810
==========

[nhiroki, 2016-09-23 07:40:33]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843
==========

[nhiroki, 2016-09-23 07:41:30]

The CQ bit was unchecked by nhiroki@chromium.org

[nhiroki, 2016-09-23 07:41:32]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2016-09-23 07:41:32]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2280523002/#ps220001
(title: "tweak")

[commit-bot: I haz the power, 2016-09-23 07:41:48]

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-09-23 11:29:47]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843
==========

[commit-bot: I haz the power, 2016-09-23 11:29:48]

Committed patchset #8 (id:220001)

[commit-bot: I haz the power, 2016-09-23 11:33:15]

Description was changed from

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843
==========

to

==========
Worker: Unify worker thread shutdown sequences.

This CL unifies worker thread shutdown sequences to fix a crash and simplify
shutdown.

<Problem>

When termination is requested on the main thread before WorkerThread is
initialized on the worker thread, shutdown sequence runs in a different way from
regular shutdown sequence: initialization sequence seamlessly switches to
shutdown sequence and asks the main thread to destroy WorkerThread
(see WorkerThread::initializeOnWorkerThread).

This causes a crash in a following scenario:

  1) Request to start the worker thread from the main thread.
  2) Post a task to the worker thread from the main thread.
  3) Request to terminate the worker thread from the main thread.
  4) Start initialziation sequence and switch to shutdown sequence on the worker
     thread.
  5) WorkerThread is destroyed on the main thread.
  6) The posted task runs on the worker thread and crashes.

<Solution>

This CL makes the initialization sequence complete regardless of termination
request and defer to the regular shutdown task posted from the main thread.
Other tasks also posted from the main thread(*) are guaranteed to be drained
until the shutdown task runs.

<Appendix>

Regarding (*), you might wonder if tasks posted from/to the worker thread cannot
be guaranteed to be drained until the shutdown tasks run. This case is covered
by other mechanism. Let's consider it in following 2 cases:

  1) An uninitialized worker thread does not post a task to itself, so there
     should be no tasks when termination happens before initialization.
  2) Otherwise, WorkerBackingThread::shutdown drains all tasks.

Therefore, we can ensure that tasks posted from the worker thread also never run
after shutdown.

BUG=632810, 640843

Committed: https://crrev.com/cfa01627d191d2a1a801943994f287b8cf5b1c42
Cr-Commit-Position: refs/heads/master@{#420592}
==========

[commit-bot: I haz the power, 2016-09-23 11:33:17]

Patchset 8 (id:??) landed as
https://crrev.com/cfa01627d191d2a1a801943994f287b8cf5b1c42
Cr-Commit-Position: refs/heads/master@{#420592}