chromeos: Make CrosDBusService not be a singleton.

chromeos/dbus/services/cros_dbus_service.cc

Index: chromeos/dbus/services/cros_dbus_service.cc
diff --git a/chromeos/dbus/services/cros_dbus_service.cc b/chromeos/dbus/services/cros_dbus_service.cc
index 07cfc38beaee17d9e077fc000326d8ecb7e4f728..ee3770569b4d216b343c4e223082102ce1aaa1b2 100644
--- a/chromeos/dbus/services/cros_dbus_service.cc
+++ b/chromeos/dbus/services/cros_dbus_service.cc
@@ -8,66 +8,60 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/sys_info.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "dbus/bus.h"
 #include "dbus/exported_object.h"
 #include "dbus/object_path.h"
-#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
-namespace {
-
-CrosDBusService* g_cros_dbus_service = NULL;
-
-}  // namespace
-
 // The CrosDBusService implementation used in production, and unit tests.
 class CrosDBusServiceImpl : public CrosDBusService {
  public:
-  CrosDBusServiceImpl(dbus::Bus* bus, ServiceProviderList service_providers)
+  CrosDBusServiceImpl(dbus::Bus* bus,
+                      const std::string& service_name,
+                      const dbus::ObjectPath& object_path,
+                      ServiceProviderList service_providers)
       : service_started_(false),
         origin_thread_id_(base::PlatformThread::CurrentId()),
         bus_(bus),
-        service_providers_(std::move(service_providers)) {}
-
-  ~CrosDBusServiceImpl() override {
+        service_name_(service_name),
+        object_path_(object_path),
+        service_providers_(std::move(service_providers)) {
+    DCHECK(bus);
+    DCHECK(!service_name_.empty());
+    DCHECK(object_path_.IsValid());
   }
 
+  ~CrosDBusServiceImpl() override = default;
+
   // Starts the D-Bus service.
   void Start() {
     // Make sure we're running on the origin thread (i.e. the UI thread in
     // production).
     DCHECK(OnOriginThread());
-
-    // Return if the service has been already started.
-    if (service_started_)
-      return;
+    DCHECK(!service_started_);
 
     // There are some situations, described in http://crbug.com/234382#c27,
     // where processes on Linux can wind up stuck in an uninterruptible state
-    // for tens of seconds. If this happens when Chrome is trying to exit,
-    // this unkillable process can wind up clinging to ownership of
-    // kLibCrosServiceName while the system is trying to restart the browser.
-    // This leads to a fatal situation if we don't allow the new browser
-    // instance to replace the old as the owner of kLibCrosServiceName as seen
-    // in http://crbug.com/234382. Hence, REQUIRE_PRIMARY_ALLOW_REPLACEMENT.
-    bus_->RequestOwnership(kLibCrosServiceName,
-                           dbus::Bus::REQUIRE_PRIMARY_ALLOW_REPLACEMENT,
-                           base::Bind(&CrosDBusServiceImpl::OnOwnership,
-                                      base::Unretained(this)));
-
-    exported_object_ = bus_->GetExportedObject(
-        dbus::ObjectPath(kLibCrosServicePath));
-
+    // for tens of seconds. If this happens when Chrome is trying to exit, this
+    // unkillable process can wind up clinging to ownership of |service_name_|
+    // while the system is trying to restart the browser. This leads to a fatal
+    // situation if we don't allow the new browser instance to replace the old
+    // as the owner of |service_name_| as seen in http://crbug.com/234382.
+    // Hence, REQUIRE_PRIMARY_ALLOW_REPLACEMENT.
+    bus_->RequestOwnership(
+        service_name_, dbus::Bus::REQUIRE_PRIMARY_ALLOW_REPLACEMENT,
+        base::Bind(&CrosDBusServiceImpl::OnOwnership, base::Unretained(this)));
+
+    exported_object_ = bus_->GetExportedObject(object_path_);
     for (size_t i = 0; i < service_providers_.size(); ++i)
       service_providers_[i]->Start(exported_object_);
 
     service_started_ = true;
-
-    VLOG(1) << "CrosDBusServiceImpl started.";
   }
 
  private:
@@ -85,64 +79,56 @@ class CrosDBusServiceImpl : public CrosDBusService {
   bool service_started_;
   base::PlatformThreadId origin_thread_id_;
   dbus::Bus* bus_;
+  std::string service_name_;
+  dbus::ObjectPath object_path_;
   scoped_refptr<dbus::ExportedObject> exported_object_;
 
   // Service providers that form CrosDBusService.
   ServiceProviderList service_providers_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrosDBusServiceImpl);
 };
 
 // The stub CrosDBusService implementation used on Linux desktop,
 // which does nothing as of now.
 class CrosDBusServiceStubImpl : public CrosDBusService {
  public:
-  CrosDBusServiceStubImpl() {
-  }
+  CrosDBusServiceStubImpl() = default;
+  ~CrosDBusServiceStubImpl() override = default;
 
-  ~CrosDBusServiceStubImpl() override {}
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CrosDBusServiceStubImpl);
 };
 
 // static
-void CrosDBusService::Initialize(ServiceProviderList service_providers) {
-  if (g_cros_dbus_service) {
-    LOG(WARNING) << "CrosDBusService was already initialized";
-    return;
-  }
-  dbus::Bus* bus = DBusThreadManager::Get()->GetSystemBus();
-  if (base::SysInfo::IsRunningOnChromeOS() && bus) {
-    auto* service = new CrosDBusServiceImpl(bus, std::move(service_providers));
-    g_cros_dbus_service = service;
-    service->Start();
-  } else {
-    g_cros_dbus_service = new CrosDBusServiceStubImpl;
-  }
-  VLOG(1) << "CrosDBusService initialized";
+std::unique_ptr<CrosDBusService> CrosDBusService::Create(
+    const std::string& service_name,
+    const dbus::ObjectPath& object_path,
+    ServiceProviderList service_providers) {
+  if (DBusThreadManager::Get()->IsUsingFakes())
+    return base::MakeUnique<CrosDBusServiceStubImpl>();
+
+  return CreateRealImpl(DBusThreadManager::Get()->GetSystemBus(), service_name,
+                        object_path, std::move(service_providers));
 }
 
 // static
-void CrosDBusService::InitializeForTesting(
+std::unique_ptr<CrosDBusService> CrosDBusService::CreateRealImpl(
     dbus::Bus* bus,
+    const std::string& service_name,
+    const dbus::ObjectPath& object_path,
     ServiceProviderList service_providers) {
-  if (g_cros_dbus_service) {
-    LOG(WARNING) << "CrosDBusService was already initialized";
-    return;
-  }
-  auto* service = new CrosDBusServiceImpl(bus, std::move(service_providers));
+  auto service = base::MakeUnique<CrosDBusServiceImpl>(
+      bus, service_name, object_path, std::move(service_providers));
   service->Start();
-  g_cros_dbus_service = service;
-  VLOG(1) << "CrosDBusService initialized";
+  return std::move(service);
 }
 
-// static
-void CrosDBusService::Shutdown() {
-  delete g_cros_dbus_service;
-  g_cros_dbus_service = NULL;
-  VLOG(1) << "CrosDBusService Shutdown completed";
-}
+CrosDBusService::~CrosDBusService() = default;
 
-CrosDBusService::~CrosDBusService() {
-}
+CrosDBusService::CrosDBusService() = default;
 
-CrosDBusService::ServiceProviderInterface::~ServiceProviderInterface() {
-}
+CrosDBusService::ServiceProviderInterface::~ServiceProviderInterface() =
+    default;
 
 }  // namespace chromeos