Pass the client end handle of the network-to-daemon IPC channel via handle inheritance.

remoting/host/win/worker_process_launcher.cc

Index: remoting/host/win/worker_process_launcher.cc
diff --git a/remoting/host/win/worker_process_launcher.cc b/remoting/host/win/worker_process_launcher.cc
index b06b4a39813fb99382aa80d0fe005389ae003a4e..54da0c1e745b04016e53495b14175eda3be51c7b 100644
--- a/remoting/host/win/worker_process_launcher.cc
+++ b/remoting/host/win/worker_process_launcher.cc
@@ -4,39 +4,19 @@
 
 #include "remoting/host/win/worker_process_launcher.h"
 
-#include <sddl.h>
-#include <limits>
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
-#include "base/process_util.h"
-#include "base/rand_util.h"
-#include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/timer.h"
-#include "base/utf_string_conversions.h"
 #include "base/win/object_watcher.h"
-#include "base/win/scoped_handle.h"
-#include "ipc/ipc_channel.h"
-#include "ipc/ipc_channel_proxy.h"
+#include "ipc/ipc_listener.h"
 #include "ipc/ipc_message.h"
 #include "net/base/backoff_entry.h"
 #include "remoting/host/host_exit_codes.h"
 #include "remoting/host/worker_process_ipc_delegate.h"
 
-using base::win::ScopedHandle;
 using base::TimeDelta;
-
-// Match the pipe name prefix used by Chrome IPC channels so that the client
-// could use Chrome IPC APIs instead of connecting manually.
-const char kChromePipeNamePrefix[] = "\\\\.\\pipe\\chrome.";
-
-// The minimum and maximum delays between attempts to inject host process into
-// a session.
-const int kMaxLaunchDelaySeconds = 60;
-const int kMinLaunchDelaySeconds = 1;
+using base::win::ScopedHandle;
 
 const net::BackoffEntry::Policy kDefaultBackoffPolicy = {
   // Number of initial errors (in sequence) to ignore before applying
@@ -83,13 +63,10 @@ class WorkerProcessLauncher::Core
   //
   // The caller should call all the methods on this class on
   // the |caller_task_runner| thread. Methods of both delegate interfaces are
-  // called on the |caller_task_runner| thread as well. |io_task_runner| is used
-  // to perform background IPC I/O.
+  // called on the |caller_task_runner| thread as well.
   Core(scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
-       scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
        scoped_ptr<WorkerProcessLauncher::Delegate> launcher_delegate,
-       WorkerProcessIpcDelegate* worker_delegate,
-       const std::string& pipe_security_descriptor);
+       WorkerProcessIpcDelegate* worker_delegate);
 
   // Launches the worker process.
   void Start();
@@ -115,14 +92,6 @@ class WorkerProcessLauncher::Core
   friend class base::RefCountedThreadSafe<Core>;
   virtual ~Core();
 
-  // Creates the server end of the Chromoting IPC channel.
-  bool CreatePipeForIpcChannel(const std::string& channel_name,
-                               const std::string& pipe_security_descriptor,
-                               base::win::ScopedHandle* pipe_out);
-
-  // Generates random channel ID.
-  std::string GenerateRandomChannelId();
-
   // Attempts to launch the worker process. Schedules next launch attempt if
   // creation of the process fails.
   void LaunchWorker();
@@ -137,17 +106,14 @@ class WorkerProcessLauncher::Core
   // All public methods are called on the |caller_task_runner| thread.
   scoped_refptr<base::SingleThreadTaskRunner> caller_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<WorkerProcessLauncher::Delegate> launcher_delegate_;
 
   // Handles IPC messages sent by the worker process.
   WorkerProcessIpcDelegate* worker_delegate_;
 
-  // The IPC channel connecting to the launched process.
-  scoped_ptr<IPC::ChannelProxy> ipc_channel_;
+  // True if IPC messages should be passed to |worker_delegate_|.
+  bool ipc_enabled_;
 
   // The timer used to delay termination of the process in the case of an IPC
   // error.
@@ -167,13 +133,7 @@ class WorkerProcessLauncher::Core
 
   // A waiting handle that becomes signalled once the launched process has
   // been terminated.
