[Chromoting] Reimplemented the worker process launcher to take into account the encountered issues:

remoting/host/win/worker_process_launcher.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef REMOTING_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_
 #define REMOTING_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_
 
-#include <windows.h>
-
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/process.h"
+#include "base/time.h"
 #include "base/timer.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/object_watcher.h"
 #include "ipc/ipc_channel.h"
-#include "remoting/base/stoppable.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 } // namespace base
 
 namespace IPC {
 class ChannelProxy;
 class Message;
 } // namespace IPC
 
 namespace remoting {
 
 class WorkerProcessIpcDelegate;
 
 // Launches a worker process that is controlled via an IPC channel. All
 // interaction with the spawned process is through the IPC::Listener and Send()
 // method. In case of error the channel is closed and the worker process is
 // terminated.
-//
-// WorkerProcessLauncher object is good for one process launch attempt only.
 class WorkerProcessLauncher
-    : public Stoppable,
+    : public base::RefCountedThreadSafe<WorkerProcessLauncher>,

[Wez, 2012/10/09 03:40:39]

Ouch.  Could this be a caller-owned class with a ref-counted core instead?

[alexeypa (please no reviews), 2012/10/09 19:42:04]

Done.

       public base::win::ObjectWatcher::Delegate,
       public IPC::Listener {
  public:
   class Delegate {
    public:
     virtual ~Delegate();
 
+    // Returns the exit code of the worker process.
+    virtual DWORD GetExitCode() = 0;
+
+    // Terminates the worker process with the given exit code.
+    virtual void KillProcess(DWORD exit_code) = 0;
+
     // Starts the worker process and passes |channel_name| to it.
     // |process_exit_event_out| receives a handle that becomes signalled once
     // the launched process has been terminated.
-    virtual bool DoLaunchProcess(
+    virtual bool LaunchProcess(
         const std::string& channel_name,
         base::win::ScopedHandle* process_exit_event_out) = 0;
-
-    // Terminates the worker process with the given exit code.
-    virtual void DoKillProcess(DWORD exit_code) = 0;
   };
 
   // Creates the launcher that will use |launcher_delegate| to manage the worker
-  // process and |worker_delegate| to handle IPCs.
+  // process and |worker_delegate| to handle IPCs. The caller must ensure that
+  // |worker_delegate| remains valid until Stoppable::Stop() method has been
+  // called.
   //
-  // |stopped_callback| and |main_task_runner| are passed to the underlying
-  // |Stoppable| implementation. The caller should call all the methods on this
-  // class on the |main_task_runner| thread. |ipc_task_runner| is used to
-  // perform background IPC I/O.
+  // The caller should call all the methods on this class on
+  // the |main_task_runner| thread. Methods of both delegate interfaces are
+  // called on the |main_task_runner| thread as well. |io_task_runner| is used
+  // to perform background IPC I/O.

[Wez, 2012/10/09 03:40:39]

Could you grab |main_thread_task_runner| implicitly via ThreadTaskRunnerHandle?

If not, consider renaming |main_task_runner| to |caller_task_runner|.

[alexeypa (please no reviews), 2012/10/09 19:42:04]

I don't think so. ThreadTaskRunnerHandle::Get() will not get me
AutoThreadTaskRunner while I need one to make sure shutdown code will run.
Done.

   WorkerProcessLauncher(
-      Delegate* launcher_delegate,
+      scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      scoped_ptr<Delegate> launcher_delegate,
       WorkerProcessIpcDelegate* worker_delegate,
-      const base::Closure& stopped_callback,
-      scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-      scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner);
-  virtual ~WorkerProcessLauncher();
+      const std::string& pipe_security_descriptor);
 
-  // Starts the worker process.
-  void Start(const std::string& pipe_sddl);
+  // Launches the worker process.
+  void Start();
+
+  // Stops the worker process asynchronously. The caller can drop the reference
+  // to |this| as soon as Stop() returns.
+  void Stop();

[Wez, 2012/10/09 03:40:39]

nit: The callers seem to Stop() and immediately drop the reference; if you
restructure this as a caller-owned class with a ref-counted core then you can
let the caller simply delete it to stop it.

[alexeypa (please no reviews), 2012/10/09 19:42:04]

Done.

 
   // Sends an IPC message to the worker process. The message will be silently
   // dropped if Send() is called before Start() or after stutdown has been
   // initiated.
   void Send(IPC::Message* message);
 
   // base::win::ObjectWatcher::Delegate implementation.
   virtual void OnObjectSignaled(HANDLE object) OVERRIDE;
 
   // IPC::Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
   virtual void OnChannelConnected(int32 peer_pid) OVERRIDE;
   virtual void OnChannelError() OVERRIDE;
 
- protected:
-  // Stoppable implementation.
-  virtual void DoStop() OVERRIDE;
+ private:
+  friend class base::RefCountedThreadSafe<WorkerProcessLauncher>;
+  virtual ~WorkerProcessLauncher();
 
- private:
   // Creates the server end of the Chromoting IPC channel.
   bool CreatePipeForIpcChannel(const std::string& channel_name,
-                               const std::string& pipe_sddl,
+                               const std::string& pipe_security_descriptor,
                                base::win::ScopedHandle* pipe_out);
 
   // Generates random channel ID.
   std::string GenerateRandomChannelId();
 
-  Delegate* launcher_delegate_;
+  // Attempts to launch the worker process. Schedules next launch attempt if
+  // creation of the process fails.
+  void LaunchWorker();
+
+  // Stops the worker process asynchronously and schedules next launch attempt
+  // unless Stop() has been called already.
+  void StopWorker();
+
+  // All public methods are called on the |main_task_runner| thread.
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+
+  // The task runner is used perform background IPC I/O.
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+
+  // Implements specifics of launching a worker process.
+  scoped_ptr<Delegate> launcher_delegate_;
+
+  // Handles IPC messages sent by the worker process.
   WorkerProcessIpcDelegate* worker_delegate_;
 
-  // The main service message loop.
-  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  // The IPC channel connecting to the launched process.
+  scoped_ptr<IPC::ChannelProxy> ipc_channel_;
 
-  // Message loop used by the IPC channel.
-  scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner_;
+  // The timer used to delay termination of the process in the case of an IPC
+  // error.
+  scoped_ptr<base::OneShotTimer<WorkerProcessLauncher> > ipc_error_timer_;
+
+  // Time of the last launch attempt.
+  base::Time launch_time_;
+
+  // Current backoff delay.
+  base::TimeDelta launch_backoff_;
+
+  // Timer used to schedule the next attempt to launch the process.
+  scoped_ptr<base::OneShotTimer<WorkerProcessLauncher> > launch_timer_;
 
   // Used to determine when the launched process terminates.
   base::win::ObjectWatcher process_watcher_;
 
   // A waiting handle that becomes signalled once the launched process has
   // been terminated.
   base::win::ScopedHandle process_exit_event_;
 
-  // The IPC channel connecting to the launched process.
-  scoped_ptr<IPC::ChannelProxy> ipc_channel_;
-
-  // The timer used to delay termination of the process in the case of an IPC
-  // error.
-  base::OneShotTimer<WorkerProcessLauncher> ipc_error_timer_;
-
   // The server end of the pipe.
   base::win::ScopedHandle pipe_;
 
+  // The security descriptor (as SDDL) of the server end of the pipe.
+  std::string pipe_security_descriptor_;
+
+  // Self reference to keep the object alive while the worker process is being
+  // terminated.
+  scoped_refptr<WorkerProcessLauncher> self_;
+
+  // True when Stop() has been called.
+  bool stopping_;
+
   DISALLOW_COPY_AND_ASSIGN(WorkerProcessLauncher);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_