Stop Initializing DBusThreadManager in GetSetterForTesting()

Instead, initialize the manager in each test. This simplifies
the manager implementation.

Also, with this CL, DBusThreadManagerSetter no longer depends
on DBusThreadManager. Instead, DBusThreadManager calls into
DBusThreadManagerSetter when it exists (i.e. in tests.)

This is necessary because we still have to allow browser tests
to call DBusThreadManager::GetSetterForTesting() in
SetUpInProcessBrowserTestFixture() before DBusThreadManager is
automatically initialized in chromeos::InitializeDBus() (which
is part of the browser process startup.) Once the browser process
startup code runs, some browser code holds raw pointers to DBus
clients e.g. DebugDaemonClient. Because of this, calling e.g.
GetSetterForTesting()->SetDebugDaemonClient(...) AFTER browser
startup (in SetUpOnMainThread()) is no-op. The reversed dependency
allows browser tests to call GetSetterForTesting() in
SetUpInProcessBrowserTestFixture().

BUG=None
TEST=try
TEST=ASAN_OPTIONS=detect_odr_violation=0 ./testing/xvfb.py \
  ./out/asan/browser_tests --gtest_filter='WizardController*'

Change-Id: Ic8bd619d05f3db7eee317fcd655be3aec12935ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2896604
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#884490}

chrome/browser/ash/login/enable_debugging_browsertest.cc

