Don't launch service process if it's already running

BUG=52891
TEST=browser_tests --gtest_filter=ServiceProcess*

Removed ServiceProcessControl::Connect() and just have a Launch() method.
The Launch() method will connect to the service process if the it has
already been launched but not connected. The method will
simply return if the process is already been launched connected.

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

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

3 files changed, 42 insertions, 25 deletions

diff --git a/chrome/browser/service/service_process_control.cc b/chrome/browser/service/service_process_control.cc
index b495364..05dc446 100644
--- a/chrome/browser/service/service_process_control.cc
+++ b/chrome/browser/service/service_process_control.cc
@@ -85,21 +85,21 @@ ServiceProcessControl::ServiceProcessControl(Profile* profile,
 ServiceProcessControl::~ServiceProcessControl() {
 }
 
-void ServiceProcessControl::Connect(Task* task) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+void ServiceProcessControl::ConnectInternal(Task* task) {
+  // If the channel has already been established then we run the task
+  // and return.
   if (channel_.get()) {
-    task->Run();
-    delete task;
+    if (task) {
+      task->Run();
+      delete task;
+    }
     return;
   }
 
-  // Saves the task.
+  // Actually going to connect.
+  LOG(INFO) << "Connecting to Service Process IPC Server";
   connect_done_task_.reset(task);
-  ConnectInternal();
-}
 
-void ServiceProcessControl::ConnectInternal() {
-  LOG(INFO) << "Connecting to Service Process IPC Server";
   // Run the IPC channel on the shared IO thread.
   base::Thread* io_thread = g_browser_process->io_thread();
 
@@ -114,11 +114,10 @@ void ServiceProcessControl::ConnectInternal() {
 
 void ServiceProcessControl::Launch(Task* task) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  if (channel_.get()) {
-    if (task) {
-      task->Run();
-      delete task;
-    }
+
+  // If the service process is already running then connects to it.
+  if (CheckServiceProcessRunning(kServiceProcessCloudPrint)) {
+    ConnectInternal(task);
     return;
   }
 
@@ -157,12 +156,9 @@ void ServiceProcessControl::Launch(Task* task) {
 void ServiceProcessControl::OnProcessLaunched(Task* task) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   if (launcher_->launched()) {
-    // Now use the launch task as the connect task.
-    connect_done_task_.reset(task);
-
     // After we have successfully created the service process we try to connect
     // to it. The launch task is transfered to a connect task.
-    ConnectInternal();
+    ConnectInternal(task);
   } else if (task) {
     // If we don't have process handle that means launching the service process
     // has failed.
diff --git a/chrome/browser/service/service_process_control.h b/chrome/browser/service/service_process_control.h
index 2577ec6..53f40de 100644
--- a/chrome/browser/service/service_process_control.h
+++ b/chrome/browser/service/service_process_control.h
@@ -54,14 +54,11 @@ class ServiceProcessControl : public IPC::Channel::Sender,
   // Return true if this object is connected to the service.
   bool is_connected() const { return channel_.get() != NULL; }
 
-  // Initialize the connection to the service process.
-  // |connect_done_task| is invoked if the connection has succeeded or failed.
-  // User should call is_connected() to check the connection status.
-  void Connect(Task* connect_done_task);
-
   // Create a new service process and connects to it.
   // |launch_done_task| is called if launching the service process has failed
   // or we have successfully launched the process and connected to it.
+  // If the service process is already running this method will try to connect
+  // to the service process.
   void Launch(Task* launch_done_task);
 
   // IPC::Channel::Listener implementation.
@@ -99,8 +96,9 @@ class ServiceProcessControl : public IPC::Channel::Sender,
   // Method called by Launcher when the service process is launched.
   void OnProcessLaunched(Task* launch_done_task);
 
-  // Used internally to connect to the service process.
-  void ConnectInternal();
+  // Used internally to connect to the service process. |task| is executed
+  // when the connection is made or an error occurred.
+  void ConnectInternal(Task* task);
 
   Profile* profile_;
   ServiceProcessType type_;
diff --git a/chrome/browser/service/service_process_control_browsertest.cc b/chrome/browser/service/service_process_control_browsertest.cc
index 0dda5fa..30e65c1 100644
--- a/chrome/browser/service/service_process_control_browsertest.cc
+++ b/chrome/browser/service/service_process_control_browsertest.cc
@@ -65,6 +65,29 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, LaunchAndIPC) {
   // And then shutdown the service process.
   EXPECT_TRUE(process()->Shutdown());
 }
+
+// This tests the case when a service process is launched when browser
+// starts but we try to launch it again in the remoting setup dialog.
+// TODO(hclam): We actually need to implement singleton in the service
+// process so that even if we launch the service process twice one
+// of the them will shutdown itself and we are still able to connect
+// to the first one that gets executed.
+IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, LaunchTwice) {
+  // Launch the service process the first time.
+  LaunchServiceProcessControl();
+
+  // Make sure we are connected to the service process.
+  EXPECT_TRUE(process()->is_connected());
+  SayHelloAndWait();
+
+  // Launch the service process again.
+  LaunchServiceProcessControl();
+  EXPECT_TRUE(process()->is_connected());
+  SayHelloAndWait();
+
+  // And then shutdown the service process.
+  EXPECT_TRUE(process()->Shutdown());
+}
 #endif
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ServiceProcessControlBrowserTest);