[Chromoting] Moving common logic responsible for launching child processes to WorkerProcessLauncher…

remoting/base/stoppable.cc

Index: remoting/base/stoppable.cc
diff --git a/remoting/base/stoppable.cc b/remoting/base/stoppable.cc
index 88b3c95ef953cdf9fbe840ab4db168954d745a40..db667dc3dc8c99addc2c51d81f0ae4cf6074ece3 100644
--- a/remoting/base/stoppable.cc
+++ b/remoting/base/stoppable.cc
@@ -24,9 +24,12 @@ Stoppable::~Stoppable() {
 void Stoppable::Stop() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
+  // DoStop() can be called multiple times. stoppable_state() can be queried
+  // to tell the first call from the follow up calls.
+  DoStop();

[Wez, 2012/08/08 20:13:41]

Isn't there a risk that something in DoStop() triggers code that naively checks
stoppable_state() against kRunning or kStopping, and so won't behave as expected
if triggered from the first DoStop()? Surely the Delegate can set a flag on the
first DoStop() if it cares about distinguishing duplicate calls?

[alexeypa (please no reviews), 2012/08/08 21:49:58]

This is the intended behavior. :-)
It is not different from (and even worse) that relying on stoppable_state().
stoppable_state() is intended to be used by the Delegate() which is aware of
Stop()/DoStop() semantic. Having it's own flag will either duplicate
stoppable_state() or duplicate it badly because of missing DCHECKs.

[alexeypa (please no reviews), 2012/08/10 16:18:44]

It turned out that this breaks stoppable state validation, so now it is up to
DoStop implementation to distinction the first and all other invocations of
DoStop().

+
   if (state_ == kRunning) {
     state_ = kStopping;
-    DoStop();
   }
 }