@@ -189,10 +189,8 @@ class EnableDebuggingTestBase : public OobeBaseTest {
         chromeos::switches::kDisableHIDDetectionOnOOBEForTesting);
   }
   void SetUpInProcessBrowserTestFixture() override {
-    std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
     debug_daemon_client_ = new TestDebugDaemonClient;
-    dbus_setter->SetDebugDaemonClient(
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetDebugDaemonClient(
         std::unique_ptr<DebugDaemonClient>(debug_daemon_client_));
 
     OobeBaseTest::SetUpInProcessBrowserTestFixture();
@@ -373,9 +371,7 @@ class EnableDebuggingNonDevTest : public EnableDebuggingTestBase {
   EnableDebuggingNonDevTest() = default;
 
   void SetUpInProcessBrowserTestFixture() override {
-    std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
-    dbus_setter->SetDebugDaemonClient(
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetDebugDaemonClient(
         std::unique_ptr<DebugDaemonClient>(new FakeDebugDaemonClient));
     EnableDebuggingTestBase::SetUpInProcessBrowserTestFixture();
   }

chrome/browser/ash/login/screens/update_required_screen_unittest.cc

@@ -60,6 +60,7 @@ class UpdateRequiredScreenUnitTest : public testing::Test {
     wizard_context_ = std::make_unique<WizardContext>();
     fake_view_ = std::make_unique<FakeUpdateRequiredScreenHandler>();
     fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::unique_ptr<UpdateEngineClient>(fake_update_engine_client_));
 

chrome/browser/ash/login/screens/update_screen_unittest.cc

@@ -74,6 +74,7 @@ class UpdateScreenUnitTest : public testing::Test {
     wizard_context_ = std::make_unique<WizardContext>();
     PowerManagerClient::InitializeFake();
     fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::unique_ptr<UpdateEngineClient>(fake_update_engine_client_));
     network_handler_test_helper_ = std::make_unique<NetworkHandlerTestHelper>();

chrome/browser/ash/login/test/oobe_base_test.cc

@@ -148,10 +148,8 @@ void OobeBaseTest::SetUpInProcessBrowserTestFixture() {
   // based on timer. It is nice simulation for chromeos-on-linux, but
   // may lead to flakiness in debug/*SAN tests.
   // Set up FakeUpdateEngineClient that does not have any timer-based logic.
-  std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
-      chromeos::DBusThreadManager::GetSetterForTesting();
   update_engine_client_ = new FakeUpdateEngineClient;
-  dbus_setter->SetUpdateEngineClient(
+  chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
       std::unique_ptr<UpdateEngineClient>(update_engine_client_));
 }
 

chrome/browser/ash/login/version_updater/version_updater_unittest.cc

@@ -90,6 +90,7 @@ class VersionUpdaterUnitTest : public testing::Test {
   void SetUp() override {
     // Initialize objects needed by VersionUpdater.
     fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::unique_ptr<UpdateEngineClient>(fake_update_engine_client_));
 

chrome/browser/ash/login/wizard_controller_browsertest.cc

@@ -1069,16 +1069,6 @@ class WizardControllerDeviceStateTest : public WizardControllerFlowTest {
   }
 
   // WizardControllerFlowTest:
-  void SetUpInProcessBrowserTestFixture() override {
-    WizardControllerFlowTest::SetUpInProcessBrowserTestFixture();
-
-    // We need to initialize some dbus clients here, otherwise this test will
-    // timeout in WaitForAutoEnrollmentState on asan builds. TODO(stevenjb):
-    // Determine which client(s) need to be created and extract and initialize
-    // them. https://crbug.com/949063.
-    DBusThreadManager::Initialize();
-  }
-
   void SetUpOnMainThread() override {
     WizardControllerFlowTest::SetUpOnMainThread();
 

chrome/browser/ash/notifications/gnubby_notification_unittest.cc

@@ -23,6 +23,7 @@ class GnubbyNotificationTest : public BrowserWithTestWindowTest {
   ~GnubbyNotificationTest() override {}
 
   void SetUp() override {
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetGnubbyClient(
         std::unique_ptr<chromeos::GnubbyClient>(
             new chromeos::FakeGnubbyClient));

chrome/browser/ash/notifications/update_required_notification_unittest.cc

@@ -128,6 +128,7 @@ void UpdateRequiredNotificationTest::SetUp() {
   auto fake_update_engine_client =
       std::make_unique<chromeos::FakeUpdateEngineClient>();
   fake_update_engine_client_ = fake_update_engine_client.get();
+  chromeos::DBusThreadManager::Initialize();
   chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
       std::move(fake_update_engine_client));
   network_handler_test_helper_ =

chrome/browser/ash/system/automatic_reboot_manager_unittest.cc

@@ -333,6 +333,7 @@ void AutomaticRebootManagerBasicTest::SetUp() {
   AutomaticRebootManager::RegisterPrefs(local_state_.registry());
 
   update_engine_client_ = new FakeUpdateEngineClient;
+  DBusThreadManager::Initialize();
   DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
       std::unique_ptr<UpdateEngineClient>(update_engine_client_));
   PowerManagerClient::InitializeFake();

chrome/browser/ash/system_logs/single_debug_daemon_log_source_unittest.cc

@@ -26,6 +26,7 @@ class SingleDebugDaemonLogSourceTest : public ::testing::Test {
   void SetUp() override {
     // Since no debug daemon will be available during a unit test, use
     // FakeDebugDaemonClient to provide dummy DebugDaemonClient functionality.
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetDebugDaemonClient(
         std::make_unique<chromeos::FakeDebugDaemonClient>());
   }

chrome/browser/chromeos/eol_notification_unittest.cc

@@ -32,6 +32,7 @@ class EolNotificationTest : public BrowserWithTestWindowTest {
 
   void SetUp() override {
     fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         base::WrapUnique<UpdateEngineClient>(fake_update_engine_client_));
     chromeos::ConciergeClient::InitializeFake(/*fake_cicerone_client=*/nullptr);

chrome/browser/chromeos/policy/device_policy_cros_browser_test.h

@@ -116,9 +116,6 @@ class DevicePolicyCrosBrowserTest : public MixinBasedInProcessBrowserTest {
   ~DevicePolicyCrosBrowserTest() override;
 
   void RefreshDevicePolicy() { policy_helper()->RefreshDevicePolicy(); }
-  chromeos::DBusThreadManagerSetter* dbus_setter() {
-    return dbus_setter_.get();
-  }
 
   DevicePolicyBuilder* device_policy() {
     return policy_helper()->device_policy();
@@ -134,9 +131,6 @@ class DevicePolicyCrosBrowserTest : public MixinBasedInProcessBrowserTest {
 
  private:
   DevicePolicyCrosTestHelper policy_helper_;
-
-  // FakeDBusThreadManager uses FakeSessionManagerClient.
-  std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter_;
 };
 
 }  // namespace policy

chrome/browser/chromeos/policy/minimum_version_policy_handler_unittest.cc

@@ -113,6 +113,7 @@ void MinimumVersionPolicyHandlerTest::SetUp() {
   auto fake_update_engine_client =
       std::make_unique<chromeos::FakeUpdateEngineClient>();
   fake_update_engine_client_ = fake_update_engine_client.get();
+  chromeos::DBusThreadManager::Initialize();
   chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
       std::move(fake_update_engine_client));
   network_handler_test_helper_ =

chrome/browser/chromeos/policy/scheduled_update_checker/device_scheduled_update_checker_unittest.cc

@@ -315,6 +315,7 @@ class DeviceScheduledUpdateCheckerTest : public testing::Test {
     auto fake_update_engine_client =
         std::make_unique<chromeos::FakeUpdateEngineClient>();
     fake_update_engine_client_ = fake_update_engine_client.get();
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::move(fake_update_engine_client));
 

chrome/browser/chromeos/policy/status_collector/child_status_collector_browsertest.cc

@@ -205,9 +205,8 @@ class ChildStatusCollectorTest : public testing::Test {
     TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_);
 
     // Use FakeUpdateEngineClient.
-    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
-    dbus_setter->SetUpdateEngineClient(
+    chromeos::DBusThreadManager::Initialize();
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         base::WrapUnique<chromeos::UpdateEngineClient>(update_engine_client_));
 
     chromeos::CiceroneClient::InitializeFake();

chrome/browser/chromeos/policy/status_collector/device_status_collector_browsertest.cc

@@ -849,9 +849,8 @@ class DeviceStatusCollectorTest : public testing::Test {
     chromeos::KioskCryptohomeRemover::RegisterPrefs(local_state_.registry());
 
     // Use FakeUpdateEngineClient.
-    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
-    dbus_setter->SetUpdateEngineClient(
+    chromeos::DBusThreadManager::Initialize();
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         base::WrapUnique<chromeos::UpdateEngineClient>(update_engine_client_));
 
     chromeos::CrasAudioHandler::InitializeForTesting();

chrome/browser/chromeos/smb_client/smb_file_system_unittest.cc

@@ -67,6 +67,7 @@ class SmbFileSystemTest : public testing::Test {
     // The mock needs to be marked as leaked because ownership is passed to
     // DBusThreadManager.
     testing::Mock::AllowLeak(mock_client.get());
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetSmbProviderClient(
         std::move(mock_client));
   }

chrome/browser/chromeos/smb_client/smb_service_unittest.cc

@@ -171,6 +171,7 @@ class SmbServiceWithSmbfsTest : public testing::Test {
         std::move(user_manager_temp));
 
     // This isn't used, but still needs to exist.
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetSmbProviderClient(
         std::make_unique<FakeSmbProviderClient>());
     chromeos::ConciergeClient::InitializeFake(/*fake_cicerone_client=*/nullptr);

chrome/browser/component_updater/cros_component_installer_chromeos_unittest.cc

@@ -207,6 +207,7 @@ class CrOSComponentInstallerTest : public testing::Test {
     auto fake_image_loader_client =
         std::make_unique<chromeos::FakeImageLoaderClient>();
     image_loader_client_ = fake_image_loader_client.get();
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetImageLoaderClient(
         std::move(fake_image_loader_client));
   }

chrome/browser/extensions/api/image_writer_private/test_utils.cc

@@ -243,12 +243,15 @@ void ImageWriterTestUtils::SetUp(bool is_browser_test) {
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (!chromeos::DBusThreadManager::IsInitialized()) {
-    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
-    chromeos::ConciergeClient::InitializeFake(/*fake_cicerone_client=*/nullptr);
-    std::unique_ptr<chromeos::ImageBurnerClient> image_burner_fake(
-        new ImageWriterFakeImageBurnerClient());
-    dbus_setter->SetImageBurnerClient(std::move(image_burner_fake));
+    if (!is_browser_test) {
+      // For browser tests, chromeos::InitializeDBus() automatically does the
+      // same.
+      chromeos::DBusThreadManager::Initialize();
+      chromeos::ConciergeClient::InitializeFake(
+          /*fake_cicerone_client=*/nullptr);
+    }
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetImageBurnerClient(
+        std::make_unique<ImageWriterFakeImageBurnerClient>());
   }
 
   FakeDiskMountManager* disk_manager = new FakeDiskMountManager();
@@ -271,6 +274,8 @@ void ImageWriterTestUtils::SetUp(bool is_browser_test) {
 void ImageWriterTestUtils::TearDown() {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (chromeos::DBusThreadManager::IsInitialized()) {
+    // When in browser_tests, this path is not taken. These clients have already
+    // been shut down by chromeos::ShutdownDBus().
     chromeos::ConciergeClient::Shutdown();
     chromeos::DBusThreadManager::Shutdown();
   }

chrome/browser/feedback/system_logs/log_sources/crash_ids_source_unittest.cc

@@ -59,8 +59,9 @@ TestDebugDaemonClient* fake_debug_client() {
 TEST(CrashIdsSourceTest, CallsCrashSender) {
   content::BrowserTaskEnvironment task_environment;
 
-  auto setter = chromeos::DBusThreadManager::GetSetterForTesting();
-  setter->SetDebugDaemonClient(std::make_unique<TestDebugDaemonClient>());
+  chromeos::DBusThreadManager::Initialize();
+  chromeos::DBusThreadManager::GetSetterForTesting()->SetDebugDaemonClient(
+      std::make_unique<TestDebugDaemonClient>());
 
   CrashIdsSource source;
   source.SetUploadListForTesting(new StubUploadList());

chrome/browser/ui/webui/help/version_updater_chromeos_unittest.cc

@@ -57,9 +57,8 @@ class VersionUpdaterCrosTest : public ::testing::Test {
 
   void SetUp() override {
     fake_update_engine_client_ = new FakeUpdateEngineClient();
-    std::unique_ptr<DBusThreadManagerSetter> dbus_setter =
-        DBusThreadManager::GetSetterForTesting();
-    dbus_setter->SetUpdateEngineClient(
+    DBusThreadManager::Initialize();
+    DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::unique_ptr<UpdateEngineClient>(fake_update_engine_client_));
 
     EXPECT_CALL(*mock_user_manager_, IsCurrentUserOwner())

chrome/browser/ui/webui/settings/about_handler_unittest.cc

@@ -39,9 +39,9 @@ class AboutHandlerTest : public testing::Test {
 
   void SetUp() override {
     fake_update_engine_client_ = new FakeUpdateEngineClient();
+    DBusThreadManager::Initialize();
     DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         base::WrapUnique<UpdateEngineClient>(fake_update_engine_client_));
-    DBusThreadManager::Initialize();
     ConciergeClient::InitializeFake(/*fake_cicerone_client=*/nullptr);
 
     handler_ = std::make_unique<TestAboutHandler>(&profile_);

chrome/browser/upgrade_detector/installed_version_updater_chromeos_unittest.cc

@@ -30,6 +30,7 @@ class InstalledVersionUpdaterTest : public ::testing::Test {
     auto fake_update_engine_client =
         std::make_unique<chromeos::FakeUpdateEngineClient>();
     fake_update_engine_client_ = fake_update_engine_client.get();
+    chromeos::DBusThreadManager::Initialize();
     chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::move(fake_update_engine_client));
 

chrome/browser/upgrade_detector/upgrade_detector_chromeos_unittest.cc

@@ -78,9 +78,8 @@ class UpgradeDetectorChromeosTest : public ::testing::Test {
                                            std::make_unique<base::Value>(true));
 
     fake_update_engine_client_ = new chromeos::FakeUpdateEngineClient();
-    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
-    dbus_setter->SetUpdateEngineClient(
+    chromeos::DBusThreadManager::Initialize();
+    chromeos::DBusThreadManager::GetSetterForTesting()->SetUpdateEngineClient(
         std::unique_ptr<chromeos::UpdateEngineClient>(
             fake_update_engine_client_));