Terminate NaCl broker process when no loader processes are running

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/669019

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

7 files changed, 73 insertions, 14 deletions

diff --git a/chrome/browser/child_process_host.h b/chrome/browser/child_process_host.h
index 735bcd8..c7c27be 100644
--- a/chrome/browser/child_process_host.h
+++ b/chrome/browser/child_process_host.h
@@ -118,13 +118,13 @@ class ChildProcessHost : public ResourceDispatcherHost::Receiver,
 
   virtual bool DidChildCrash();
 
+  // Called when the child process goes away.
+  virtual void OnChildDied();
+
  private:
   // Sends the given notification to the notification service on the UI thread.
   void Notify(NotificationType type);
 
-  // Called when the child process goes away.
-  void OnChildDied();
-
   // By using an internal class as the IPC::Channel::Listener, we can intercept
   // OnMessageReceived/OnChannelConnected and do our own processing before
   // calling the subclass' implementation.
diff --git a/chrome/browser/nacl_host/nacl_broker_host.cc b/chrome/browser/nacl_host/nacl_broker_host.cc
index 92ed181..ec8aafd 100644
--- a/chrome/browser/nacl_host/nacl_broker_host.cc
+++ b/chrome/browser/nacl_host/nacl_broker_host.cc
@@ -16,7 +16,8 @@
 
 NaClBrokerHost::NaClBrokerHost(
     ResourceDispatcherHost* resource_dispatcher_host)
