The service process now enables a timebomb on startup to check if its services are needed. If not, it shuts down. Also the ShutdownIfNeeded method checks to see if clients are connected before shutting down. And we only try to recreate the channel if we were previously connected to a client.

BUG=None.
TEST=Run the service process without any enabled services. It should die within a minute.

Review URL: http://codereview.chromium.org/3562004

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

5 files changed, 46 insertions, 11 deletions

diff --git a/chrome/service/service_ipc_server.cc b/chrome/service/service_ipc_server.cc
index 958eade..f912701 100644
--- a/chrome/service/service_ipc_server.cc
+++ b/chrome/service/service_ipc_server.cc
@@ -10,7 +10,7 @@
 #include "ipc/ipc_logging.h"
 
 ServiceIPCServer::ServiceIPCServer(const std::string& channel_name)
-    : channel_name_(channel_name) {
+    : channel_name_(channel_name), client_connected_(false) {
 }
 
 bool ServiceIPCServer::Init() {
@@ -50,11 +50,19 @@ ServiceIPCServer::~ServiceIPCServer() {
   channel_->ClearIPCMessageLoop();
 }
 
+void ServiceIPCServer::OnChannelConnected(int32 peer_pid) {
+  DCHECK(!client_connected_);
+  client_connected_ = true;
+}
+
 void ServiceIPCServer::OnChannelError() {
   // When a client (typically a browser process) disconnects, the pipe is
   // closed and we get an OnChannelError. Since we want to keep servicing
   // client requests, we will recreate the channel.
-  CreateChannel();
+  bool client_was_connected = client_connected_;
+  client_connected_ = false;
+  if (client_was_connected)
+    CreateChannel();
 }
 
 bool ServiceIPCServer::Send(IPC::Message* msg) {
diff --git a/chrome/service/service_ipc_server.h b/chrome/service/service_ipc_server.h
index 02bf3bb..1c8c19b 100644
--- a/chrome/service/service_ipc_server.h
+++ b/chrome/service/service_ipc_server.h
@@ -30,10 +30,13 @@ class ServiceIPCServer : public IPC::Channel::Listener,
   // lifetime is less than the main thread.
   IPC::SyncMessageFilter* sync_message_filter() { return sync_message_filter_; }
 
+  bool is_client_connected() const { return client_connected_; }
+
 
  private:
   // IPC::Channel::Listener implementation:
   virtual void OnMessageReceived(const IPC::Message& msg);
+  virtual void OnChannelConnected(int32 peer_pid);
   virtual void OnChannelError();
 
   // IPC message handlers.
@@ -54,6 +57,8 @@ class ServiceIPCServer : public IPC::Channel::Listener,
 
   std::string channel_name_;
   scoped_ptr<IPC::SyncChannel> channel_;
+  // Indicates whether a client is currently connected to the channel.
+  bool client_connected_;
 
   // Allows threads other than the main thread to send sync messages.
   scoped_refptr<IPC::SyncMessageFilter> sync_message_filter_;
diff --git a/chrome/service/service_main.cc b/chrome/service/service_main.cc
index 28eda80..f71c405 100644
--- a/chrome/service/service_main.cc
+++ b/chrome/service/service_main.cc
@@ -29,7 +29,11 @@ int ServiceProcessMain(const MainFunctionParams& parameters) {
 #endif   // defined(OS_WIN)
 
   ServiceProcess service_process;
-  service_process.Initialize(&main_message_loop, parameters.command_line_);
+  if (!service_process.Initialize(&main_message_loop,
+                                  parameters.command_line_)) {
+    LOG(ERROR) << "Service process failed to initialize";
+    return 0;
+  }
 
   MessageLoop::current()->Run();
   service_process.Teardown();
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index f22db02..821bfd2 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -114,6 +114,9 @@ bool ServiceProcess::Initialize(MessageLoop* message_loop,
   // After the IPC server has started we signal that the service process is
   // running.
   SignalServiceProcessRunning(kServiceProcessCloudPrint);
+
+  // See if we need to stay running.
+  ScheduleShutdownCheck();
   return true;
 }
 
@@ -183,16 +186,28 @@ void ServiceProcess::OnServiceDisabled() {
   if (0 == enabled_services_) {
     RemoveServiceProcessFromAutoStart();
     // We will wait for some time to respond to IPCs before shutting down.
-    MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        NewRunnableMethod(this, &ServiceProcess::ShutdownIfNoServices),
-        kShutdownDelay);
+    ScheduleShutdownCheck();
   }
 }
 
-void ServiceProcess::ShutdownIfNoServices() {
+void ServiceProcess::ScheduleShutdownCheck() {
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      NewRunnableMethod(this, &ServiceProcess::ShutdownIfNeeded),
+      kShutdownDelay);
+}
+
+void ServiceProcess::ShutdownIfNeeded() {
   if (0 == enabled_services_) {
-    Shutdown();
+    if (ipc_server_->is_client_connected()) {
+      // If there is a client connected, we need to try again later.
+      // Note that there is still a timing window here because a client may
+      // decide to connect at this point.
+      // TODO(sanjeevr): Fix this timing window.
+      ScheduleShutdownCheck();
+    } else {
+      Shutdown();
+    }
   }
 }
 
diff --git a/chrome/service/service_process.h b/chrome/service/service_process.h
index 55c6d62..3e99d21 100644
--- a/chrome/service/service_process.h
+++ b/chrome/service/service_process.h
@@ -104,8 +104,11 @@ class ServiceProcess : public RemotingDirectoryService::Client,
 #endif
 
  private:
-  // Shuts down the process if no services are enabled.
-  void ShutdownIfNoServices();
+  // Schedule a call to ShutdownIfNeeded.
+  void ScheduleShutdownCheck();
+  // Shuts down the process if no services are enabled and no clients are
+  // connected.
+  void ShutdownIfNeeded();
   // Called exactly ONCE per process instance for each service that gets
   // enabled in this process.
   void OnServiceEnabled();