-  base::win::ScopedHandle process_exit_event_;
-
-  // 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_;
+  ScopedHandle process_exit_event_;
 
   // Self reference to keep the object alive while the worker process is being
   // terminated.
@@ -190,16 +150,13 @@ WorkerProcessLauncher::Delegate::~Delegate() {
 
 WorkerProcessLauncher::Core::Core(
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
     scoped_ptr<WorkerProcessLauncher::Delegate> launcher_delegate,
-    WorkerProcessIpcDelegate* worker_delegate,
-    const std::string& pipe_security_descriptor)
+    WorkerProcessIpcDelegate* worker_delegate)
     : caller_task_runner_(caller_task_runner),
-      io_task_runner_(io_task_runner),
       launcher_delegate_(launcher_delegate.Pass()),
       worker_delegate_(worker_delegate),
+      ipc_enabled_(false),
       launch_backoff_(&kDefaultBackoffPolicy),
-      pipe_security_descriptor_(pipe_security_descriptor),
       stopping_(false) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
@@ -229,11 +186,7 @@ void WorkerProcessLauncher::Core::Stop() {
 void WorkerProcessLauncher::Core::Send(IPC::Message* message) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
-  if (ipc_channel_.get()) {
-    ipc_channel_->Send(message);
-  } else {
-    delete message;
-  }
+  launcher_delegate_->Send(message);
 }
 
 void WorkerProcessLauncher::Core::OnObjectSignaled(HANDLE object) {
@@ -246,14 +199,15 @@ void WorkerProcessLauncher::Core::OnObjectSignaled(HANDLE object) {
 bool WorkerProcessLauncher::Core::OnMessageReceived(
     const IPC::Message& message) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
-  DCHECK(ipc_channel_.get() != NULL);
+
+  if (!ipc_enabled_)
+    return false;
 
   return worker_delegate_->OnMessageReceived(message);
 }
 
 void WorkerProcessLauncher::Core::OnChannelConnected(int32 peer_pid) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
-  DCHECK(ipc_channel_.get() != NULL);
 
   // |peer_pid| is send by the client and cannot be trusted.
   // GetNamedPipeClientProcessId() is not available on XP. The pipe's security
@@ -263,12 +217,12 @@ void WorkerProcessLauncher::Core::OnChannelConnected(int32 peer_pid) {
   // If we'd like to be able to launch low-privileged workers and let them
   // connect back, the pipe handle should be passed to the worker instead of
   // the pipe name.
-  worker_delegate_->OnChannelConnected();
+  if (ipc_enabled_)
+    worker_delegate_->OnChannelConnected();
 }
 
 void WorkerProcessLauncher::Core::OnChannelError() {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
-  DCHECK(ipc_channel_.get() != NULL);
 
   // Schedule a delayed termination of the worker process. Usually, the pipe is
   // disconnected when the worker process is about to exit. Waiting a little bit
@@ -283,96 +237,27 @@ WorkerProcessLauncher::Core::~Core() {
   DCHECK(stopping_);
 }
 
-// Creates the server end of the Chromoting IPC channel.
-bool WorkerProcessLauncher::Core::CreatePipeForIpcChannel(
-    const std::string& channel_name,
-    const std::string& pipe_security_descriptor,
-    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 (!ConvertStringSecurityDescriptorToSecurityDescriptor(
-          UTF8ToUTF16(pipe_security_descriptor).c_str(),
-          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 the channel name to the pipe name.
-  std::string 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.
-  base::win::ScopedHandle pipe;
-  pipe.Set(CreateNamedPipe(
-      UTF8ToUTF16(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.IsValid()) {
-    LOG_GETLASTERROR(ERROR) <<
-        "Failed to create the server end of the Chromoting IPC channel";
-    LocalFree(security_attributes.lpSecurityDescriptor);
-    return false;
-  }
-
-  LocalFree(security_attributes.lpSecurityDescriptor);
-
-  *pipe_out = pipe.Pass();
-  return true;
-}
-
-// N.B. Copied from src/content/common/child_process_host_impl.cc
-std::string WorkerProcessLauncher::Core::GenerateRandomChannelId() {
-  return base::StringPrintf("%d.%p.%d",
-                            base::GetCurrentProcId(), this,
-                            base::RandInt(0, std::numeric_limits<int>::max()));
-}
-
 void WorkerProcessLauncher::Core::LaunchWorker() {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
-  DCHECK(ipc_channel_.get() == NULL);
+  DCHECK(!ipc_enabled_);
   DCHECK(!launch_success_timer_->IsRunning());
   DCHECK(!launch_timer_->IsRunning());
-  DCHECK(!pipe_.IsValid());
   DCHECK(!process_exit_event_.IsValid());
   DCHECK(process_watcher_.GetWatchedObject() == NULL);
 
-  std::string channel_name = GenerateRandomChannelId();
-  if (CreatePipeForIpcChannel(channel_name, pipe_security_descriptor_,
-                              &pipe_)) {
-    // Wrap the pipe into an IPC channel.
-    ipc_channel_.reset(new IPC::ChannelProxy(
-        IPC::ChannelHandle(pipe_),
-        IPC::Channel::MODE_SERVER,
-        this,
-        io_task_runner_));
-
-    // Launch the process and attach an object watcher to the returned process
-    // handle so that we get notified if the process terminates.
-    if (launcher_delegate_->LaunchProcess(channel_name, &process_exit_event_)) {
-      if (process_watcher_.StartWatching(process_exit_event_, this)) {
-        // Record a successful launch once the process has been running for at
-        // least two seconds.
-        launch_success_timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(2),
-                                     this, &Core::RecordSuccessfullLaunch);
-        return;
-      }
-
-      launcher_delegate_->KillProcess(CONTROL_C_EXIT);
+  // Launch the process and attach an object watcher to the returned process
+  // handle so that we get notified if the process terminates.
+  if (launcher_delegate_->LaunchProcess(this, &process_exit_event_)) {
+    if (process_watcher_.StartWatching(process_exit_event_, this)) {
+      ipc_enabled_ = true;
+      // Record a successful launch once the process has been running for at
+      // least two seconds.
+      launch_success_timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(2),
+                                   this, &Core::RecordSuccessfullLaunch);

[simonmorris, 2012/10/12 16:31:13]

Successfull -> Successful

[alexeypa (please no reviews), 2012/10/12 18:44:39]

Done.

+      return;
     }
+
+    launcher_delegate_->KillProcess(CONTROL_C_EXIT);
   }
 
   launch_backoff_.InformOfRequest(false);
@@ -397,8 +282,8 @@ void WorkerProcessLauncher::Core::StopWorker() {
   // Keep |this| alive until the worker process is terminated.
   self_ = this;
 
-  ipc_channel_.reset();
-  pipe_.Close();
+  // Ignore any remaining IPC messages.
+  ipc_enabled_ = false;
 
   // Kill the process if it has been started already.
   if (process_watcher_.GetWatchedObject() != NULL) {
@@ -406,12 +291,9 @@ void WorkerProcessLauncher::Core::StopWorker() {
     return;
   }
 
-  ipc_error_timer_->Stop();
-
-  DCHECK(ipc_channel_.get() == NULL);
-  DCHECK(!pipe_.IsValid());
   DCHECK(process_watcher_.GetWatchedObject() == NULL);
 
+  ipc_error_timer_->Stop();
   process_exit_event_.Close();
 
   // Do not relaunch the worker process if the caller has asked us to stop.
@@ -443,12 +325,10 @@ void WorkerProcessLauncher::Core::StopWorker() {
 
 WorkerProcessLauncher::WorkerProcessLauncher(
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
     scoped_ptr<Delegate> launcher_delegate,
-    WorkerProcessIpcDelegate* worker_delegate,
-    const std::string& pipe_security_descriptor) {
-  core_ = new Core(caller_task_runner, io_task_runner, launcher_delegate.Pass(),
-                   worker_delegate, pipe_security_descriptor);
+    WorkerProcessIpcDelegate* worker_delegate) {
+  core_ = new Core(caller_task_runner, launcher_delegate.Pass(),
+                   worker_delegate);
   core_->Start();
 }