-    : ChildProcessHost(NACL_BROKER_PROCESS, resource_dispatcher_host) {
+    : ChildProcessHost(NACL_BROKER_PROCESS, resource_dispatcher_host),
+      stopping_(false) {
 }
 
 NaClBrokerHost::~NaClBrokerHost() {
@@ -72,3 +73,18 @@ void NaClBrokerHost::OnLoaderLaunched(const std::wstring& loader_channel_id,
                                       base::ProcessHandle handle) {
   NaClBrokerService::GetInstance()->OnLoaderLaunched(loader_channel_id, handle);
 }
+
+void NaClBrokerHost::StopBroker() {
+  stopping_ = true;
+  Send(new NaClProcessMsg_StopBroker());
+}
+
+void NaClBrokerHost::OnChildDied() {
+  if (!stopping_) {
+    // If the broker stops unexpectedly (and not when asked by the broker
+    // service), we need to notify the broker service. In any other case
+    // the broker service may have a new broker host pointer by this time.
+    NaClBrokerService::GetInstance()->OnBrokerDied();
+  }
+  ChildProcessHost::OnChildDied();
+}
diff --git a/chrome/browser/nacl_host/nacl_broker_host.h b/chrome/browser/nacl_host/nacl_broker_host.h
index 15dd18b..47c3e95 100644
--- a/chrome/browser/nacl_host/nacl_broker_host.h
+++ b/chrome/browser/nacl_host/nacl_broker_host.h
@@ -23,6 +23,12 @@ class NaClBrokerHost : public ChildProcessHost {
   // a Native Client loader process.
   bool LaunchLoader(const std::wstring& loader_channel_id);
 
+  // Stop the broker process.
+  void StopBroker();
+
+ protected:
+  virtual void OnChildDied();
+
  private:
   // ResourceDispatcherHost::Receiver implementation:
   virtual URLRequestContext* GetRequestContext(
@@ -40,6 +46,8 @@ class NaClBrokerHost : public ChildProcessHost {
   // IPC::Channel::Listener
   virtual void OnMessageReceived(const IPC::Message& msg);
 
+  bool stopping_;
+
   DISALLOW_COPY_AND_ASSIGN(NaClBrokerHost);
 };
 
diff --git a/chrome/browser/nacl_host/nacl_broker_service.cc b/chrome/browser/nacl_host/nacl_broker_service.cc
index 6a58c2f..69c919b 100644
--- a/chrome/browser/nacl_host/nacl_broker_service.cc
+++ b/chrome/browser/nacl_host/nacl_broker_service.cc
@@ -15,16 +15,18 @@ NaClBrokerService* NaClBrokerService::GetInstance() {
 NaClBrokerService::NaClBrokerService()
     : broker_started_(false),
       broker_host_(NULL),
+      loaders_running_(0),
       resource_dispatcher_host_(NULL),
       initialized_(false) {
 }
 
 void NaClBrokerService::Init(ResourceDispatcherHost* resource_dispatcher_host) {
-  if (initialized_) {
-    return;
-  }
-  resource_dispatcher_host_ = resource_dispatcher_host;
-  StartBroker();
+  if (!initialized_)
+    resource_dispatcher_host_ = resource_dispatcher_host;
+
+  if (broker_host_ == NULL)
+    StartBroker();
+
   initialized_ = true;
 }
 
@@ -51,9 +53,9 @@ bool NaClBrokerService::LaunchLoader(NaClProcessHost* nacl_process_host,
 
 void NaClBrokerService::OnBrokerStarted() {
   PendingLaunchesMap::iterator it;
-  for (it  = pending_launches_.begin(); it != pending_launches_.end(); it++) {
+  for (it  = pending_launches_.begin(); it != pending_launches_.end(); it++)
     broker_host_->LaunchLoader(it->first);
-  }
+
   broker_started_ = true;
 }
 
@@ -61,10 +63,28 @@ void NaClBrokerService::OnLoaderLaunched(const std::wstring& channel_id,
                                          base::ProcessHandle handle) {
   NaClProcessHost* client;
   PendingLaunchesMap::iterator it = pending_launches_.find(channel_id);
-  if (pending_launches_.end() == it) {
+  if (pending_launches_.end() == it)
     NOTREACHED();
-  }
+
   client = it->second;
   client->OnProcessLaunchedByBroker(handle);
   pending_launches_.erase(it);
+  ++loaders_running_;
+}
+
+void NaClBrokerService::OnLoaderDied() {
+  --loaders_running_;
+  // Stop the broker only if there are no loaders running or being launched.
+  if (loaders_running_ + pending_launches_.size() == 0 &&
+      broker_host_ != NULL) {
+    broker_host_->StopBroker();
+    // Reset the pointer to the broker host.
+    OnBrokerDied();
+  }
+}
+
+void NaClBrokerService::OnBrokerDied() {
+  // NaClBrokerHost object will be destructed by ChildProcessHost
+  broker_started_ = false;
+  broker_host_.release();
 }
diff --git a/chrome/browser/nacl_host/nacl_broker_service.h b/chrome/browser/nacl_host/nacl_broker_service.h
index 75e089d..8518cd4 100644
--- a/chrome/browser/nacl_host/nacl_broker_service.h
+++ b/chrome/browser/nacl_host/nacl_broker_service.h
@@ -36,6 +36,12 @@ class NaClBrokerService {
   void OnLoaderLaunched(const std::wstring& channel_id,
                         base::ProcessHandle handle);
 
+  // Called by NaClProcessHost when a loader process is terminated
+  void OnLoaderDied();
+
+  // Called by NaClBrokerHost when the broker process is terminated.
+  void OnBrokerDied();
+
  private:
   typedef std::map<std::wstring, NaClProcessHost*>
       PendingLaunchesMap;
@@ -47,6 +53,7 @@ class NaClBrokerService {
 
   bool broker_started_;
   scoped_ptr<NaClBrokerHost> broker_host_;
+  int loaders_running_;
   bool initialized_;
   ResourceDispatcherHost* resource_dispatcher_host_;
   PendingLaunchesMap pending_launches_;
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 9a08d54..1114cb7 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -125,10 +125,17 @@ bool NaClProcessHost::DidChildCrash() {
   return ChildProcessHost::DidChildCrash();
 }
 
+void NaClProcessHost::OnChildDied() {
+#if defined(OS_WIN)
+  NaClBrokerService::GetInstance()->OnLoaderDied();
+#endif
+  ChildProcessHost::OnChildDied();
+}
+
 void NaClProcessHost::OnProcessLaunched() {
   nacl::FileDescriptor imc_handle;
   base::ProcessHandle nacl_process_handle;
-#if NACL_WINDOWS
+#if defined(OS_WIN)
   // Duplicate the IMC handle
   // We assume the size of imc_handle has the same size as HANDLE, so the cast
   // below is safe.
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index a89b2da..28893fc 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -37,6 +37,7 @@ class NaClProcessHost : public ChildProcessHost {
 
  protected:
   virtual bool DidChildCrash();
+  virtual void OnChildDied();
 
  private:
   bool LaunchSelLdr();