Don't pass ProcessHandle through ChildProcessHostDelegate.

ChildProcessHostDelegate doesn't make clear what ownership rules apply to
the process handle and as a result one of the implementations assume that
the caller has to delete the handle (ServiceUtilityProcessHost) while the
other one assumes that it does not (BrowserChildProcessHostImpl).

On the other hand, one caller closes the handle (ChildProcessHostImpl)
and the other does not (PpapiPluginProcessHost).

This CL enforces the API of not transfering ownership when getting the
process handle (as both implementations keep using the handle after they
receive the call).

BUG=417532

Review URL: https://codereview.chromium.org/725353003

Cr-Commit-Position: refs/heads/master@{#304664}

2 files changed, 14 insertions, 11 deletions

diff --git a/chrome/service/service_utility_process_host.cc b/chrome/service/service_utility_process_host.cc
index f771662..fcbc1bf 100644
--- a/chrome/service/service_utility_process_host.cc
+++ b/chrome/service/service_utility_process_host.cc
@@ -17,6 +17,7 @@
 #include "base/metrics/histogram.h"
 #include "base/process/kill.h"
 #include "base/process/launch.h"
+#include "base/process/process_handle.h"
 #include "base/task_runner_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_utility_printing_messages.h"
@@ -153,8 +154,7 @@ class ServiceUtilityProcessHost::PdfToEmfState {
 ServiceUtilityProcessHost::ServiceUtilityProcessHost(
     Client* client,
     base::MessageLoopProxy* client_message_loop_proxy)
-    : handle_(base::kNullProcessHandle),
-      client_(client),
+    : client_(client),
       client_message_loop_proxy_(client_message_loop_proxy),
       waiting_for_reply_(false),
       weak_ptr_factory_(this) {
@@ -163,7 +163,7 @@ ServiceUtilityProcessHost::ServiceUtilityProcessHost(
 
 ServiceUtilityProcessHost::~ServiceUtilityProcessHost() {
   // We need to kill the child process when the host dies.
-  base::KillProcess(handle_, content::RESULT_CODE_NORMAL_EXIT, false);
+  base::KillProcess(process_.Handle(), content::RESULT_CODE_NORMAL_EXIT, false);
 }
 
 bool ServiceUtilityProcessHost::StartRenderPDFPagesToMetafile(
@@ -233,12 +233,15 @@ bool ServiceUtilityProcessHost::Launch(base::CommandLine* cmd_line,
                                        bool no_sandbox) {
   if (no_sandbox) {
     cmd_line->AppendSwitch(switches::kNoSandbox);
-    base::LaunchProcess(*cmd_line, base::LaunchOptions(), &handle_);
+    base::ProcessHandle handle;
+    if (base::LaunchProcess(*cmd_line, base::LaunchOptions(), &handle))
+      process_ = base::Process(handle);
   } else {
     ServiceSandboxedProcessLauncherDelegate delegate;
-    handle_ = content::StartSandboxedProcess(&delegate, cmd_line);
+    process_ =
+        base::Process(content::StartSandboxedProcess(&delegate, cmd_line));
   }
-  return (handle_ != base::kNullProcessHandle);
+  return process_.IsValid();
 }
 
 bool ServiceUtilityProcessHost::Send(IPC::Message* msg) {
@@ -294,8 +297,8 @@ bool ServiceUtilityProcessHost::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-base::ProcessHandle ServiceUtilityProcessHost::GetHandle() const {
-  return handle_;
+const base::Process& ServiceUtilityProcessHost::GetProcess() const {
+  return process_;
 }
 
 void ServiceUtilityProcessHost::OnMetafileSpooled(bool success) {
diff --git a/chrome/service/service_utility_process_host.h b/chrome/service/service_utility_process_host.h
index 6b320aa..db9d884 100644
--- a/chrome/service/service_utility_process_host.h
+++ b/chrome/service/service_utility_process_host.h
@@ -115,7 +115,7 @@ class ServiceUtilityProcessHost : public content::ChildProcessHostDelegate {
   // ChildProcessHostDelegate implementation:
   virtual void OnChildDisconnected() override;
   virtual bool OnMessageReceived(const IPC::Message& message) override;
-  virtual base::ProcessHandle GetHandle() const override;
+  virtual const base::Process& GetProcess() const override;
 
  private:
   // Starts a process.  Returns true iff it succeeded.
@@ -124,7 +124,7 @@ class ServiceUtilityProcessHost : public content::ChildProcessHostDelegate {
   // Launch the child process synchronously.
   bool Launch(base::CommandLine* cmd_line, bool no_sandbox);
 
-  base::ProcessHandle handle() const { return handle_; }
+  base::ProcessHandle handle() const { return process_.Handle(); }
 
   void OnMetafileSpooled(bool success);
   void OnPDFToEmfFinished(bool success);
@@ -143,7 +143,7 @@ class ServiceUtilityProcessHost : public content::ChildProcessHostDelegate {
       const std::string& printer_name);
 
   scoped_ptr<content::ChildProcessHost> child_process_host_;
-  base::ProcessHandle handle_;
+  base::Process process_;
   // A pointer to our client interface, who will be informed of progress.
   scoped_refptr<Client> client_;
   scoped_refptr<base::MessageLoopProxy> client_message_loop_proxy_;