Introducing the WorkerProcessLauncher class implementing the common logic for spawning a worker pro…

remoting/host/wts_session_process_launcher_win.cc

Index: remoting/host/wts_session_process_launcher_win.cc
diff --git a/remoting/host/wts_session_process_launcher_win.cc b/remoting/host/wts_session_process_launcher_win.cc
index 45d81cf7b2ca2a5826097dc0c7d93344cf48b3c0..85b13ff8f7d315712ea2b8066229a1b4749d04e1 100644
--- a/remoting/host/wts_session_process_launcher_win.cc
+++ b/remoting/host/wts_session_process_launcher_win.cc
@@ -9,8 +9,9 @@
 
 #include <windows.h>
 #include <sddl.h>
-#include <limits>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/rand_util.h"
@@ -25,6 +26,7 @@
 
 #include "remoting/host/chromoting_messages.h"
 #include "remoting/host/sas_injector.h"
+#include "remoting/host/worker_process_launcher.h"
 #include "remoting/host/wts_console_monitor_win.h"
 
 using base::win::ScopedHandle;
@@ -137,68 +139,6 @@ bool CreateSessionToken(uint32 session_id,
   return true;
 }
 
-// Generates random channel ID.
-// N.B. Stolen from src/content/common/child_process_host_impl.cc
-string16 GenerateRandomChannelId(void* instance) {
-  return base::StringPrintf(ASCIIToUTF16("%d.%p.%d").c_str(),
-                            base::GetCurrentProcId(), instance,
-                            base::RandInt(0, std::numeric_limits<int>::max()));
-}
-
-// Creates the server end of the Chromoting IPC channel.
-// N.B. This code is based on IPC::Channel's implementation.
-bool CreatePipeForIpcChannel(void* instance,
-                             string16* channel_name_out,
-                             ScopedHandle* pipe_out) {
-  // Create security descriptor for the channel.
-  SECURITY_ATTRIBUTES security_attributes;
-  security_attributes.nLength = sizeof(security_attributes);
-  security_attributes.bInheritHandle = FALSE;
-
-  ULONG security_descriptor_length = 0;
-  if (!ConvertStringSecurityDescriptorToSecurityDescriptorA(
-           kChromotingChannelSecurityDescriptor,
-           SDDL_REVISION_1,
-           reinterpret_cast<PSECURITY_DESCRIPTOR*>(
-               &security_attributes.lpSecurityDescriptor),
-           &security_descriptor_length)) {
-    LOG_GETLASTERROR(ERROR) <<
-        "Failed to create a security descriptor for the Chromoting IPC channel";
-    return false;
-  }
-
-  // Generate a random channel name.
-  string16 channel_name(GenerateRandomChannelId(instance));
-
-  // Convert it to the pipe name.
-  string16 pipe_name(ASCIIToUTF16(kChromePipeNamePrefix));
-  pipe_name.append(channel_name);
-
-  // Create the server end of the pipe. This code should match the code in
-  // IPC::Channel with exception of passing a non-default security descriptor.
-  HANDLE pipe = CreateNamedPipeW(pipe_name.c_str(),
-                                 PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
-                                     FILE_FLAG_FIRST_PIPE_INSTANCE,
-                                 PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
-                                 1,
-                                 IPC::Channel::kReadBufferSize,
-                                 IPC::Channel::kReadBufferSize,
-                                 5000,
-                                 &security_attributes);
-  if (pipe == INVALID_HANDLE_VALUE) {
-    LOG_GETLASTERROR(ERROR) <<
-        "Failed to create the server end of the Chromoting IPC channel";
-    LocalFree(security_attributes.lpSecurityDescriptor);
-    return false;
-  }
-
-  LocalFree(security_attributes.lpSecurityDescriptor);
-
-  *channel_name_out = channel_name;
-  pipe_out->Set(pipe);
-  return true;
-}
-
 // Launches |binary| in the security context of the supplied |user_token|.
 bool LaunchProcessAsUser(const FilePath& binary,
                          const string16& command_line,
@@ -242,89 +182,127 @@ namespace remoting {
 WtsSessionProcessLauncher::WtsSessionProcessLauncher(
     WtsConsoleMonitor* monitor,
     const FilePath& host_binary,
-    base::Thread* io_thread)
+    base::MessageLoopProxy* ipc_message_loop)
     : host_binary_(host_binary),
-      io_thread_(io_thread),
-      monitor_(monitor),
-      state_(StateDetached) {
+      ipc_message_loop_(ipc_message_loop),
+      monitor_(monitor) {
   monitor_->AddWtsConsoleObserver(this);
 }
 
 WtsSessionProcessLauncher::~WtsSessionProcessLauncher() {
-  DCHECK(state_ == StateDetached);
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
 
   monitor_->RemoveWtsConsoleObserver(this);
 }
 
-void WtsSessionProcessLauncher::LaunchProcess() {
-  DCHECK(state_ == StateStarting);
-  DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
+// Creates the server end of the Chromoting IPC channel.
+// N.B. This code is based on IPC::Channel's implementation.
+bool WtsSessionProcessLauncher::CreateChannelHandle(
+    const std::string& channel_name,
+    IPC::ChannelHandle* channel_handle_out) {
 
-  launch_time_ = base::Time::Now();
+  // Create security descriptor for the channel.
+  SECURITY_ATTRIBUTES security_attributes;
+  security_attributes.nLength = sizeof(security_attributes);
+  security_attributes.bInheritHandle = FALSE;
 
-  string16 channel_name;
-  ScopedHandle pipe;
-  if (CreatePipeForIpcChannel(this, &channel_name, &pipe)) {
-    // Wrap the pipe into an IPC channel.
-    chromoting_channel_.reset(new IPC::ChannelProxy(
-        IPC::ChannelHandle(pipe.Get()),
-        IPC::Channel::MODE_SERVER,
-        this,
-        io_thread_->message_loop_proxy().get()));
-
-    string16 command_line =
-        base::StringPrintf(ASCIIToUTF16(kHostProcessCommandLineFormat).c_str(),
-                           host_binary_.value().c_str(),
-                           channel_name.c_str());
-
-    // Try to launch the process and attach an object watcher to the returned
-    // handle so that we get notified when the process terminates.
-    if (LaunchProcessAsUser(host_binary_, command_line, session_token_,
-                            &process_)) {
-      if (process_watcher_.StartWatching(process_.handle(), this)) {
-        state_ = StateAttached;
-        return;
-      } else {
-        LOG(ERROR) << "Failed to arm the process watcher.";
-        process_.Terminate(0);
-        process_.Close();
-      }
-    }
+  ULONG security_descriptor_length = 0;
+  if (!ConvertStringSecurityDescriptorToSecurityDescriptorA(
+           kChromotingChannelSecurityDescriptor,
+           SDDL_REVISION_1,
+           reinterpret_cast<PSECURITY_DESCRIPTOR*>(
+               &security_attributes.lpSecurityDescriptor),
+           &security_descriptor_length)) {
+    LOG_GETLASTERROR(ERROR) <<
+        "Failed to create a security descriptor for the Chromoting IPC channel";
+    return false;
+  }
+
+  // Convert it to the pipe name.
+  string16 pipe_name(ASCIIToUTF16(kChromePipeNamePrefix));
+  pipe_name.append(ASCIIToUTF16(channel_name));
 
-    chromoting_channel_.reset();
+  // Create the server end of the pipe. This code should match the code in
+  // IPC::Channel with exception of passing a non-default security descriptor.
+  ipc_pipe_.Set(CreateNamedPipeW(pipe_name.c_str(),
+                                 PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
+                                     FILE_FLAG_FIRST_PIPE_INSTANCE,
+                                 PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
+                                 1,
+                                 IPC::Channel::kReadBufferSize,
+                                 IPC::Channel::kReadBufferSize,
+                                 5000,
+                                 &security_attributes));
+  if (!ipc_pipe_.IsValid()) {
+    LOG_GETLASTERROR(ERROR) <<
+        "Failed to create the server end of the Chromoting IPC channel";
+    LocalFree(security_attributes.lpSecurityDescriptor);
+    return false;
   }
 
-  // Something went wrong. Try to launch the host again later. The attempts rate
-  // is limited by exponential backoff.
-  launch_backoff_ = std::max(launch_backoff_ * 2,
-                             TimeDelta::FromSeconds(kMinLaunchDelaySeconds));
-  launch_backoff_ = std::min(launch_backoff_,
-                             TimeDelta::FromSeconds(kMaxLaunchDelaySeconds));
-  timer_.Start(FROM_HERE, launch_backoff_,
-               this, &WtsSessionProcessLauncher::LaunchProcess);
+  LocalFree(security_attributes.lpSecurityDescriptor);
+
+  *channel_handle_out = IPC::ChannelHandle(ipc_pipe_.Get());
+  return true;
+
 }
 
-void WtsSessionProcessLauncher::OnObjectSignaled(HANDLE object) {
-  DCHECK(state_ == StateAttached);
+bool WtsSessionProcessLauncher::LaunchWorker(const std::string& channel_name,
+                                             base::Process* process_out) {
+
+  string16 command_line =
+      base::StringPrintf(ASCIIToUTF16(kHostProcessCommandLineFormat).c_str(),
+                         host_binary_.value().c_str(),
+                         ASCIIToUTF16(channel_name).c_str());
+
+  return LaunchProcessAsUser(host_binary_, command_line, session_token_,
+                             process_out);
+}
+
+void WtsSessionProcessLauncher::LaunchProcess() {
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() != NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() != NULL);
 
-  // The host process has been terminated for some reason. The handle can now be
-  // closed.
-  process_.Close();
-  chromoting_channel_.reset();
+  launch_time_ = base::Time::Now();
+  launcher_ = WorkerProcessLauncher::Create(ipc_message_loop_.get());
+  bool success =
+      launcher_->Start(
+          this,
+          base::Bind(&WtsSessionProcessLauncher::CreateChannelHandle,
+                     base::Unretained(this)),
+          base::Bind(&WtsSessionProcessLauncher::LaunchWorker,
+                     base::Unretained(this)));
+
+  // The pipe handle has been duplicated by the IPC::Channel. We can close our
+  // copy now.
+  ipc_pipe_.Close();
+
+  if (!success) {
+    // Something went wrong. Try to launch the host again later. The attempts
+    // rate is limited by exponential backoff.
+    launch_backoff_ = std::max(launch_backoff_ * 2,
+                               TimeDelta::FromSeconds(kMinLaunchDelaySeconds));
+    launch_backoff_ = std::min(launch_backoff_,
+                              TimeDelta::FromSeconds(kMaxLaunchDelaySeconds));
+    timer_.Start(FROM_HERE, launch_backoff_,
+                 this, &WtsSessionProcessLauncher::LaunchProcess);
+  }
+}
+
+bool WtsSessionProcessLauncher::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(WtsSessionProcessLauncher, message)
+      IPC_MESSAGE_HANDLER(ChromotingHostMsg_SendSasToConsole,
+                          OnSendSasToConsole)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void WtsSessionProcessLauncher::OnChannelError() {
+  DCHECK(!timer_.IsRunning());
 
   // Expand the backoff interval if the process has died quickly or reset it if
-  // it was up longer than the maximum backoff delay.
+  // the process was up longer than the maximum backoff delay.
   base::TimeDelta delta = base::Time::Now() - launch_time_;
   if (delta < base::TimeDelta() ||
       delta >= base::TimeDelta::FromSeconds(kMaxLaunchDelaySeconds)) {
@@ -337,39 +315,22 @@ void WtsSessionProcessLauncher::OnObjectSignaled(HANDLE object) {
   }
 
   // Try to restart the host.
-  state_ = StateStarting;
   timer_.Start(FROM_HERE, launch_backoff_,
                this, &WtsSessionProcessLauncher::LaunchProcess);
 }
 
-bool WtsSessionProcessLauncher::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(WtsSessionProcessLauncher, message)
-      IPC_MESSAGE_HANDLER(ChromotingHostMsg_SendSasToConsole,
-                          OnSendSasToConsole)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
 void WtsSessionProcessLauncher::OnSendSasToConsole() {
-  if (state_ == StateAttached) {
-    if (sas_injector_.get() == NULL) {
-      sas_injector_ = SasInjector::Create();
-    }
+  if (sas_injector_.get() == NULL) {
+    sas_injector_ = SasInjector::Create();
+  }
 
-    if (sas_injector_.get() != NULL) {
-      sas_injector_->InjectSas();
-    }
+  if (sas_injector_.get() != NULL) {
+    sas_injector_->InjectSas();
   }
 }
 
 void WtsSessionProcessLauncher::OnSessionAttached(uint32 session_id) {
-  DCHECK(state_ == StateDetached);
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
 
   // Temporarily enable the SE_TCB_NAME privilege. The privileged token is
   // created as needed and kept for later reuse.
@@ -393,51 +354,16 @@ void WtsSessionProcessLauncher::OnSessionAttached(uint32 session_id) {
   // CreateProcessAsUser() successfully.
   CHECK(RevertToSelf());
 
-  if (!result)
-    return;
-
   // Now try to launch the host.
-  state_ = StateStarting;
-  LaunchProcess();
+  if (result) {
+    LaunchProcess();
+  }
 }
 
 void WtsSessionProcessLauncher::OnSessionDetached() {
-  DCHECK(state_ == StateDetached ||
-         state_ == StateStarting ||
-         state_ == StateAttached);
-
-  switch (state_) {
-    case StateDetached:
-      DCHECK(!timer_.IsRunning());
-      DCHECK(process_.handle() == NULL);
-      DCHECK(process_watcher_.GetWatchedObject() == NULL);
-      DCHECK(chromoting_channel_.get() == NULL);
-      break;
-
-    case StateStarting:
-      DCHECK(timer_.IsRunning());
-      DCHECK(process_.handle() == NULL);
-      DCHECK(process_watcher_.GetWatchedObject() == NULL);
-      DCHECK(chromoting_channel_.get() == NULL);
-
-      timer_.Stop();
-      launch_backoff_ = base::TimeDelta();
-      state_ = StateDetached;
-      break;
-
-    case StateAttached:
-      DCHECK(!timer_.IsRunning());
-      DCHECK(process_.handle() != NULL);
-      DCHECK(process_watcher_.GetWatchedObject() != NULL);
-      DCHECK(chromoting_channel_.get() != NULL);
-
-      process_watcher_.StopWatching();
-      process_.Terminate(0);
-      process_.Close();
-      chromoting_channel_.reset();
-      state_ = StateDetached;
-      break;
-  }
+  launch_backoff_ = base::TimeDelta();
+  timer_.Stop();
+  launcher_.reset();
 }
 
 } // namespace remoting