chromeos: Introduce use_real_dbus_clients GN arg

use_real_dbus_clients=true will be used to avoid linking fakes to the
shipped binary.

BUG=952745
TEST=build

Change-Id: I419b9b5c2460dbc9dd66fff7490d06d34ec02c9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1571329Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652498}

ash/DEPS

@@ -61,6 +61,7 @@ include_rules = [
   "+chromeos/components/multidevice/logging/logging.h",
   "+chromeos/constants",
   # TODO(https://crbug.com/940810): Eliminate this.
+  "+chromeos/dbus/initialize_dbus_client.h",
   "+chromeos/dbus/audio",
   "+chromeos/dbus/hammerd",
   # TODO(https://crbug.com/644348): Eliminate this.

ash/ash_service.cc

@@ -18,6 +18,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "chromeos/audio/cras_audio_handler.h"
 #include "chromeos/dbus/hammerd/hammerd_client.h"
+#include "chromeos/dbus/initialize_dbus_client.h"
 #include "chromeos/dbus/power/power_policy_controller.h"
 #include "chromeos/dbus/shill/shill_clients.h"
 #include "chromeos/dbus/system_clock/system_clock_client.h"
@@ -158,23 +159,13 @@ void AshService::InitForMash() {
 void AshService::InitializeDBusClients() {
   dbus::Bus* bus = ash_dbus_helper_->bus();
 
-  if (bus) {
-    chromeos::CrasAudioClient::Initialize(bus);
-    chromeos::HammerdClient::Initialize(bus);
-    chromeos::PowerManagerClient::Initialize(bus);
-    chromeos::SystemClockClient::Initialize(bus);
-    chromeos::shill_clients::Initialize(bus);
-    // TODO(ortuno): Eliminate BluezDBusManager code from Ash, crbug.com/830893.
-    bluez::BluezDBusManager::Initialize(bus);
-  } else {
-    chromeos::CrasAudioClient::InitializeFake();
-    chromeos::HammerdClient::InitializeFake();
-    chromeos::PowerManagerClient::InitializeFake();
-    chromeos::SystemClockClient::InitializeFake();
-    chromeos::shill_clients::InitializeFakes();
-    // TODO(ortuno): Eliminate BluezDBusManager code from Ash, crbug.com/830893.
-    bluez::BluezDBusManager::InitializeFake();
-  }
+  chromeos::InitializeDBusClient<chromeos::CrasAudioClient>(bus);
+  chromeos::InitializeDBusClient<chromeos::HammerdClient>(bus);
+  chromeos::InitializeDBusClient<chromeos::PowerManagerClient>(bus);
+  chromeos::InitializeDBusClient<chromeos::SystemClockClient>(bus);
+  // TODO(ortuno): Eliminate BluezDBusManager code from Ash, crbug.com/830893.
+  chromeos::InitializeDBusClient<bluez::BluezDBusManager>(bus);
+  chromeos::shill_clients::Initialize(bus);
 
   // TODO(https://crbug.com/644336): Initialize real audio handler.
   chromeos::CrasAudioHandler::InitializeForTesting();

ash/shell.cc

@@ -175,6 +175,7 @@
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/trace_event/trace_event.h"
+#include "chromeos/dbus/initialize_dbus_client.h"
 #include "chromeos/dbus/power/power_policy_controller.h"
 #include "chromeos/system/devicemode.h"
 #include "components/exo/file_helper.h"
@@ -995,13 +996,9 @@ void Shell::Init(
   if (!::features::IsMultiProcessMash()) {
     // DBus clients only needed in Ash. For MultiProcessMash these are
     // initialized in AshService::InitializeDBusClients.
-    if (dbus_bus) {
-      // Required by DetachableBaseHandler.
-      chromeos::HammerdClient::Initialize(dbus_bus.get());
-    } else {
-      // Required by DetachableBaseHandler.
-      chromeos::HammerdClient::InitializeFake();
-    }
+
+    // Required by DetachableBaseHandler.
+    chromeos::InitializeDBusClient<chromeos::HammerdClient>(dbus_bus.get());
   }
 
   // This creates the MessageCenter object which is used by some other objects

chrome/browser/chromeos/dbus/dbus_helper.cc

@@ -16,6 +16,7 @@
 #include "chromeos/dbus/biod/biod_client.h"
 #include "chromeos/dbus/cups_proxy/cups_proxy_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/initialize_dbus_client.h"
 #include "chromeos/dbus/kerberos/kerberos_client.h"
 #include "chromeos/dbus/machine_learning/machine_learning_client.h"
 #include "chromeos/dbus/media_analytics/media_analytics_client.h"
@@ -54,40 +55,21 @@ void InitializeDBus() {
 
   // NOTE: base::Feature is not initialized yet, so any non MultiProcessMash
   // dbus client initialization for Ash should be done in Shell::Init.
-
-  if (bus) {
-    ArcCameraClient::Initialize(bus);
-    AuthPolicyClient::Initialize(bus);
-    BiodClient::Initialize(bus);  // For device::Fingerprint.
-    bluez::BluezDBusManager::Initialize(bus);
-    CrasAudioClient::Initialize(bus);
-    CryptohomeClient::Initialize(bus);
-    CupsProxyClient::Initialize(bus);
-    KerberosClient::Initialize(bus);
-    MachineLearningClient::Initialize(bus);
-    MediaAnalyticsClient::Initialize(bus);
-    PermissionBrokerClient::Initialize(bus);
-    PowerManagerClient::Initialize(bus);
-    SessionManagerClient::Initialize(bus);
-    SystemClockClient::Initialize(bus);
-    UpstartClient::Initialize(bus);
-  } else {
-    ArcCameraClient::InitializeFake();
-    AuthPolicyClient::InitializeFake();
-    BiodClient::InitializeFake();  // For device::Fingerprint.
-    bluez::BluezDBusManager::InitializeFake();
-    CrasAudioClient::InitializeFake();
-    CryptohomeClient::InitializeFake();
-    CupsProxyClient::InitializeFake();
-    KerberosClient::InitializeFake();
-    MachineLearningClient::InitializeFake();
-    MediaAnalyticsClient::InitializeFake();
-    PermissionBrokerClient::InitializeFake();
-    PowerManagerClient::InitializeFake();
-    SessionManagerClient::InitializeFake();
-    SystemClockClient::InitializeFake();
-    UpstartClient::InitializeFake();
-  }
+  InitializeDBusClient<ArcCameraClient>(bus);
+  InitializeDBusClient<AuthPolicyClient>(bus);
+  InitializeDBusClient<BiodClient>(bus);  // For device::Fingerprint.
+  InitializeDBusClient<bluez::BluezDBusManager>(bus);
+  InitializeDBusClient<CrasAudioClient>(bus);
+  InitializeDBusClient<CryptohomeClient>(bus);
+  InitializeDBusClient<CupsProxyClient>(bus);
+  InitializeDBusClient<KerberosClient>(bus);
+  InitializeDBusClient<MachineLearningClient>(bus);
+  InitializeDBusClient<MediaAnalyticsClient>(bus);
+  InitializeDBusClient<PermissionBrokerClient>(bus);
+  InitializeDBusClient<PowerManagerClient>(bus);
+  InitializeDBusClient<SessionManagerClient>(bus);
+  InitializeDBusClient<SystemClockClient>(bus);
+  InitializeDBusClient<UpstartClient>(bus);
 
   // Initialize the device settings service so that we'll take actions per
   // signals sent from the session manager. This needs to happen before

chromeos/dbus/BUILD.gn

@@ -7,6 +7,11 @@ import("//third_party/protobuf/proto_library.gni")
 
 assert(is_chromeos, "Non-Chrome-OS builds must not depend on //chromeos")
 
+declare_args() {
+  # Instantiate real D-Bus clients instead of fakes.
+  use_real_dbus_clients = false
+}
+
 component("dbus") {
   output_name = "chromeos_dbus"  # Avoid conflict with //dbus
   defines = [ "IS_CHROMEOS_DBUS_IMPL" ]
@@ -129,10 +134,17 @@ component("dbus") {
   ]
 }
 
+config("use_real_dbus_clients_config") {
+  if (use_real_dbus_clients) {
+    defines = [ "USE_REAL_DBUS_CLIENTS" ]
+  }
+}
+
 component("common") {
   output_name = "chromeos_dbus_common"
   defines = [ "IS_CHROMEOS_DBUS_IMPL" ]
 
+  all_dependent_configs = [ ":use_real_dbus_clients_config" ]
   public_deps = [
     "//chromeos/dbus/constants",
   ]
@@ -148,6 +160,7 @@ component("common") {
     "blocking_method_caller.h",
     "dbus_method_call_status.cc",
     "dbus_method_call_status.h",
+    "initialize_dbus_client.h",
     "pipe_reader.cc",
     "pipe_reader.h",
   ]

chromeos/dbus/dbus_clients_browser.cc

@@ -47,22 +47,29 @@
 
 namespace chromeos {
 
+// CREATE_DBUS_CLIENT creates the appropriate version of D-Bus client.
+#if defined(USE_REAL_DBUS_CLIENTS)
+// Create the real D-Bus client. use_real_clients is ignored.
+#define CREATE_DBUS_CLIENT(type, use_real_clients) type::Create()
+#else
+// Create a fake if use_real_clients == false.
+// TODO(hashimoto): Always use fakes after adding
+// use_real_dbus_clients=true to where needed. crbug.com/952745
+#define CREATE_DBUS_CLIENT(type, use_real_clients) \
+  (use_real_clients ? type::Create() : std::make_unique<Fake##type>())
+#endif  // USE_REAL_DBUS_CLIENTS
+
 DBusClientsBrowser::DBusClientsBrowser(bool use_real_clients) {
+  // TODO(hashimoto): Use CREATE_DBUS_CLIENT for all clients after removing
+  // DBusClientImplementationType and converting all Create() methods to return
+  // std::unique_ptr. crbug.com/952745
   const DBusClientImplementationType client_impl_type =
       use_real_clients ? REAL_DBUS_CLIENT_IMPLEMENTATION
                        : FAKE_DBUS_CLIENT_IMPLEMENTATION;
 
-  if (use_real_clients) {
-    arc_appfuse_provider_client_ = ArcAppfuseProviderClient::Create();
-  } else {
-    arc_appfuse_provider_client_ =
-        std::make_unique<FakeArcAppfuseProviderClient>();
-  }
-
-  if (use_real_clients)
-    arc_midis_client_ = ArcMidisClient::Create();
-  else
-    arc_midis_client_.reset(new FakeArcMidisClient);
+  arc_appfuse_provider_client_ =
+      CREATE_DBUS_CLIENT(ArcAppfuseProviderClient, use_real_clients);
+  arc_midis_client_ = CREATE_DBUS_CLIENT(ArcMidisClient, use_real_clients);
 
   if (use_real_clients)
     arc_obb_mounter_client_.reset(ArcObbMounterClient::Create());
@@ -78,10 +85,7 @@ DBusClientsBrowser::DBusClientsBrowser(bool use_real_clients) {
 
   cros_disks_client_.reset(CrosDisksClient::Create(client_impl_type));
 
-  if (use_real_clients)
-    cicerone_client_ = CiceroneClient::Create();
-  else
-    cicerone_client_ = std::make_unique<FakeCiceroneClient>();
+  cicerone_client_ = CREATE_DBUS_CLIENT(CiceroneClient, use_real_clients);
 
   if (use_real_clients)
     concierge_client_.reset(ConciergeClient::Create());
@@ -93,10 +97,8 @@ DBusClientsBrowser::DBusClientsBrowser(bool use_real_clients) {
   else
     debug_daemon_client_.reset(new FakeDebugDaemonClient);
 
-  if (use_real_clients)
-    diagnosticsd_client_ = DiagnosticsdClient::Create();
-  else
-    diagnosticsd_client_ = std::make_unique<FakeDiagnosticsdClient>();
+  diagnosticsd_client_ =
+      CREATE_DBUS_CLIENT(DiagnosticsdClient, use_real_clients);
 
   if (use_real_clients)
     easy_unlock_client_.reset(EasyUnlockClient::Create());
@@ -118,20 +120,11 @@ DBusClientsBrowser::DBusClientsBrowser(bool use_real_clients) {
   else
     lorgnette_manager_client_.reset(new FakeLorgnetteManagerClient);
 
-  if (use_real_clients)
-    oobe_configuration_client_ = OobeConfigurationClient::Create();
-  else
-    oobe_configuration_client_.reset(new FakeOobeConfigurationClient);
-
-  if (use_real_clients)
-    runtime_probe_client_ = RuntimeProbeClient::Create();
-  else
-    runtime_probe_client_ = std::make_unique<FakeRuntimeProbeClient>();
-
-  if (use_real_clients)
-    seneschal_client_ = SeneschalClient::Create();
-  else
-    seneschal_client_ = std::make_unique<FakeSeneschalClient>();
+  oobe_configuration_client_ =
+      CREATE_DBUS_CLIENT(OobeConfigurationClient, use_real_clients);
+  runtime_probe_client_ =
+      CREATE_DBUS_CLIENT(RuntimeProbeClient, use_real_clients);
+  seneschal_client_ = CREATE_DBUS_CLIENT(SeneschalClient, use_real_clients);
 
   if (use_real_clients)
     smb_provider_client_.reset(SmbProviderClient::Create());

chromeos/dbus/dbus_thread_manager.cc

@@ -245,10 +245,7 @@ void DBusThreadManager::InitializeClients() {
 
   // TODO(stevenjb): Move these to dbus_helper.cc in src/chrome and any tests
   // that require Shill clients. https://crbug.com/948390.
-  if (use_real_clients_)
-    shill_clients::Initialize(GetSystemBus());
-  else
-    shill_clients::InitializeFakes();
+  shill_clients::Initialize(GetSystemBus());
 
   if (clients_browser_)
     clients_browser_->Initialize(GetSystemBus());
@@ -271,9 +268,15 @@ void DBusThreadManager::Initialize(ClientSet client_set) {
     return;
 
   CHECK(!g_dbus_thread_manager);
+#if defined(USE_REAL_DBUS_CLIENTS)
+  bool use_real_clients = true;
+#else
+  // TODO(hashimoto): Always use fakes after adding
+  // use_real_dbus_clients=true to where needed. crbug.com/952745
   bool use_real_clients = base::SysInfo::IsRunningOnChromeOS() &&
                           !base::CommandLine::ForCurrentProcess()->HasSwitch(
                               chromeos::switches::kDbusStub);
+#endif
   g_dbus_thread_manager = new DBusThreadManager(client_set, use_real_clients);
   g_dbus_thread_manager->InitializeClients();
 }

chromeos/dbus/initialize_dbus_client.h

+// Copyright 2019 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 CHROMEOS_DBUS_INITIALIZE_DBUS_CLIENT_H_
+#define CHROMEOS_DBUS_INITIALIZE_DBUS_CLIENT_H_
+
+namespace dbus {
+class Bus;
+}  // namespace dbus
+
+namespace chromeos {
+
+// Initializes the appropriate version of D-Bus client.
+template <typename T>
+void InitializeDBusClient(dbus::Bus* bus) {
+#if defined(USE_REAL_DBUS_CLIENTS)
+  T::Initialize(bus);
+#else
+  // TODO(hashimoto): Always use fakes after adding
+  // use_real_dbus_clients=true to where needed. crbug.com/952745
+  if (bus) {
+    T::Initialize(bus);
+  } else {
+    T::InitializeFake();
+  }
+#endif
+}
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_DBUS_INITIALIZE_DBUS_CLIENT_H_

chromeos/dbus/shill/shill_clients.cc

@@ -18,6 +18,10 @@ namespace chromeos {
 namespace shill_clients {
 
 void Initialize(dbus::Bus* system_bus) {
+#if !defined(USE_REAL_DBUS_CLIENTS)
+  if (!system_bus)
+    return InitializeFakes();
+#endif
   DCHECK(system_bus);
   GsmSMSClient::Initialize(system_bus);
   ModemMessagingClient::Initialize(system_bus);