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

remoting/host/win/wts_session_process_launcher.cc

Index: remoting/host/win/wts_session_process_launcher.cc
diff --git a/remoting/host/win/wts_session_process_launcher.cc b/remoting/host/win/wts_session_process_launcher.cc
index 6548fd4fbe2149a6275606ee306e3abb0bcc65d3..c3bd9e5aa8e1038420de55727edca96e15a5aea2 100644
--- a/remoting/host/win/wts_session_process_launcher.cc
+++ b/remoting/host/win/wts_session_process_launcher.cc
@@ -23,6 +23,7 @@
 #include "base/process_util.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_message.h"
@@ -46,9 +47,6 @@ const int kMinLaunchDelaySeconds = 1;
 const FilePath::CharType kMe2meHostBinaryName[] =
     FILE_PATH_LITERAL("remoting_me2me_host.exe");
 
-// Match the pipe name prefix used by Chrome IPC channels.
-const wchar_t kChromePipeNamePrefix[] = L"\\\\.\\pipe\\chrome.";
-
 // The IPC channel name is passed to the host in the command line.
 const char kChromotingIpcSwitchName[] = "chromoting-ipc";
 
@@ -59,70 +57,7 @@ const char* kCopiedSwitchNames[] = {
 
 // The security descriptor of the Chromoting IPC channel. It gives full access
 // to LocalSystem and denies access by anyone else.
-const wchar_t kChromotingChannelSecurityDescriptor[] =
-    L"O:SYG:SYD:(A;;GA;;;SY)";
-
-// Generates random channel ID.
-// N.B. Stolen from src/content/common/child_process_host_impl.cc
-std::wstring GenerateRandomChannelId(void* instance) {
-  return base::StringPrintf(L"%d.%p.%d",
-                            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,
-                             std::wstring* 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 (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
-           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.
-  std::wstring channel_name(GenerateRandomChannelId(instance));
-
-  // Convert it to the pipe name.
-  std::wstring pipe_name(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;
-}
+const char kChromotingChannelSecurityDescriptor[] = "O:SYG:SYD:(A;;GA;;;SY)";
 
 } // namespace
 
@@ -134,151 +69,155 @@ WtsSessionProcessLauncher::WtsSessionProcessLauncher(
     scoped_refptr<base::SingleThreadTaskRunner> main_message_loop,
     scoped_refptr<base::SingleThreadTaskRunner> ipc_message_loop)
     : Stoppable(main_message_loop, stopped_callback),
+      attached_(false),
       main_message_loop_(main_message_loop),
       ipc_message_loop_(ipc_message_loop),
-      monitor_(monitor),
-      state_(StateDetached) {
+      monitor_(monitor) {
   monitor_->AddWtsConsoleObserver(this);
 }
 
 WtsSessionProcessLauncher::~WtsSessionProcessLauncher() {
   monitor_->RemoveWtsConsoleObserver(this);
-  if (state_ != StateDetached) {
-    OnSessionDetached();
-  }
 
-  DCHECK(state_ == StateDetached);
+  DCHECK(!attached_);
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
 }
 
 void WtsSessionProcessLauncher::LaunchProcess() {
   DCHECK(main_message_loop_->BelongsToCurrentThread());
-  DCHECK(state_ == StateStarting);
+  DCHECK(attached_);
+  DCHECK(launcher_.get() == NULL);
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
+  DCHECK(!worker_process_.IsValid());
 
   launch_time_ = base::Time::Now();
+  launcher_.reset(new WorkerProcessLauncher(
+      base::Bind(&WtsSessionProcessLauncher::OnLauncherStopped,
+                 base::Unretained(this)),
+      main_message_loop_,
+      ipc_message_loop_));
+  launcher_->Start(this, kChromotingChannelSecurityDescriptor);
+}
+
+void WtsSessionProcessLauncher::OnLauncherStopped() {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
+
+  DWORD exit_code;
+  if (!::GetExitCodeProcess(worker_process_, &exit_code)) {

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

nit: Info-log GetLastError(), in case we hit some situation where
GetExitCodeProcess() is failing and we can't work out why?

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

Done.

+    exit_code = CONTROL_C_EXIT;
+  }
+
+  launcher_.reset(NULL);
+  worker_process_.Close();
+
+  // Complete stoppping is shutdown is in progress.

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

typo: stopping
typo: is

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

nit: We use "Finish" rather than "Complete" elsewhere in the remoting code.

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

nit: Reword the comment to clarify _why_ we call CompleteStopping() if we're not
kRunning.  e.g. "If the caller has asked us to stop then finish stopping rather
than trying to relaunch a new worker."

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

Done.

+  if (stoppable_state() != Stoppable::kRunning) {
+    CompleteStopping();
+    return;
+  }
+
+  // Stop trying to restart the host if its process exited due to
+  // misconfiguration.
+  if (kMinPermanentErrorExitCode <= exit_code &&
+      exit_code <= kMaxPermanentErrorExitCode) {
+    Stop();
+    return;
+  }
+
+  // Try to restart the process if we are still attached to the session.

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

nit: the session -> a session

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

Done.

+  if (attached_) {
+    // Expand the backoff interval if the process has died quickly or reset it
+    // if it 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)) {
+      launch_backoff_ = base::TimeDelta();
+    } else {
+      launch_backoff_ = std::max(
+          launch_backoff_ * 2, TimeDelta::FromSeconds(kMinLaunchDelaySeconds));
+      launch_backoff_ = std::min(
+          launch_backoff_, TimeDelta::FromSeconds(kMaxLaunchDelaySeconds));
+    }
+
+    // Try to restart the host.

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

nit: "Try to launch the session process."

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

Done.

+    timer_.Start(FROM_HERE, launch_backoff_,
+                 this, &WtsSessionProcessLauncher::LaunchProcess);
+  }
+}
+
+bool WtsSessionProcessLauncher::DoLaunchProcess(const std::string& channel_name,
+                                                ScopedHandle* wait_out) {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
+  DCHECK(!worker_process_.IsValid());
 
   // Construct the host binary name.
   FilePath dir_path;
   if (!PathService::Get(base::DIR_EXE, &dir_path)) {
     LOG(ERROR) << "Failed to get the executable file name.";
-    Stop();
-    return;
+    return false;
   }
   FilePath host_binary = dir_path.Append(kMe2meHostBinaryName);
 
-  std::wstring 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,
-        ipc_message_loop_));
-
-    // Create the host process command line passing the name of the IPC channel
-    // to use and copying known switches from the service's command line.
-    CommandLine command_line(host_binary);
-    command_line.AppendSwitchNative(kChromotingIpcSwitchName, channel_name);
-    command_line.CopySwitchesFrom(*CommandLine::ForCurrentProcess(),
-                                  kCopiedSwitchNames,
-                                  _countof(kCopiedSwitchNames));
-
-    // Try to launch the process and attach an object watcher to the returned
-    // handle so that we get notified when the process terminates.
-    if (LaunchProcessWithToken(host_binary,
-                               command_line.GetCommandLineString(),
-                               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();
-      }
-    }
+  // Create the host process command line passing the name of the IPC channel
+  // to use and copying known switches from the service's command line.
+  CommandLine command_line(host_binary);
+  command_line.AppendSwitchNative(kChromotingIpcSwitchName,
+                                  UTF8ToWide(channel_name));
+  command_line.CopySwitchesFrom(*CommandLine::ForCurrentProcess(),
+                                kCopiedSwitchNames,
+                                _countof(kCopiedSwitchNames));
+
+  // Try to launch the process and attach an object watcher to the returned
+  // handle so that we get notified when the process terminates.
+  if (!LaunchProcessWithToken(host_binary,
+                              command_line.GetCommandLineString(),
+                              session_token_,
+                              &worker_process_)) {
+    return false;
+  }
 
-    chromoting_channel_.reset();
+  ScopedHandle wait;
+  if (!DuplicateHandle(GetCurrentProcess(),
+                       worker_process_,
+                       GetCurrentProcess(),
+                       wait.Receive(),
+                       SYNCHRONIZE,
+                       FALSE,
+                       0)) {
+    LOG_GETLASTERROR(ERROR) << "Failed to duplicate a handle";
+    DoKillProcess(CONTROL_C_EXIT);
+    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);
+  *wait_out = wait.Pass();
+  return true;
 }
 
-void WtsSessionProcessLauncher::OnObjectSignaled(HANDLE object) {
-  if (!main_message_loop_->BelongsToCurrentThread()) {
-    main_message_loop_->PostTask(
-        FROM_HERE, base::Bind(&WtsSessionProcessLauncher::OnObjectSignaled,
-                              base::Unretained(this), object));
-    return;
-  }
+void WtsSessionProcessLauncher::DoKillProcess(DWORD exit_code) {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
 
-  // It is possible that OnObjectSignaled() task will be queued by another
-  // thread right before |process_watcher_| was stopped. It such a case it is
-  // safe to ignore this notification.
-  if (state_ != StateAttached) {
-    return;
+  if (worker_process_.IsValid()) {
+    TerminateProcess(worker_process_, exit_code);
   }
+}
 
-  DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() != NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() != NULL);
+void WtsSessionProcessLauncher::OnChannelConnected(
+    base::win::ScopedHandle peer) {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
 
-  // Stop trying to restart the host if its process exited due to
-  // misconfiguration.
-  int exit_code;
-  bool stop_trying =
-      base::WaitForExitCodeWithTimeout(
-          process_.handle(), &exit_code, base::TimeDelta()) &&
-      kMinPermanentErrorExitCode <= exit_code &&
-      exit_code <= kMaxPermanentErrorExitCode;
-
-  // The host process has been terminated for some reason. The handle can now be
-  // closed.
-  process_.Close();
-  chromoting_channel_.reset();
-  state_ = StateStarting;
-
-  if (stop_trying) {
+  DWORD client_process_id = GetProcessId(peer);
+  DWORD expected_process_id = GetProcessId(worker_process_);
+  if (client_process_id != expected_process_id) {
+    LOG(ERROR)
+        << "Unexpected client connected: expected=" << expected_process_id
+        << ", actual=" << client_process_id;
     Stop();
-    return;
   }
-
-  // Expand the backoff interval if the process has died quickly or reset it if
-  // it 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)) {
-    launch_backoff_ = base::TimeDelta();
-  } else {
-    launch_backoff_ = std::max(launch_backoff_ * 2,
-                               TimeDelta::FromSeconds(kMinLaunchDelaySeconds));
-    launch_backoff_ = std::min(launch_backoff_,
-                               TimeDelta::FromSeconds(kMaxLaunchDelaySeconds));
-  }
-
-  // Try to restart the host.
-  timer_.Start(FROM_HERE, launch_backoff_,
-               this, &WtsSessionProcessLauncher::LaunchProcess);
 }
 
 bool WtsSessionProcessLauncher::OnMessageReceived(const IPC::Message& message) {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
+
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(WtsSessionProcessLauncher, message)
       IPC_MESSAGE_HANDLER(ChromotingHostMsg_SendSasToConsole,
@@ -289,14 +228,9 @@ bool WtsSessionProcessLauncher::OnMessageReceived(const IPC::Message& message) {
 }
 
 void WtsSessionProcessLauncher::OnSendSasToConsole() {
-  if (!main_message_loop_->BelongsToCurrentThread()) {
-    main_message_loop_->PostTask(
-        FROM_HERE, base::Bind(&WtsSessionProcessLauncher::OnSendSasToConsole,
-                              base::Unretained(this)));
-    return;
-  }
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
 
-  if (state_ == StateAttached) {
+  if (attached_) {
     if (sas_injector_.get() == NULL) {
       sas_injector_ = SasInjector::Create();
     }
@@ -314,68 +248,43 @@ void WtsSessionProcessLauncher::OnSessionAttached(uint32 session_id) {
     return;
   }
 
-  DCHECK(state_ == StateDetached);
+  DCHECK(!attached_);
   DCHECK(!timer_.IsRunning());
-  DCHECK(process_.handle() == NULL);
-  DCHECK(process_watcher_.GetWatchedObject() == NULL);
-  DCHECK(chromoting_channel_.get() == NULL);
+
+  attached_ = true;
 
   // Create a session token for the launched process.
   if (!CreateSessionToken(session_id, &session_token_))
     return;
 
   // Now try to launch the host.
-  state_ = StateStarting;
   LaunchProcess();
 }
 
 void WtsSessionProcessLauncher::OnSessionDetached() {
   DCHECK(main_message_loop_->BelongsToCurrentThread());
-  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(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;
-  }
+  DCHECK(attached_);
 
+  attached_ = false;
+  launch_backoff_ = base::TimeDelta();
   session_token_.Close();
+  timer_.Stop();
+
+  if (launcher_.get() != NULL) {
+    launcher_->Stop();
+  }
 }
 
 void WtsSessionProcessLauncher::DoStop() {
-  if (state_ != StateDetached) {
+  DCHECK(main_message_loop_->BelongsToCurrentThread());
+
+  if (attached_) {
     OnSessionDetached();
   }
 
-  CompleteStopping();
+  if (launcher_.get() == NULL) {
+    CompleteStopping();
+  }
 }
 
 } // namespace remoting