The ServiceProcessControl class is now a singleton and is not keyed off the profile.

BUG=80170
TEST=Unit-tests, Cloud Print proxy.


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

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

10 files changed, 47 insertions, 151 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 938d434..92ea2dd 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -68,7 +68,6 @@
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/service/service_process_control.h"
-#include "chrome/browser/service/service_process_control_manager.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/translate/translate_manager.h"
 #include "chrome/browser/ui/browser.h"
diff --git a/chrome/browser/browser_shutdown.cc b/chrome/browser/browser_shutdown.cc
index 81e4e28..0ab6675 100644
--- a/chrome/browser/browser_shutdown.cc
+++ b/chrome/browser/browser_shutdown.cc
@@ -28,7 +28,7 @@
 #include "chrome/browser/plugin_updater.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/service/service_process_control_manager.h"
+#include "chrome/browser/service/service_process_control.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/common/chrome_paths.h"
@@ -124,8 +124,8 @@ void Shutdown() {
   // shutdown.
   base::ThreadRestrictions::SetIOAllowed(true);
 
-  // Shutdown all IPC channels to service processes.
-  ServiceProcessControlManager::GetInstance()->Shutdown();
+  // Shutdown the IPC channel to the service processes.
+  ServiceProcessControl::GetInstance()->Disconnect();
 
 #if defined(OS_CHROMEOS)
   // The system key event listener needs to be shut down earlier than when
diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc
index 6962926..86dc073 100644
--- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc
+++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc
@@ -16,7 +16,6 @@
 #include "chrome/browser/printing/cloud_print/cloud_print_setup_flow.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/service/service_process_control.h"
-#include "chrome/browser/service/service_process_control_manager.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/cloud_print/cloud_print_proxy_info.h"
 #include "chrome/common/pref_names.h"
@@ -158,8 +157,7 @@ void CloudPrintProxyService::OnCloudPrintSetupClosed() {
 
 void CloudPrintProxyService::RefreshCloudPrintProxyStatus() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  ServiceProcessControl* process_control =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(profile_);
+  ServiceProcessControl* process_control = ServiceProcessControl::GetInstance();
   DCHECK(process_control->is_connected());
   ServiceProcessControl::CloudPrintProxyInfoHandler* callback =
        NewCallback(this, &CloudPrintProxyService::ProxyInfoCallback);
@@ -169,8 +167,7 @@ void CloudPrintProxyService::RefreshCloudPrintProxyStatus() {
 
 void CloudPrintProxyService::EnableCloudPrintProxy(const std::string& lsid,
                                                    const std::string& email) {
-  ServiceProcessControl* process_control =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(profile_);
+  ServiceProcessControl* process_control = ServiceProcessControl::GetInstance();
   DCHECK(process_control->is_connected());
   process_control->Send(new ServiceMsg_EnableCloudPrintProxy(lsid));
   // Assume the IPC worked.
@@ -181,8 +178,7 @@ void CloudPrintProxyService::EnableCloudPrintProxyWithRobot(
     const std::string& robot_auth_code,
     const std::string& robot_email,
     const std::string& user_email) {
-  ServiceProcessControl* process_control =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(profile_);
+  ServiceProcessControl* process_control = ServiceProcessControl::GetInstance();
   DCHECK(process_control->is_connected());
   process_control->Send(new ServiceMsg_EnableCloudPrintProxyWithRobot(
       robot_auth_code,
@@ -194,8 +190,7 @@ void CloudPrintProxyService::EnableCloudPrintProxyWithRobot(
 
 
 void CloudPrintProxyService::DisableCloudPrintProxy() {
-  ServiceProcessControl* process_control =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(profile_);
+  ServiceProcessControl* process_control = ServiceProcessControl::GetInstance();
   DCHECK(process_control->is_connected());
   process_control->Send(new ServiceMsg_DisableCloudPrintProxy);
   // Assume the IPC worked.
@@ -210,10 +205,6 @@ void CloudPrintProxyService::ProxyInfoCallback(
 }
 
 bool CloudPrintProxyService::InvokeServiceTask(Task* task) {
-  ServiceProcessControl* process_control =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(profile_);
-  DCHECK(process_control);
-  if (process_control)
-    process_control->Launch(task, NULL);
-  return !!process_control;
+  ServiceProcessControl::GetInstance()->Launch(task, NULL);
+  return true;
 }
diff --git a/chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc b/chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc
index 20763b5..6f1fd1e 100644
--- a/chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc
+++ b/chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc
@@ -17,7 +17,6 @@
 #include "chrome/browser/printing/cloud_print/cloud_print_url.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/service/service_process_control.h"
-#include "chrome/browser/service/service_process_control_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_list.h"
diff --git a/chrome/browser/service/service_process_control.cc b/chrome/browser/service/service_process_control.cc
index 5c07223..959c005 100644
--- a/chrome/browser/service/service_process_control.cc
+++ b/chrome/browser/service/service_process_control.cc
@@ -22,8 +22,7 @@
 
 
 // ServiceProcessControl implementation.
-ServiceProcessControl::ServiceProcessControl(Profile* profile)
-    : profile_(profile) {
+ServiceProcessControl::ServiceProcessControl() {
 }
 
 ServiceProcessControl::~ServiceProcessControl() {
@@ -166,6 +165,11 @@ void ServiceProcessControl::Launch(Task* success_task, Task* failure_task) {
       NewRunnableMethod(this, &ServiceProcessControl::OnProcessLaunched));
 }
 
+void ServiceProcessControl::Disconnect() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  channel_.reset();
+}
+
 void ServiceProcessControl::OnProcessLaunched() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (launcher_->launched()) {
@@ -241,17 +245,24 @@ void ServiceProcessControl::OnCloudPrintProxyInfo(
 
 bool ServiceProcessControl::GetCloudPrintProxyInfo(
     CloudPrintProxyInfoHandler* cloud_print_info_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(cloud_print_info_callback);
   cloud_print_info_callback_.reset(cloud_print_info_callback);
   return Send(new ServiceMsg_GetCloudPrintProxyInfo());
 }
 
 bool ServiceProcessControl::Shutdown() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   bool ret = Send(new ServiceMsg_Shutdown());
   channel_.reset();
   return ret;
 }
 
+// static
+ServiceProcessControl* ServiceProcessControl::GetInstance() {
+  return Singleton<ServiceProcessControl>::get();
+}
+
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ServiceProcessControl);
 
 ServiceProcessControl::Launcher::Launcher(ServiceProcessControl* process,
diff --git a/chrome/browser/service/service_process_control.h b/chrome/browser/service/service_process_control.h
index 10e9aa0..b849550b 100644
--- a/chrome/browser/service/service_process_control.h
+++ b/chrome/browser/service/service_process_control.h
@@ -14,13 +14,13 @@
 #include "base/callback_old.h"
 #include "base/id_map.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/singleton.h"
 #include "base/process.h"
 #include "base/task.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
 #include "ipc/ipc_sync_channel.h"
 
-class Profile;
 class CommandLine;
 
 namespace cloud_print {
@@ -46,13 +46,8 @@ class ServiceProcessControl : public IPC::Channel::Sender,
   typedef Callback1<const cloud_print::CloudPrintProxyInfo&>::Type
       CloudPrintProxyInfoHandler;
 
-  // Construct a ServiceProcessControl with |profile|..
-  explicit ServiceProcessControl(Profile* profile);
-  virtual ~ServiceProcessControl();
-
-  // Return the user profile associated with this service process.
-  Profile* profile() const { return profile_; }
-
+  // Returns the singleton instance of this class.
+  static ServiceProcessControl* GetInstance();
   // Return true if this object is connected to the service.
   bool is_connected() const { return channel_.get() != NULL; }
 
@@ -69,6 +64,8 @@ class ServiceProcessControl : public IPC::Channel::Sender,
   // |success_task| can be invoked in the context of the Launch call.
   // Takes ownership of |success_task| and |failure_task|.
   void Launch(Task* success_task, Task* failure_task);
+  // Disconnect the IPC channel from the service process.
+  void Disconnect();
 
   // IPC::Channel::Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message);
@@ -128,6 +125,11 @@ class ServiceProcessControl : public IPC::Channel::Sender,
     uint32 retry_count_;
   };
 
+  ServiceProcessControl();
+  virtual ~ServiceProcessControl();
+
+  friend struct DefaultSingletonTraits<ServiceProcessControl>;
+
   typedef std::vector<Task*> TaskList;
 
   // Helper method to invoke all the callbacks based on success on failure.
@@ -141,8 +143,6 @@ class ServiceProcessControl : public IPC::Channel::Sender,
 
   static void RunAllTasksHelper(TaskList* task_list);
 
-  Profile* profile_;
-
   // IPC channel to the service process.
   scoped_ptr<IPC::SyncChannel> channel_;
 
diff --git a/chrome/browser/service/service_process_control_browsertest.cc b/chrome/browser/service/service_process_control_browsertest.cc
index 96ad827..9f0abd1 100644
--- a/chrome/browser/service/service_process_control_browsertest.cc
+++ b/chrome/browser/service/service_process_control_browsertest.cc
@@ -5,7 +5,6 @@
 #include "base/process_util.h"
 #include "base/test/test_timeouts.h"
 #include "chrome/browser/service/service_process_control.h"
-#include "chrome/browser/service/service_process_control_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/service_process_util.h"
@@ -21,8 +20,6 @@ class ServiceProcessControlBrowserTest
   ~ServiceProcessControlBrowserTest() {
     base::CloseProcessHandle(service_process_handle_);
     service_process_handle_ = base::kNullProcessHandle;
-    // Delete all instances of ServiceProcessControl.
-    ServiceProcessControlManager::GetInstance()->Shutdown();
   }
 
 #if defined(OS_MACOSX)
@@ -34,13 +31,8 @@ class ServiceProcessControlBrowserTest
 
  protected:
   void LaunchServiceProcessControl() {
-    ServiceProcessControl* process =
-        ServiceProcessControlManager::GetInstance()->GetProcessControl(
-            browser()->profile());
-    process_ = process;
-
     // Launch the process asynchronously.
-    process->Launch(
+    ServiceProcessControl::GetInstance()->Launch(
         NewRunnableMethod(
             this,
             &ServiceProcessControlBrowserTest::ProcessControlLaunched),
@@ -54,7 +46,7 @@ class ServiceProcessControlBrowserTest
 
   // Send a Cloud Print status request and wait for a reply from the service.
   void SendRequestAndWait() {
-    process()->GetCloudPrintProxyInfo(NewCallback(
+    ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo(NewCallback(
         this, &ServiceProcessControlBrowserTest::CloudPrintInfoCallback));
     ui_test_utils::RunMessageLoop();
   }
@@ -65,10 +57,8 @@ class ServiceProcessControlBrowserTest
   }
 
   void Disconnect() {
-    // This will delete all instances of ServiceProcessControl and close the IPC
-    // connections.
-    ServiceProcessControlManager::GetInstance()->Shutdown();
-    process_ = NULL;
+    // This will close the IPC connection.
+    ServiceProcessControl::GetInstance()->Disconnect();
   }
 
   void WaitForShutdown() {
@@ -97,10 +87,7 @@ class ServiceProcessControlBrowserTest
     MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask());
   }
 
-  ServiceProcessControl* process() { return process_; }
-
  private:
-  ServiceProcessControl* process_;
   base::ProcessHandle service_process_handle_;
 };
 
@@ -112,11 +99,11 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest,
   LaunchServiceProcessControl();
 
   // Make sure we are connected to the service process.
-  EXPECT_TRUE(process()->is_connected());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->is_connected());
   SendRequestAndWait();
 
   // And then shutdown the service process.
-  EXPECT_TRUE(process()->Shutdown());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->Shutdown());
 }
 
 // This tests the case when a service process is launched when the browser
@@ -132,16 +119,16 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, MAYBE_LaunchTwice) {
   LaunchServiceProcessControl();
 
   // Make sure we are connected to the service process.
-  EXPECT_TRUE(process()->is_connected());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->is_connected());
   SendRequestAndWait();
 
   // Launch the service process again.
   LaunchServiceProcessControl();
-  EXPECT_TRUE(process()->is_connected());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->is_connected());
   SendRequestAndWait();
 
   // And then shutdown the service process.
-  EXPECT_TRUE(process()->Shutdown());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->Shutdown());
 }
 
 static void DecrementUntilZero(int* count) {
@@ -160,9 +147,7 @@ static void DecrementUntilZero(int* count) {
 #endif
 IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest,
                        MAYBE_MultipleLaunchTasks) {
-  ServiceProcessControl* process =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(
-          browser()->profile());
+  ServiceProcessControl* process = ServiceProcessControl::GetInstance();
   int launch_count = 5;
   for (int i = 0; i < launch_count; i++) {
     // Launch the process asynchronously.
@@ -185,9 +170,7 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest,
 #define MAYBE_SameLaunchTask SameLaunchTask
 #endif
 IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, MAYBE_SameLaunchTask) {
-  ServiceProcessControl* process =
-      ServiceProcessControlManager::GetInstance()->GetProcessControl(
-          browser()->profile());
+  ServiceProcessControl* process = ServiceProcessControl::GetInstance();
   int launch_count = 5;
   for (int i = 0; i < launch_count; i++) {
     // Launch the process asynchronously.
@@ -214,7 +197,7 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest,
   // Launch the service process.
   LaunchServiceProcessControl();
   // Make sure we are connected to the service process.
-  EXPECT_TRUE(process()->is_connected());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->is_connected());
   Disconnect();
   WaitForShutdown();
 }
@@ -225,7 +208,7 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest,
   // Launch the service process.
   LaunchServiceProcessControl();
   // Make sure we are connected to the service process.
-  EXPECT_TRUE(process()->is_connected());
+  EXPECT_TRUE(ServiceProcessControl::GetInstance()->is_connected());
   base::ProcessId service_pid;
   EXPECT_TRUE(GetServiceProcessData(NULL, &service_pid));
   EXPECT_NE(static_cast<base::ProcessId>(0), service_pid);
@@ -247,6 +230,8 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, MAYBE_CheckPid) {
   LaunchServiceProcessControl();
   EXPECT_TRUE(GetServiceProcessData(NULL, &service_pid));
   EXPECT_NE(static_cast<base::ProcessId>(0), service_pid);
+  // Disconnect from service process.
+  ServiceProcessControl::GetInstance()->Disconnect();
 }
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ServiceProcessControlBrowserTest);
diff --git a/chrome/browser/service/service_process_control_manager.cc b/chrome/browser/service/service_process_control_manager.cc
deleted file mode 100644
index 1bb71a6..0000000
--- a/chrome/browser/service/service_process_control_manager.cc
+++ /dev/null
@@ -1,47 +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/browser/service/service_process_control_manager.h"
-
-#include "base/memory/singleton.h"
-#include "base/stl_util-inl.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/service/service_process_control.h"
-#include "content/browser/browser_thread.h"
-
-ServiceProcessControlManager::ServiceProcessControlManager() {
-}
-
-ServiceProcessControlManager::~ServiceProcessControlManager() {
-  DCHECK(process_control_list_.empty());
-}
-
-ServiceProcessControl* ServiceProcessControlManager::GetProcessControl(
-    Profile* profile) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  // TODO(hclam): We will have different service process for different types of
-  // service, but now we only create a new process for a different profile.
-  for (ServiceProcessControlList::iterator i = process_control_list_.begin();
-       i != process_control_list_.end(); ++i) {
-    if ((*i)->profile() == profile)
-      return *i;
-  }
-
-  // Couldn't find a ServiceProcess so construct a new one.
-  ServiceProcessControl* process  = new ServiceProcessControl(profile);
-  process_control_list_.push_back(process);
-  return process;
-}
-
-void ServiceProcessControlManager::Shutdown() {
-  // When we shutdown the manager we just clear the list and don't actually
-  // shutdown the service process.
-  STLDeleteElements(&process_control_list_);
-}
-
-// static
-ServiceProcessControlManager* ServiceProcessControlManager::GetInstance() {
-  return Singleton<ServiceProcessControlManager>::get();
-}
diff --git a/chrome/browser/service/service_process_control_manager.h b/chrome/browser/service/service_process_control_manager.h
deleted file mode 100644
index a27ec03..0000000
--- a/chrome/browser/service/service_process_control_manager.h
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright (c) 2010 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_BROWSER_SERVICE_SERVICE_PROCESS_CONTROL_MANAGER_H_
-#define CHROME_BROWSER_SERVICE_SERVICE_PROCESS_CONTROL_MANAGER_H_
-
-#include <vector>
-
-class Profile;
-class ServiceProcessControl;
-
-// ServiceProcessControlManager is a registrar for all ServiceProcess created
-// in the browser process. It is also a factory for creating new
-// ServiceProcess.
-class ServiceProcessControlManager {
- public:
-  typedef std::vector<ServiceProcessControl*> ServiceProcessControlList;
-
-  ServiceProcessControlManager();
-  ~ServiceProcessControlManager();
-
-  // Get the ServiceProcess instance corresponding to |profile| and |type|.
-  // If such an instance doesn't exist a new instance is created.
-  //
-  // There will be at most one ServiceProcess for a |profile|.
-  //
-  // This method should only be accessed on the UI thread.
-  ServiceProcessControl* GetProcessControl(Profile* profile);
-
-  // Destroy all ServiceProcess objects created.
-  void Shutdown();
-
-  // Return the instance of ServiceProcessControlManager.
-  static ServiceProcessControlManager* GetInstance();
-
- private:
-  ServiceProcessControlList process_control_list_;
-};
-
-#endif  // CHROME_BROWSER_SERVICE_SERVICE_PROCESS_CONTROL_MANAGER_H_
diff --git a/chrome/browser/ui/webui/options/advanced_options_handler.cc b/chrome/browser/ui/webui/options/advanced_options_handler.cc
index 535663a..c85e197a 100644
--- a/chrome/browser/ui/webui/options/advanced_options_handler.cc
+++ b/chrome/browser/ui/webui/options/advanced_options_handler.cc
@@ -21,7 +21,6 @@
 #include "chrome/browser/printing/cloud_print/cloud_print_url.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/service/service_process_control.h"
-#include "chrome/browser/service/service_process_control_manager.h"
 #include "chrome/browser/ui/options/options_util.h"
 #include "chrome/browser/ui/webui/options/options_managed_banner_handler.h"
 #include "chrome/common/chrome_switches.h"