Explicitly initialize dbus in ash

While mash was in early development we supported a number of different
inititialization paths. That has since been simplified, we should now
explicitly initialize dbus from the correct places.

Bug: 647367
Change-Id: If5ec193ffe2a8db03bf1ddfe756815fb806d8b06
Reviewed-on: https://chromium-review.googlesource.com/c/1490400Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636182}

ash/ash_service.cc

@@ -18,6 +18,7 @@
 #include "chromeos/audio/cras_audio_handler.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_policy_controller.h"
+#include "chromeos/dbus/system_clock_client.h"
 #include "chromeos/network/network_connect.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/system/fake_statistics_provider.h"
@@ -83,14 +84,11 @@ AshService::~AshService() {
   chromeos::CrasAudioHandler::Shutdown();
   chromeos::NetworkConnect::Shutdown();
   network_connect_delegate_.reset();
-  // We may not have started the NetworkHandler.
-  if (network_handler_initialized_)
-    chromeos::NetworkHandler::Shutdown();
+  chromeos::NetworkHandler::Shutdown();
   device::BluetoothAdapterFactory::Shutdown();
   bluez::BluezDBusManager::Shutdown();
   chromeos::PowerPolicyController::Shutdown();
-  if (dbus_thread_manager_initialized_)
-    chromeos::DBusThreadManager::Shutdown();
+  chromeos::DBusThreadManager::Shutdown();
 
   // |gpu_host_| must be completely destroyed before Env as GpuHost depends on
   // Ozone, which Env owns.
@@ -125,25 +123,7 @@ void AshService::InitForMash() {
 
   // Must occur after mojo::ApplicationRunner has initialized AtExitManager, but
   // before WindowManager::Init(). Tests might initialize their own instance.
-  if (!chromeos::DBusThreadManager::IsInitialized()) {
-    chromeos::DBusThreadManager::Initialize(
-        chromeos::DBusThreadManager::kShared);
-    dbus_thread_manager_initialized_ = true;
-  }
-  chromeos::PowerPolicyController::Initialize(
-      chromeos::DBusThreadManager::Get()->GetPowerManagerClient());
-
-  // The initialization matches that in ChromeBrowserMainPartsChromeos.
-
-  bluez::BluezDBusManager::Initialize();
-  if (!chromeos::NetworkHandler::IsInitialized()) {
-    chromeos::NetworkHandler::Initialize();
-    network_handler_initialized_ = true;
-  }
-  network_connect_delegate_ = std::make_unique<NetworkConnectDelegateMus>();
-  chromeos::NetworkConnect::Initialize(network_connect_delegate_.get());
-  // TODO(jamescook): Initialize real audio handler.
-  chromeos::CrasAudioHandler::InitializeForTesting();
+  InitializeDBusClients();
 
   // TODO(jamescook): Refactor StatisticsProvider so we can get just the data
   // we need in ash. Right now StatisticsProviderImpl launches the crossystem
@@ -166,6 +146,32 @@ void AshService::InitForMash() {
   Shell::GetPrimaryRootWindow()->GetHost()->Show();
 }
 
+void AshService::InitializeDBusClients() {
+  CHECK(!chromeos::DBusThreadManager::IsInitialized());
+
+  // TODO(stevenjb): Eliminate use of DBusThreadManager and initialize
+  // dbus::Thread, dbus::Bus and required clients directly.
+  chromeos::DBusThreadManager::Initialize(chromeos::DBusThreadManager::kShared);
+  dbus::Bus* bus = chromeos::DBusThreadManager::Get()->GetSystemBus();
+  bool use_fakes = chromeos::DBusThreadManager::Get()->IsUsingFakes();
+  CHECK(bus || use_fakes);
+
+  chromeos::PowerPolicyController::Initialize(
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient());
+
+  // TODO(ortuno): Eliminate BluezDBusManager code from Ash, crbug.com/830893.
+  bluez::BluezDBusManager::Initialize();
+
+  // TODO(stevenjb): Eliminate NetworkHandler code from Ash, crbug.com/644355.
+  CHECK(!chromeos::NetworkHandler::IsInitialized());
+  chromeos::NetworkHandler::Initialize();
+  network_connect_delegate_ = std::make_unique<NetworkConnectDelegateMus>();
+  chromeos::NetworkConnect::Initialize(network_connect_delegate_.get());
+
+  // TODO(jamescook): Initialize real audio handler.
+  chromeos::CrasAudioHandler::InitializeForTesting();
+}
+
 void AshService::OnStart() {
   mojo_interface_factory::RegisterInterfaces(
       &registry_, base::ThreadTaskRunnerHandle::Get());

ash/ash_service.h

@@ -86,6 +86,8 @@ class ASH_EXPORT AshService : public service_manager::Service,
   // once the service starts (from OnStart()).
   void InitForMash();
 
+  void InitializeDBusClients();
+
   void BindServiceFactory(
       service_manager::mojom::ServiceFactoryRequest request);
 
@@ -123,12 +125,6 @@ class ASH_EXPORT AshService : public service_manager::Service,
 
   std::unique_ptr<ws::InputDeviceController> input_device_controller_;
 
-  // Whether this class initialized NetworkHandler and needs to clean it up.
-  bool network_handler_initialized_ = false;
-
-  // Whether this class initialized DBusThreadManager and needs to clean it up.
-  bool dbus_thread_manager_initialized_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(AshService);
 };
 

ash/dbus/ash_dbus_services.cc

@@ -17,14 +17,8 @@
 namespace ash {
 
 AshDBusServices::AshDBusServices() {
-  // TODO(stevenjb): Figure out where else the D-Bus thread is getting
-  // initialized and then always init it here when we have the MASH
-  // config after the contention is sorted out.
-  if (!chromeos::DBusThreadManager::IsInitialized()) {
-    chromeos::DBusThreadManager::Initialize(
-        chromeos::DBusThreadManager::kShared);
-    initialized_dbus_thread_ = true;
-  }
+  // DBusThreadManager is initialized in Chrome or in AshService::InitForMash().
+  CHECK(chromeos::DBusThreadManager::IsInitialized());
 
   dbus::Bus* system_bus =
       chromeos::DBusThreadManager::Get()->IsUsingFakes()
@@ -57,9 +51,6 @@ AshDBusServices::~AshDBusServices() {
   display_service_.reset();
   liveness_service_.reset();
   url_handler_service_.reset();
-  if (initialized_dbus_thread_) {
-    chromeos::DBusThreadManager::Shutdown();
-  }
 }
 
 }  // namespace ash

ash/dbus/ash_dbus_services.h

@@ -27,7 +27,6 @@ class AshDBusServices {
   void EmitAshInitialized();
 
  private:
-  bool initialized_dbus_thread_{false};
   std::unique_ptr<chromeos::CrosDBusService> display_service_;
   std::unique_ptr<chromeos::CrosDBusService> liveness_service_;
   std::unique_ptr<chromeos::CrosDBusService> url_handler_service_;