Don't make classes derive from ChildProcessHost, and instead have them use it through composition. This cleans up the code and makes it easier to understand (as well as more closely conform to the Google C++ style guide). It also makes it possible to add an interface around ChildProcessHost in a future change.

BUG=98716
Review URL: http://codereview.chromium.org/8774040

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112769 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 98 insertions, 145 deletions

diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index e4d5776..c2aa4ec 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -117,6 +117,7 @@
 #include "content/browser/plugin_service.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/tab_contents/interstitial_page.h"
+#include "content/common/child_process_host.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/common_param_traits.h"
diff --git a/chrome/browser/nacl_host/nacl_broker_host_win.cc b/chrome/browser/nacl_host/nacl_broker_host_win.cc
index 8bd6077..0c05a8d 100644
--- a/chrome/browser/nacl_host/nacl_broker_host_win.cc
+++ b/chrome/browser/nacl_host/nacl_broker_host_win.cc
@@ -14,6 +14,7 @@
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/nacl_cmd_line.h"
 #include "chrome/common/nacl_messages.h"
+#include "content/common/child_process_host.h"
 
 NaClBrokerHost::NaClBrokerHost()
     : BrowserChildProcessHost(content::PROCESS_TYPE_NACL_BROKER),
@@ -25,7 +26,7 @@ NaClBrokerHost::~NaClBrokerHost() {
 
 bool NaClBrokerHost::Init() {
   // Create the channel that will be used for communicating with the broker.
-  if (!CreateChannel())
+  if (!child_process_host()->CreateChannel())
     return false;
 
   // Create the path to the nacl broker/loader executable.
@@ -39,7 +40,8 @@ bool NaClBrokerHost::Init() {
 
   cmd_line->AppendSwitchASCII(switches::kProcessType,
                               switches::kNaClBrokerProcess);
-  cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id());
+  cmd_line->AppendSwitchASCII(switches::kProcessChannelID,
+                              child_process_host()->channel_id());
   if (logging::DialogsAreSuppressed())
     cmd_line->AppendSwitch(switches::kNoErrorDialogs);
 
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index c84f416..46a030c 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -23,6 +23,7 @@
 #include "chrome/common/nacl_messages.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
+#include "content/common/child_process_host.h"
 #include "ipc/ipc_switches.h"
 #include "native_client/src/shared/imc/nacl_imc.h"
 
@@ -116,6 +117,21 @@ NaClProcessHost::NaClProcessHost(const std::wstring& url)
 }
 
 NaClProcessHost::~NaClProcessHost() {
+  int exit_code;
+  GetChildTerminationStatus(&exit_code);
+  std::string message =
+      base::StringPrintf("NaCl process exited with status %i (0x%x)",
+                         exit_code, exit_code);
+  if (exit_code == 0) {
+    LOG(INFO) << message;
+  } else {
+    LOG(ERROR) << message;
+  }
+
+#if defined(OS_WIN)
+  NaClBrokerService::GetInstance()->OnLoaderDied();
+#endif
+
   for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
     if (nacl::Close(internal_->sockets_for_renderer[i]) != 0) {
       LOG(ERROR) << "nacl::Close() failed";
@@ -212,7 +228,7 @@ bool NaClProcessHost::Launch(
 }
 
 bool NaClProcessHost::LaunchSelLdr() {
-  if (!CreateChannel())
+  if (!child_process_host()->CreateChannel())
     return false;
 
   CommandLine::StringType nacl_loader_prefix;
@@ -230,14 +246,15 @@ bool NaClProcessHost::LaunchSelLdr() {
   // to accomodate this request will exist in the child process' address
   // space. Disable PIE for NaCl processes. See http://crbug.com/90221 and
   // http://code.google.com/p/nativeclient/issues/detail?id=2043.
-  int flags = CHILD_NO_PIE;
+  int flags = ChildProcessHost::CHILD_NO_PIE;
 #elif defined(OS_LINUX)
-  int flags = nacl_loader_prefix.empty() ? CHILD_ALLOW_SELF : CHILD_NORMAL;
+  int flags = nacl_loader_prefix.empty() ? ChildProcessHost::CHILD_ALLOW_SELF :
+                                           ChildProcessHost::CHILD_NORMAL;
 #else
-  int flags = CHILD_NORMAL;
+  int flags = ChildProcessHost::CHILD_NORMAL;
 #endif
 
-  FilePath exe_path = GetChildPath(flags);
+  FilePath exe_path = ChildProcessHost::GetChildPath(flags);
   if (exe_path.empty())
     return false;
 
@@ -246,7 +263,8 @@ bool NaClProcessHost::LaunchSelLdr() {
 
   cmd_line->AppendSwitchASCII(switches::kProcessType,
                               switches::kNaClLoaderProcess);
-  cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id());
+  cmd_line->AppendSwitchASCII(switches::kProcessChannelID,
+                              child_process_host()->channel_id());
   if (logging::DialogsAreSuppressed())
     cmd_line->AppendSwitch(switches::kNoErrorDialogs);
 
@@ -257,7 +275,7 @@ bool NaClProcessHost::LaunchSelLdr() {
 #if defined(OS_WIN)
   if (RunningOnWOW64()) {
     return NaClBrokerService::GetInstance()->LaunchLoader(
-        this, ASCIIToWide(channel_id()));
+        this, ASCIIToWide(child_process_host()->channel_id()));
   } else {
     BrowserChildProcessHost::Launch(FilePath(), cmd_line);
   }
@@ -282,24 +300,6 @@ base::TerminationStatus NaClProcessHost::GetChildTerminationStatus(
   return BrowserChildProcessHost::GetChildTerminationStatus(exit_code);
 }
 
-void NaClProcessHost::OnChildDied() {
-  int exit_code;
-  GetChildTerminationStatus(&exit_code);
-  std::string message =
-    base::StringPrintf("NaCl process exited with status %i (0x%x)",
-                       exit_code, exit_code);
-  if (exit_code == 0) {
-    LOG(INFO) << message;
-  } else {
-    LOG(ERROR) << message;
-  }
-
-#if defined(OS_WIN)
-  NaClBrokerService::GetInstance()->OnLoaderDied();
-#endif
-  BrowserChildProcessHost::OnChildDied();
-}
-
 // This only ever runs on the BrowserThread::FILE thread.
 // If multiple tasks are posted, the later ones are no-ops.
 void NaClBrowser::OpenIrtLibraryFile() {
@@ -519,7 +519,3 @@ bool NaClProcessHost::OnMessageReceived(const IPC::Message& msg) {
   NOTREACHED() << "Invalid message with type = " << msg.type();
   return false;
 }
-
-bool NaClProcessHost::CanShutdown() {
-  return true;
-}
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index 1ccb0f8..c2d8d8f 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -43,7 +43,6 @@ class NaClProcessHost : public BrowserChildProcessHost {
  protected:
   virtual base::TerminationStatus GetChildTerminationStatus(
       int* exit_code) OVERRIDE;
-  virtual void OnChildDied() OVERRIDE;
 
  private:
   // Internal class that holds the nacl::Handle objecs so that
@@ -59,8 +58,6 @@ class NaClProcessHost : public BrowserChildProcessHost {
   void IrtReady();
   void SendStart(base::PlatformFile irt_file);
 
-  virtual bool CanShutdown() OVERRIDE;
-
  private:
   // The ChromeRenderMessageFilter that requested this NaCl process.  We use
   // this for sending the reply once the process has started.
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 854a235..f8d1793 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -604,8 +604,6 @@
       'sources': [
         'service/chrome_service_application_mac.h',
         'service/chrome_service_application_mac.mm',
-        'service/service_child_process_host.cc',
-        'service/service_child_process_host.h',
         'service/service_ipc_server.cc',
         'service/service_ipc_server.h',
         'service/service_main.cc',
diff --git a/chrome/service/service_child_process_host.cc b/chrome/service/service_child_process_host.cc
deleted file mode 100644
index 234b899..0000000
--- a/chrome/service/service_child_process_host.cc
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright (c) 2011 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.
-
-#include "chrome/service/service_child_process_host.h"
-
-#include "base/command_line.h"
-#include "base/logging.h"
-#include "base/process_util.h"
-#include "chrome/common/chrome_switches.h"
-#include "content/public/common/result_codes.h"
-
-#if defined(OS_WIN)
-#include "base/file_path.h"
-#include "content/common/sandbox_policy.h"
-#endif  // defined(OS_WIN)
-
-ServiceChildProcessHost::ServiceChildProcessHost()
-    : handle_(base::kNullProcessHandle) {
-}
-
-ServiceChildProcessHost::~ServiceChildProcessHost() {
-  // We need to kill the child process when the host dies.
-  base::KillProcess(handle_, content::RESULT_CODE_NORMAL_EXIT, false);
-}
-
-bool ServiceChildProcessHost::Launch(CommandLine* cmd_line,
-                                     bool no_sandbox,
-                                     const FilePath& exposed_dir) {
-#if !defined(OS_WIN)
-  // TODO(sanjeevr): Implement for non-Windows OSes.
-  NOTIMPLEMENTED();
-  return false;
-#else  // !defined(OS_WIN)
-
-  if (no_sandbox) {
-    base::ProcessHandle process = base::kNullProcessHandle;
-    cmd_line->AppendSwitch(switches::kNoSandbox);
-    base::LaunchProcess(*cmd_line, base::LaunchOptions(), &handle_);
-  } else {
-    handle_ = sandbox::StartProcessWithAccess(cmd_line, exposed_dir);
-  }
-  return (handle_ != base::kNullProcessHandle);
-#endif  // !defined(OS_WIN)
-}
diff --git a/chrome/service/service_child_process_host.h b/chrome/service/service_child_process_host.h
deleted file mode 100644
index 44337be..0000000
--- a/chrome/service/service_child_process_host.h
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2011 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 CHROME_SERVICE_SERVICE_CHILD_PROCESS_HOST_H_
-#define CHROME_SERVICE_SERVICE_CHILD_PROCESS_HOST_H_
-#pragma once
-
-#include "base/process.h"
-#include "content/common/child_process_host.h"
-
-class CommandLine;
-
-// Plugins/workers and other child processes that live on the IO thread should
-// derive from this class.
-//
-class ServiceChildProcessHost : public ChildProcessHost {
- public:
-  virtual ~ServiceChildProcessHost();
-
- protected:
-  ServiceChildProcessHost();
-
-  // Derived classes call this to launch the child process synchronously.
-  // TODO(sanjeevr): Determine whether we need to make the launch asynchronous.
-  // |exposed_dir| is the path to tbe exposed to the sandbox. This is ignored
-  // if |no_sandbox| is true.
-  bool Launch(CommandLine* cmd_line,
-              bool no_sandbox,
-              const FilePath& exposed_dir);
-
-  base::ProcessHandle handle() const { return handle_; }
-
- private:
-  base::ProcessHandle handle_;
-
-  DISALLOW_COPY_AND_ASSIGN(ServiceChildProcessHost);
-};
-
-#endif  // CHROME_SERVICE_SERVICE_CHILD_PROCESS_HOST_H_
diff --git a/chrome/service/service_utility_process_host.cc b/chrome/service/service_utility_process_host.cc
index 8352526..d82007a 100644
--- a/chrome/service/service_utility_process_host.cc
+++ b/chrome/service/service_utility_process_host.cc
@@ -7,31 +7,41 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
+#include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
+#include "base/process_util.h"
 #include "base/scoped_temp_dir.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_utility_messages.h"
+#include "content/common/child_process_host.h"
+#include "content/public/common/result_codes.h"
 #include "ipc/ipc_switches.h"
 #include "printing/page_range.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/rect.h"
 
 #if defined(OS_WIN)
+#include "base/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/win/scoped_handle.h"
+#include "content/common/sandbox_policy.h"
 #include "printing/emf_win.h"
 #endif
 
 ServiceUtilityProcessHost::ServiceUtilityProcessHost(
     Client* client, base::MessageLoopProxy* client_message_loop_proxy)
-        : client_(client),
+        : handle_(base::kNullProcessHandle),
+          client_(client),
           client_message_loop_proxy_(client_message_loop_proxy),
           waiting_for_reply_(false) {
+  child_process_host_.reset(new ChildProcessHost(this));
 }
 
 ServiceUtilityProcessHost::~ServiceUtilityProcessHost() {
+  // We need to kill the child process when the host dies.
+  base::KillProcess(handle_, content::RESULT_CODE_NORMAL_EXIT, false);
 }
 
 bool ServiceUtilityProcessHost::StartRenderPDFPagesToMetafile(
@@ -72,7 +82,7 @@ bool ServiceUtilityProcessHost::StartRenderPDFPagesToMetafile(
   if (!pdf_file_in_utility_process)
     return false;
   waiting_for_reply_ = true;
-  return Send(
+  return child_process_host_->Send(
       new ChromeUtilityMsg_RenderPDFPagesToMetafile(
           pdf_file_in_utility_process,
           metafile_path_,
@@ -87,12 +97,13 @@ bool ServiceUtilityProcessHost::StartGetPrinterCapsAndDefaults(
   if (!StartProcess(true, exposed_path))
     return false;
   waiting_for_reply_ = true;
-  return Send(new ChromeUtilityMsg_GetPrinterCapsAndDefaults(printer_name));
+  return child_process_host_->Send(
+      new ChromeUtilityMsg_GetPrinterCapsAndDefaults(printer_name));
 }
 
 bool ServiceUtilityProcessHost::StartProcess(bool no_sandbox,
                                              const FilePath& exposed_dir) {
-  if (!CreateChannel())
+  if (!child_process_host_->CreateChannel())
     return false;
 
   FilePath exe_path = GetUtilityProcessCmd();
@@ -103,34 +114,57 @@ bool ServiceUtilityProcessHost::StartProcess(bool no_sandbox,
 
   CommandLine cmd_line(exe_path);
   cmd_line.AppendSwitchASCII(switches::kProcessType, switches::kUtilityProcess);
-  cmd_line.AppendSwitchASCII(switches::kProcessChannelID, channel_id());
+  cmd_line.AppendSwitchASCII(switches::kProcessChannelID,
+                             child_process_host_->channel_id());
   cmd_line.AppendSwitch(switches::kLang);
 
   return Launch(&cmd_line, no_sandbox, exposed_dir);
 }
 
+bool ServiceUtilityProcessHost::Launch(CommandLine* cmd_line,
+                                       bool no_sandbox,
+                                       const FilePath& exposed_dir) {
+#if !defined(OS_WIN)
+  // TODO(sanjeevr): Implement for non-Windows OSes.
+  NOTIMPLEMENTED();
+  return false;
+#else  // !defined(OS_WIN)
+
+  if (no_sandbox) {
+    base::ProcessHandle process = base::kNullProcessHandle;
+    cmd_line->AppendSwitch(switches::kNoSandbox);
+    base::LaunchProcess(*cmd_line, base::LaunchOptions(), &handle_);
+  } else {
+    handle_ = sandbox::StartProcessWithAccess(cmd_line, exposed_dir);
+  }
+  return (handle_ != base::kNullProcessHandle);
+#endif  // !defined(OS_WIN)
+}
+
 FilePath ServiceUtilityProcessHost::GetUtilityProcessCmd() {
 #if defined(OS_LINUX)
-  int flags = CHILD_ALLOW_SELF;
+  int flags = ChildProcessHost::CHILD_ALLOW_SELF;
 #else
-  int flags = CHILD_NORMAL;
+  int flags = ChildProcessHost::CHILD_NORMAL;
 #endif
-  return GetChildPath(flags);
+  return ChildProcessHost::GetChildPath(flags);
 }
 
 bool ServiceUtilityProcessHost::CanShutdown() {
   return true;
 }
 
-void ServiceUtilityProcessHost::OnChildDied() {
+void ServiceUtilityProcessHost::OnChildDisconnected() {
   if (waiting_for_reply_) {
     // If we are yet to receive a reply then notify the client that the
     // child died.
     client_message_loop_proxy_->PostTask(
         FROM_HERE, base::Bind(&Client::OnChildDied, client_.get()));
   }
-  // The base class implementation will delete |this|.
-  ServiceChildProcessHost::OnChildDied();
+  delete this;
+}
+
+void ServiceUtilityProcessHost::ShutdownStarted() {
 }
 
 bool ServiceUtilityProcessHost::OnMessageReceived(const IPC::Message& message) {
diff --git a/chrome/service/service_utility_process_host.h b/chrome/service/service_utility_process_host.h
index 49e2450..77cb2f7 100644
--- a/chrome/service/service_utility_process_host.h
+++ b/chrome/service/service_utility_process_host.h
@@ -14,11 +14,15 @@
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/process.h"
 #include "base/task.h"
 #include "ipc/ipc_channel.h"
-#include "chrome/service/service_child_process_host.h"
+#include "content/public/common/child_process_host_delegate.h"
 #include "printing/pdf_render_settings.h"
 
+class ChildProcessHost;
+class CommandLine;
 class ScopedTempDir;
 
 namespace base {
@@ -34,7 +38,7 @@ struct PrinterCapsAndDefaults;
 // Acts as the service-side host to a utility child process. A
 // utility process is a short-lived sandboxed process that is created to run
 // a specific task.
-class ServiceUtilityProcessHost : public ServiceChildProcessHost {
+class ServiceUtilityProcessHost : public content::ChildProcessHostDelegate {
  public:
   // Consumers of ServiceUtilityProcessHost must implement this interface to
   // get results back.  All functions are called on the thread passed along
@@ -101,9 +105,11 @@ class ServiceUtilityProcessHost : public ServiceChildProcessHost {
   // Allows this method to be overridden for tests.
   virtual FilePath GetUtilityProcessCmd();
 
-  // ChildProcessHost implementation.
+  // ChildProcessHostDelegate implementation:
   virtual bool CanShutdown() OVERRIDE;
-  virtual void OnChildDied() OVERRIDE;
+  virtual void OnChildDisconnected() OVERRIDE;
+  virtual void ShutdownStarted() OVERRIDE;
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
  private:
   // Starts a process.  Returns true iff it succeeded. |exposed_dir| is the
@@ -111,22 +117,26 @@ class ServiceUtilityProcessHost : public ServiceChildProcessHost {
   // true.
   bool StartProcess(bool no_sandbox, const FilePath& exposed_dir);
 
-  // IPC messages:
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-  // Called when at least one page in the specified PDF has been rendered
-  // successfully into metafile_path_;
+  // Launch the child process synchronously.
+  // TODO(sanjeevr): Determine whether we need to make the launch asynchronous.
+  // |exposed_dir| is the path to tbe exposed to the sandbox. This is ignored
+  // if |no_sandbox| is true.
+  bool Launch(CommandLine* cmd_line,
+              bool no_sandbox,
+              const FilePath& exposed_dir);
+
+  base::ProcessHandle handle() const { return handle_; }
+
+  // Messages handlers:
   void OnRenderPDFPagesToMetafileSucceeded(int highest_rendered_page_number);
-  // Called when PDF rendering failed.
   void OnRenderPDFPagesToMetafileFailed();
-  // Called when the printer capabilities and defaults have been
-  // retrieved successfully.
   void OnGetPrinterCapsAndDefaultsSucceeded(
       const std::string& printer_name,
       const printing::PrinterCapsAndDefaults& caps_and_defaults);
-  // Called when the printer capabilities and defaults could not be
-  // retrieved successfully.
   void OnGetPrinterCapsAndDefaultsFailed(const std::string& printer_name);
 
+  scoped_ptr<ChildProcessHost> child_process_host_;
+  base::ProcessHandle handle_;
   // A pointer to our client interface, who will be informed of progress.
   scoped_refptr<Client> client_;
   scoped_refptr<base::MessageLoopProxy> client_message_loop_proxy_;