Move AccountManagerAsh to chromeos/components/account_manager

Moves AccountManagerAsh to chromeos/components/account_manager to
prepare the upcoming merge of AccountManagerFacade implementations.

This CL moves AccountManagerAsh's ownership from AshChromeServiceImpl to
chromeos::AccountManagerFactory. Subsequent CLs will change
account_manager_facade_factory_ash.cc to instantiate
AccountManagerFacadeImpl instead of AccountManagerFacadeAsh (which will
be removed).

Bug: 1161699
Change-Id: I1a0dfb3ed7034b2ee8fa3fae3741f4d1555f8a27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616353Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843723}

chrome/browser/chromeos/BUILD.gn

@@ -1046,8 +1046,6 @@ source_set("chromeos") {
     "chrome_content_browser_client_chromeos_part.h",
     "concierge_helper_service.cc",
     "concierge_helper_service.h",
-    "crosapi/account_manager_ash.cc",
-    "crosapi/account_manager_ash.h",
     "crosapi/ash_chrome_service_impl.cc",
     "crosapi/ash_chrome_service_impl.h",
     "crosapi/browser_loader.cc",
@@ -3505,7 +3503,6 @@ source_set("unit_tests") {
     "child_accounts/usage_time_state_notifier_unittest.cc",
     "chrome_content_browser_client_chromeos_part_unittest.cc",
     "concierge_helper_service_unittest.cc",
-    "crosapi/account_manager_ash_unittest.cc",
     "crosapi/browser_loader_unittest.cc",
     "crosapi/browser_util_unittest.cc",
     "crosapi/message_center_ash_unittest.cc",

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.cc

@@ -11,7 +11,6 @@
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
-#include "chrome/browser/chromeos/crosapi/account_manager_ash.h"
 #include "chrome/browser/chromeos/crosapi/browser_manager.h"
 #include "chrome/browser/chromeos/crosapi/cert_database_ash.h"
 #include "chrome/browser/chromeos/crosapi/clipboard_ash.h"
@@ -29,6 +28,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chromeos/components/account_manager/account_manager.h"
+#include "chromeos/components/account_manager/account_manager_ash.h"
 #include "chromeos/components/account_manager/account_manager_factory.h"
 #include "chromeos/crosapi/mojom/feedback.mojom.h"
 #include "chromeos/crosapi/mojom/file_manager.mojom.h"
@@ -64,9 +64,6 @@ AshChromeServiceImpl::~AshChromeServiceImpl() = default;
 
 void AshChromeServiceImpl::BindAccountManager(
     mojo::PendingReceiver<mojom::AccountManager> receiver) {
-  // TODO(https://crrev.com/c/2601750): Move AccountManagerAsh ownership to
-  // chromeos::AccountManager.
-  if (!account_manager_ash_) {
   // Assumptions:
   // 1. TODO(https://crbug.com/1102768): Multi-Signin / Fast-User-Switching is
   // disabled.
@@ -80,22 +77,19 @@ void AshChromeServiceImpl::BindAccountManager(
   }
   DCHECK_EQ(1, num_regular_profiles);
 #endif  // DCHECK_IS_ON()
-    // Given these assumptions, there is 1 and only 1 Account Manager that
+  // Given these assumptions, there is 1 and only 1 AccountManagerAsh that
   // can/should be contacted - the one attached to the regular |Profile| in
   // ash-chrome, for the current |User|.
   const user_manager::User* const user =
       user_manager::UserManager::Get()->GetActiveUser();
   const Profile* const profile =
       chromeos::ProfileHelper::Get()->GetProfileByUser(user);
-    chromeos::AccountManager* const account_manager =
+  crosapi::AccountManagerAsh* const account_manager_ash =
       g_browser_process->platform_part()
           ->GetAccountManagerFactory()
-            ->GetAccountManager(
+          ->GetAccountManagerAsh(
               /* profile_path = */ profile->GetPath().value());
-    account_manager_ash_ =
-        std::make_unique<crosapi::AccountManagerAsh>(account_manager);
-  }
-  account_manager_ash_->BindReceiver(std::move(receiver));
+  account_manager_ash->BindReceiver(std::move(receiver));
 }
 
 void AshChromeServiceImpl::BindFileManager(

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.h

@@ -13,7 +13,6 @@
 
 namespace crosapi {
 
-class AccountManagerAsh;
 class CertDatabaseAsh;
 class ClipboardAsh;
 class DeviceAttributesAsh;
@@ -75,7 +74,6 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
  private:
   mojo::Receiver<mojom::AshChromeService> receiver_;
 
-  std::unique_ptr<AccountManagerAsh> account_manager_ash_;
   std::unique_ptr<DeviceAttributesAsh> device_attributes_ash_;
   std::unique_ptr<FileManagerAsh> file_manager_ash_;
   std::unique_ptr<KeystoreServiceAsh> keystore_service_ash_;

chromeos/components/account_manager/BUILD.gn

@@ -10,6 +10,8 @@ component("account_manager") {
   sources = [
     "account_manager.cc",
     "account_manager.h",
+    "account_manager_ash.cc",
+    "account_manager_ash.h",
     "account_manager_facade_ash.cc",
     "account_manager_facade_ash.h",
     "account_manager_factory.cc",
@@ -26,6 +28,7 @@ component("account_manager") {
   deps = [
     "//base",
     "//chromeos/constants",
+    "//chromeos/crosapi/mojom",
     "//components/prefs:prefs",
     "//google_apis",
     "//net",
@@ -37,12 +40,16 @@ component("account_manager") {
 
 source_set("unit_tests") {
   testonly = true
-  sources = [ "account_manager_unittest.cc" ]
+  sources = [
+    "account_manager_ash_unittest.cc",
+    "account_manager_unittest.cc",
+  ]
 
   deps = [
     ":account_manager",
     "//base",
     "//base/test:test_support",
+    "//chromeos/crosapi/mojom",
     "//components/prefs:test_support",
     "//net",
     "//services/network:test_support",

chrome/browser/chromeos/crosapi/account_manager_ash.cc → chromeos/components/account_manager/account_manager_ash.cc

@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/crosapi/account_manager_ash.h"
+#include "chromeos/components/account_manager/account_manager_ash.h"
 
 #include <utility>
 
 #include "base/callback.h"
 #include "base/notreached.h"
+#include "chromeos/components/account_manager/account_manager.h"
 #include "components/account_manager_core/account_manager_util.h"
 #include "mojo/public/cpp/bindings/remote.h"
 

chrome/browser/chromeos/crosapi/account_manager_ash.h → chromeos/components/account_manager/account_manager_ash.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_CROSAPI_ACCOUNT_MANAGER_ASH_H_
-#define CHROME_BROWSER_CHROMEOS_CROSAPI_ACCOUNT_MANAGER_ASH_H_
+#ifndef CHROMEOS_COMPONENTS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_ASH_H_
+#define CHROMEOS_COMPONENTS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_ASH_H_
 
 #include "base/optional.h"
 #include "chromeos/components/account_manager/account_manager.h"
@@ -18,7 +18,8 @@ namespace crosapi {
 // Implements the |crosapi::mojom::AccountManager| interface in ash-chrome.
 // It enables lacros-chrome to interact with accounts stored in the Chrome OS
 // Account Manager.
-class AccountManagerAsh : public mojom::AccountManager,
+class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManagerAsh
+    : public mojom::AccountManager,
       public chromeos::AccountManager::Observer {
  public:
   explicit AccountManagerAsh(chromeos::AccountManager* account_manager);
@@ -49,4 +50,4 @@ class AccountManagerAsh : public mojom::AccountManager,
 
 }  // namespace crosapi
 
-#endif  // CHROME_BROWSER_CHROMEOS_CROSAPI_ACCOUNT_MANAGER_ASH_H_
+#endif  // CHROMEOS_COMPONENTS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_ASH_H_

chrome/browser/chromeos/crosapi/account_manager_ash_unittest.cc → chromeos/components/account_manager/account_manager_ash_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/crosapi/account_manager_ash.h"
+#include "chromeos/components/account_manager/account_manager_ash.h"
 
 #include <cstddef>
 #include <memory>
@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind.h"
 #include "base/test/task_environment.h"
+#include "chromeos/components/account_manager/account_manager.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom-test-utils.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom.h"
 #include "components/account_manager_core/account_manager_util.h"
@@ -116,7 +117,7 @@ class AccountManagerAshTest : public ::testing::Test {
     return account_manager_.IsInitialized();
   }
 
-  size_t GetNumObservers() { return account_manager_ash_->observers_.size(); }
+  int GetNumObservers() { return account_manager_ash_->observers_.size(); }
 
   mojom::AccountManagerAsyncWaiter* account_manager_async_waiter() {
     return account_manager_async_waiter_.get();

chromeos/components/account_manager/account_manager_factory.cc

@@ -19,14 +19,40 @@ AccountManager* AccountManagerFactory::GetAccountManager(
     const std::string& profile_path) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  return GetAccountManagerHolder(profile_path).account_manager.get();
+}
+
+crosapi::AccountManagerAsh* AccountManagerFactory::GetAccountManagerAsh(
+    const std::string& profile_path) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  return GetAccountManagerHolder(profile_path).account_manager_ash.get();
+}
+
+AccountManagerFactory::AccountManagerHolder::AccountManagerHolder(
+    std::unique_ptr<AccountManager> account_manager,
+    std::unique_ptr<crosapi::AccountManagerAsh> account_manager_ash)
+    : account_manager(std::move(account_manager)),
+      account_manager_ash(std::move(account_manager_ash)) {}
+
+AccountManagerFactory::AccountManagerHolder::~AccountManagerHolder() = default;
+
+const AccountManagerFactory::AccountManagerHolder&
+AccountManagerFactory::GetAccountManagerHolder(
+    const std::string& profile_path) {
   auto it = account_managers_.find(profile_path);
   if (it == account_managers_.end()) {
+    auto account_manager = std::make_unique<AccountManager>();
+    auto account_manager_ash =
+        std::make_unique<crosapi::AccountManagerAsh>(account_manager.get());
     it = account_managers_
-             .emplace(profile_path, std::make_unique<AccountManager>())
+             .emplace(std::piecewise_construct,
+                      std::forward_as_tuple(profile_path),
+                      std::forward_as_tuple(std::move(account_manager),
+                                            std::move(account_manager_ash)))
              .first;
   }
-
-  return it->second.get();
+  return it->second;
 }
 
 }  // namespace chromeos

chromeos/components/account_manager/account_manager_factory.h

@@ -12,6 +12,7 @@
 #include "base/component_export.h"
 #include "base/sequence_checker.h"
 #include "chromeos/components/account_manager/account_manager.h"
+#include "chromeos/components/account_manager/account_manager_ash.h"
 
 namespace chromeos {
 
@@ -34,11 +35,29 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManagerFactory {
   // Returns the |AccountManager| corresponding to the given |profile_path|.
   AccountManager* GetAccountManager(const std::string& profile_path);
 
+  // Returns the |AccountManagerAsh| corresponding to the given |profile_path|.
+  crosapi::AccountManagerAsh* GetAccountManagerAsh(
+      const std::string& profile_path);
+
  private:
-  // A mapping from Profile path to an |AccountManager|. Acts a cache of
-  // Account Managers.
-  std::unordered_map<std::string, std::unique_ptr<AccountManager>>
-      account_managers_;
+  struct AccountManagerHolder {
+    AccountManagerHolder(
+        std::unique_ptr<AccountManager> account_manager,
+        std::unique_ptr<crosapi::AccountManagerAsh> account_manager_ash);
+    AccountManagerHolder(const AccountManagerHolder&) = delete;
+    AccountManagerHolder& operator=(const AccountManagerHolder&) = delete;
+    ~AccountManagerHolder();
+
+    const std::unique_ptr<AccountManager> account_manager;
+    const std::unique_ptr<crosapi::AccountManagerAsh> account_manager_ash;
+  };
+
+  const AccountManagerHolder& GetAccountManagerHolder(
+      const std::string& profile_path);
+
+  // A mapping from Profile path to an |AccountManagerHolder|. Acts a cache of
+  // Account Managers and AccountManagerAsh objects.
+  std::unordered_map<std::string, AccountManagerHolder> account_managers_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 };