Extract AuthPolicyClient from DBusThreadManager

Bug: 644350 Change-Id: I02fe8831543049a6b90b552679bf25240b386ba9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1532735 Reviewed-by: Roman Sorokin [CET] <[email protected]> Commit-Queue: Steven Bennetts <[email protected]> Cr-Commit-Position: refs/heads/master@{#643176}
chensowf · Mar 21, 2019 · ba8ab3d · ba8ab3d
1 parent 00a78cf
commit ba8ab3d
Show file tree

Hide file tree

Showing 23 changed files with 148 additions and 142 deletions.
diff --git a/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc b/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc
@@ -28,7 +28,6 @@
 #include "chromeos/components/account_manager/account_manager_factory.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/dbus/auth_policy/auth_policy_client.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
@@ -86,7 +85,7 @@ AuthPolicyCredentialsManager::AuthPolicyCredentialsManager(Profile* profile)
 
   // Connecting to the signal sent by authpolicyd notifying that Kerberos files
   // have changed.
-  DBusThreadManager::Get()->GetAuthPolicyClient()->ConnectToSignal(
+  AuthPolicyClient::Get()->ConnectToSignal(
       authpolicy::kUserKerberosFilesChangedSignal,
       base::Bind(
           &AuthPolicyCredentialsManager::OnUserKerberosFilesChangedCallback,
@@ -125,7 +124,7 @@ void AuthPolicyCredentialsManager::GetUserStatus() {
   authpolicy::GetUserStatusRequest request;
   request.set_user_principal_name(account_id_.GetUserEmail());
   request.set_account_id(account_id_.GetObjGuid());
-  DBusThreadManager::Get()->GetAuthPolicyClient()->GetUserStatus(
+  AuthPolicyClient::Get()->GetUserStatus(
       request,
       base::BindOnce(&AuthPolicyCredentialsManager::OnGetUserStatusCallback,
                      weak_factory_.GetWeakPtr()));
@@ -188,7 +187,7 @@ void AuthPolicyCredentialsManager::OnGetUserStatusCallback(
 }
 
 void AuthPolicyCredentialsManager::GetUserKerberosFiles() {
-  DBusThreadManager::Get()->GetAuthPolicyClient()->GetUserKerberosFiles(
+  AuthPolicyClient::Get()->GetUserKerberosFiles(
       account_id_.GetObjGuid(),
       base::BindOnce(
           &AuthPolicyCredentialsManager::OnGetUserKerberosFilesCallback,

diff --git a/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager_unittest.cc b/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager_unittest.cc
@@ -52,6 +52,7 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
   void SetUp() override {
     chromeos::DBusThreadManager::Initialize();
     chromeos::NetworkHandler::Initialize();
+    AuthPolicyClient::InitializeFake();
     fake_auth_policy_client()->DisableOperationDelayForTesting();
 
     TestingProfile::Builder profile_builder;
@@ -83,8 +84,9 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
   void TearDown() override {
     EXPECT_CALL(*mock_user_manager(), Shutdown());
     profile_.reset();
-    chromeos::NetworkHandler::Shutdown();
-    chromeos::DBusThreadManager::Shutdown();
+    AuthPolicyClient::Shutdown();
+    NetworkHandler::Shutdown();
+    DBusThreadManager::Shutdown();
   }
 
  protected:
@@ -94,8 +96,7 @@ class AuthPolicyCredentialsManagerTest : public testing::Test {
     return auth_policy_credentials_manager_;
   }
   chromeos::FakeAuthPolicyClient* fake_auth_policy_client() const {
-    return static_cast<chromeos::FakeAuthPolicyClient*>(
-        chromeos::DBusThreadManager::Get()->GetAuthPolicyClient());
+    return chromeos::FakeAuthPolicyClient::Get();
   }
 
   MockUserManager* mock_user_manager() {

diff --git a/chrome/browser/chromeos/dbus/dbus_helper.cc b/chrome/browser/chromeos/dbus/dbus_helper.cc
@@ -6,6 +6,7 @@
 
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
+#include "chromeos/dbus/auth_policy/auth_policy_client.h"
 #include "chromeos/dbus/biod/biod_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/hammerd/hammerd_client.h"
@@ -39,12 +40,14 @@ void InitializeDBus() {
   }
 
   if (bus) {
+    AuthPolicyClient::Initialize(bus);
     BiodClient::Initialize(bus);  // For device::Fingerprint.
     KerberosClient::Initialize(bus);
     PowerManagerClient::Initialize(bus);
     SystemClockClient::Initialize(bus);
     UpstartClient::Initialize(bus);
   } else {
+    AuthPolicyClient::InitializeFake();
     BiodClient::InitializeFake();  // For device::Fingerprint.
     KerberosClient::InitializeFake();
     PowerManagerClient::InitializeFake();
@@ -60,6 +63,7 @@ void InitializeDBus() {
 }
 
 void ShutdownDBus() {
+  AuthPolicyClient::Shutdown();
   UpstartClient::Shutdown();
   SystemClockClient::Shutdown();
   PowerManagerClient::Shutdown();

diff --git a/chrome/browser/chromeos/login/active_directory_login_browsertest.cc b/chrome/browser/chromeos/login/active_directory_login_browsertest.cc
@@ -83,11 +83,11 @@ class ActiveDirectoryLoginTest : public LoginManagerTest {
   void SetUpInProcessBrowserTestFixture() override {
     LoginManagerTest::SetUpInProcessBrowserTestFixture();
 
-    auto fake_client = std::make_unique<FakeAuthPolicyClient>();
-    fake_auth_policy_client_ = fake_client.get();
-    fake_auth_policy_client_->DisableOperationDelayForTesting();
-    DBusThreadManager::GetSetterForTesting()->SetAuthPolicyClient(
-        std::move(fake_client));
+    // This is called before ChromeBrowserMain initializes the fake dbus
+    // clients, and DisableOperationDelayForTesting() needs to be called before
+    // other ChromeBrowserMain initialization occurs.
+    AuthPolicyClient::InitializeFake();
+    FakeAuthPolicyClient::Get()->DisableOperationDelayForTesting();
 
     // Note: FakeCryptohomeClient needs paths to be set to load install attribs.
     active_directory_test_helper::OverridePaths();
@@ -286,15 +286,14 @@ class ActiveDirectoryLoginTest : public LoginManagerTest {
     return "document.querySelector('#" + parent_id + "')." + selector;
   }
   FakeAuthPolicyClient* fake_auth_policy_client() {
-    return fake_auth_policy_client_;
+    return FakeAuthPolicyClient::Get();
   }
 
   const std::string test_realm_;
   const std::string test_user_;
   std::string autocomplete_realm_;
 
  private:
-  FakeAuthPolicyClient* fake_auth_policy_client_;
 
   DISALLOW_COPY_AND_ASSIGN(ActiveDirectoryLoginTest);
 };

diff --git a/chrome/browser/chromeos/login/active_directory_test_helper.cc b/chrome/browser/chromeos/login/active_directory_test_helper.cc
@@ -15,7 +15,6 @@
 #include "chromeos/constants/chromeos_paths.h"
 #include "chromeos/dbus/auth_policy/auth_policy_client.h"
 #include "chromeos/dbus/authpolicy/active_directory_info.pb.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/upstart/upstart_client.h"
 #include "chromeos/dbus/util/tpm_util.h"
 #include "chromeos/login/auth/authpolicy_login_helper.h"
@@ -69,14 +68,12 @@ void PrepareLogin(const std::string& user_principal_name) {
   // Fetch device policy.
   {
     base::RunLoop run_loop;
-    chromeos::DBusThreadManager::Get()
-        ->GetAuthPolicyClient()
-        ->RefreshDevicePolicy(base::BindOnce(
-            [](base::OnceClosure quit_closure, authpolicy::ErrorType error) {
-              EXPECT_EQ(authpolicy::ERROR_NONE, error);
-              std::move(quit_closure).Run();
-            },
-            run_loop.QuitClosure()));
+    AuthPolicyClient::Get()->RefreshDevicePolicy(base::BindOnce(
+        [](base::OnceClosure quit_closure, authpolicy::ErrorType error) {
+          EXPECT_EQ(authpolicy::ERROR_NONE, error);
+          std::move(quit_closure).Run();
+        },
+        run_loop.QuitClosure()));
     run_loop.Run();
   }
 }

diff --git a/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc b/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc
@@ -216,9 +216,9 @@ class ActiveDirectoryJoinTest : public EnterpriseEnrollmentTest {
   ActiveDirectoryJoinTest() = default;
 
   void SetUp() override {
-    DBusThreadManager::GetSetterForTesting()->SetAuthPolicyClient(
-        std::make_unique<MockAuthPolicyClient>());
+    mock_auth_policy_client_ = new MockAuthPolicyClient();
     mock_auth_policy_client()->DisableOperationDelayForTesting();
+
     EnterpriseEnrollmentTestBase::SetUp();
   }
 
@@ -372,8 +372,7 @@ class ActiveDirectoryJoinTest : public EnterpriseEnrollmentTest {
 
 
   MockAuthPolicyClient* mock_auth_policy_client() {
-    return static_cast<MockAuthPolicyClient*>(
-        DBusThreadManager::Get()->GetAuthPolicyClient());
+    return mock_auth_policy_client_;
   }
 
   void SetupActiveDirectoryJSNotifications() {
@@ -418,6 +417,9 @@ class ActiveDirectoryJoinTest : public EnterpriseEnrollmentTest {
   }
 
  private:
+  // Owned by the AuthPolicyClient global instance.
+  MockAuthPolicyClient* mock_auth_policy_client_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(ActiveDirectoryJoinTest);
 };
 

diff --git a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
@@ -831,7 +831,13 @@ class ExistingUserControllerActiveDirectoryTest
   // Overriden from ExistingUserControllerTest:
   void SetUpInProcessBrowserTestFixture() override {
     ExistingUserControllerTest::SetUpInProcessBrowserTestFixture();
-    fake_authpolicy_client()->DisableOperationDelayForTesting();
+
+    // This is called before ChromeBrowserMain initializes the fake dbus
+    // clients, and DisableOperationDelayForTesting() needs to be called before
+    // other ChromeBrowserMain initialization occurs.
+    AuthPolicyClient::InitializeFake();
+    FakeAuthPolicyClient::Get()->DisableOperationDelayForTesting();
+
     ASSERT_TRUE(
         tpm_util::LockDeviceActiveDirectoryForTesting(kActiveDirectoryRealm));
     RefreshDevicePolicy();
@@ -847,11 +853,6 @@ class ExistingUserControllerActiveDirectoryTest
   }
 
  protected:
-  chromeos::FakeAuthPolicyClient* fake_authpolicy_client() {
-    return static_cast<chromeos::FakeAuthPolicyClient*>(
-        chromeos::DBusThreadManager::Get()->GetAuthPolicyClient());
-  }
-
   void LoginAdOnline() {
     ExpectLoginSuccess();
     UserContext user_context(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
@@ -899,7 +900,7 @@ class ExistingUserControllerActiveDirectoryTest
   std::string GetExpectedKerberosConfig(bool enable_dns_cname_lookup) {
     std::string config(base::StringPrintf(
         kKrb5CnameSettings, enable_dns_cname_lookup ? "true" : "false"));
-    config += fake_authpolicy_client()->user_kerberos_conf();
+    config += FakeAuthPolicyClient::Get()->user_kerberos_conf();
     return config;
   }
 
@@ -913,7 +914,8 @@ class ExistingUserControllerActiveDirectoryTest
 
     EXPECT_TRUE(base::ReadFileToString(
         base::FilePath(GetKerberosCredentialsCacheFileName()), &file_contents));
-    EXPECT_EQ(file_contents, fake_authpolicy_client()->user_kerberos_creds());
+    EXPECT_EQ(file_contents,
+              FakeAuthPolicyClient::Get()->user_kerberos_creds());
   }
 
   // Applies policy and waits until both config and credentials files changed.
@@ -944,7 +946,7 @@ class ExistingUserControllerActiveDirectoryUserWhitelistTest
     em::ChromeDeviceSettingsProto device_policy;
     device_policy.mutable_user_whitelist()->add_user_whitelist()->assign(
         kUserWhitelist);
-    fake_authpolicy_client()->set_device_policy(device_policy);
+    FakeAuthPolicyClient::Get()->set_device_policy(device_policy);
   }
 
   void SetUpLoginDisplay() override {
@@ -988,8 +990,8 @@ IN_PROC_BROWSER_TEST_F(
     DISABLED_UserKerberosFilesChangedSignalTriggersFileUpdate) {
   LoginAdOnline();
   KerberosFilesChangeWaiter files_change_waiter(true /* files_must_exist */);
-  fake_authpolicy_client()->SetUserKerberosFiles("new_kerberos_creds",
-                                                 "new_kerberos_config");
+  FakeAuthPolicyClient::Get()->SetUserKerberosFiles("new_kerberos_creds",
+                                                    "new_kerberos_config");
   files_change_waiter.Wait();
   CheckKerberosFiles(true /* enable_dns_cname_lookup */);
 }

diff --git a/chrome/browser/chromeos/policy/active_directory_policy_manager.cc b/chrome/browser/chromeos/policy/active_directory_policy_manager.cc
@@ -44,13 +44,7 @@ void RunRefreshCallback(base::OnceCallback<void(bool success)> callback,
 
 // Gets the AuthPolicy D-Bus interface.
 chromeos::AuthPolicyClient* GetAuthPolicyClient() {
-  chromeos::DBusThreadManager* thread_manager =
-      chromeos::DBusThreadManager::Get();
-  DCHECK(thread_manager);
-  chromeos::AuthPolicyClient* auth_policy_client =
-      thread_manager->GetAuthPolicyClient();
-  DCHECK(auth_policy_client);
-  return auth_policy_client;
+  return chromeos::AuthPolicyClient::Get();
 }
 
 bool IsComponentPolicyDisabled() {

diff --git a/chrome/browser/chromeos/policy/active_directory_policy_manager_unittest.cc b/chrome/browser/chromeos/policy/active_directory_policy_manager_unittest.cc
@@ -29,8 +29,6 @@ namespace {
 
 class TestAuthPolicyClient : public chromeos::AuthPolicyClient {
  public:
-  void Init(dbus::Bus* bus) override { NOTIMPLEMENTED(); }
-
   void JoinAdDomain(const authpolicy::JoinDomainRequest& request,
                     int password_fd,
                     JoinCallback callback) override {
@@ -101,16 +99,16 @@ class ActiveDirectoryPolicyManagerTest : public testing::Test {
 
   // testing::Test overrides:
   void SetUp() override {
-    auto mock_client_unique_ptr = std::make_unique<TestAuthPolicyClient>();
-    mock_client_ = mock_client_unique_ptr.get();
-    chromeos::DBusThreadManager::GetSetterForTesting()->SetAuthPolicyClient(
-        std::move(mock_client_unique_ptr));
+    // Base class constructor sets the global instance which will be destroyed
+    // in AuthPolicyClient::Shutdown().
+    mock_client_ = new TestAuthPolicyClient();
   }
 
   void TearDown() override {
     if (mock_external_data_manager())
       EXPECT_CALL(*mock_external_data_manager(), Disconnect());
     policy_manager_->Shutdown();
+    chromeos::AuthPolicyClient::Shutdown();
   }
 
  protected:
@@ -141,7 +139,7 @@ class ActiveDirectoryPolicyManagerTest : public testing::Test {
       testing::Mock::VerifyAndClearExpectations(mock_external_data_manager());
   }
 
-  // Owned by DBusThreadManager.
+  // Owned by the AuthPolicyClient global instance.
   TestAuthPolicyClient* mock_client_ = nullptr;
 
   // Used to set FakeUserManager.

diff --git a/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
@@ -748,11 +748,9 @@ void EnrollmentHandlerChromeOS::OnDeviceAccountTokenStored() {
     // policy is accepted.
     chromeos::DeviceSettingsService::Get()->SetDeviceMode(
         install_attributes_->GetMode());
-    chromeos::DBusThreadManager::Get()
-        ->GetAuthPolicyClient()
-        ->RefreshDevicePolicy(base::BindOnce(
-            &EnrollmentHandlerChromeOS::HandleActiveDirectoryPolicyRefreshed,
-            weak_ptr_factory_.GetWeakPtr()));
+    chromeos::AuthPolicyClient::Get()->RefreshDevicePolicy(base::BindOnce(
+        &EnrollmentHandlerChromeOS::HandleActiveDirectoryPolicyRefreshed,
+        weak_ptr_factory_.GetWeakPtr()));
   } else {
     store_->InstallInitialPolicy(*policy_);
   }

diff --git a/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc b/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc
@@ -165,19 +165,14 @@ class UserAffiliationBrowserTest
     chromeos::DBusThreadManager::GetSetterForTesting()->SetCryptohomeClient(
         std::make_unique<chromeos::FakeCryptohomeClient>());
 
-    // Initialize UpstartClient here so that it is available for
-    // FakeAuthPolicyClient. It will be shutdown in ChromeBrowserMain.
+    // Initialize clients here so they are available during setup. They will be
+    // shutdown in ChromeBrowserMain.
     chromeos::UpstartClient::InitializeFake();
-
     chromeos::FakeAuthPolicyClient* fake_auth_policy_client = nullptr;
     if (GetParam().active_directory) {
-      auto fake_auth_policy_client_owned =
-          std::make_unique<chromeos::FakeAuthPolicyClient>();
-      fake_auth_policy_client = fake_auth_policy_client_owned.get();
+      chromeos::AuthPolicyClient::InitializeFake();
+      fake_auth_policy_client = chromeos::FakeAuthPolicyClient::Get();
       fake_auth_policy_client->DisableOperationDelayForTesting();
-      chromeos::DBusThreadManager::GetSetterForTesting()->SetAuthPolicyClient(
-          std::move(fake_auth_policy_client_owned));
-
       // PrepareLogin requires a message loop, which isn't available yet here.
       base::MessageLoop message_loop;
       chromeos::active_directory_test_helper::PrepareLogin(