Lacros: Notify AccountManager Mojo observers

Notify AccountManager observers in lacros-chrome about account updates
in AccountManager in ash-chrome.

Bug: 1117478
Test: unit_tests --gtest_filter="*AccountManagerAshTest*"
Change-Id: Ibddec136dc428c6f07413e106a49cc5e16d227e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461272
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815922}

chrome/browser/chromeos/crosapi/account_manager_ash.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/callback.h"
+#include "base/notreached.h"
 #include "chromeos/components/account_manager/account_manager.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
@@ -17,9 +18,12 @@ AccountManagerAsh::AccountManagerAsh(
     mojo::PendingReceiver<mojom::AccountManager> receiver)
     : account_manager_(account_manager), receiver_(this, std::move(receiver)) {
   DCHECK(account_manager_);
+  account_manager_->AddObserver(this);
 }
 
-AccountManagerAsh::~AccountManagerAsh() = default;
+AccountManagerAsh::~AccountManagerAsh() {
+  account_manager_->RemoveObserver(this);
+}
 
 void AccountManagerAsh::IsInitialized(IsInitializedCallback callback) {
   std::move(callback).Run(account_manager_->IsInitialized());
@@ -32,4 +36,79 @@ void AccountManagerAsh::AddObserver(AddObserverCallback callback) {
   std::move(callback).Run(std::move(receiver));
 }
 
+void AccountManagerAsh::OnTokenUpserted(
+    const chromeos::AccountManager::Account& account) {
+  for (auto& observer : observers_)
+    observer->OnTokenUpserted(ToMojoAccount(account));
+}
+
+void AccountManagerAsh::OnAccountRemoved(
+    const chromeos::AccountManager::Account& account) {
+  for (auto& observer : observers_)
+    observer->OnAccountRemoved(ToMojoAccount(account));
+}
+
+// static
+chromeos::AccountManager::Account AccountManagerAsh::FromMojoAccount(
+    const mojom::AccountPtr& mojom_account) {
+  chromeos::AccountManager::Account account;
+
+  account.key.id = mojom_account->key->id;
+  account.key.account_type =
+      FromMojoAccountType(mojom_account->key->account_type);
+  account.raw_email = mojom_account->raw_email;
+
+  return account;
+}
+
+// static
+mojom::AccountPtr AccountManagerAsh::ToMojoAccount(
+    const chromeos::AccountManager::Account& account) {
+  mojom::AccountPtr mojom_account = mojom::Account::New();
+
+  mojom_account->key = mojom::AccountKey::New();
+  mojom_account->key->id = account.key.id;
+  mojom_account->key->account_type =
+      ToMojoAccountType(account.key.account_type);
+  mojom_account->raw_email = account.raw_email;
+
+  return mojom_account;
+}
+
+// static
+chromeos::account_manager::AccountType AccountManagerAsh::FromMojoAccountType(
+    const mojom::AccountType& account_type) {
+  switch (account_type) {
+    case mojom::AccountType::kUnspecified:
+      return chromeos::account_manager::AccountType::ACCOUNT_TYPE_UNSPECIFIED;
+    case mojom::AccountType::kGaia:
+      return chromeos::account_manager::AccountType::ACCOUNT_TYPE_GAIA;
+    case mojom::AccountType::kActiveDirectory:
+      return chromeos::account_manager::AccountType::
+          ACCOUNT_TYPE_ACTIVE_DIRECTORY;
+    default:
+      // Ash does not know about this new account type. Don't consider this as
+      // as error to preserve forwards compatibility with lacros.
+      LOG(WARNING) << "Unknown account type: " << account_type;
+      return chromeos::account_manager::AccountType::ACCOUNT_TYPE_UNSPECIFIED;
+  }
+}
+
+// static
+mojom::AccountType AccountManagerAsh::ToMojoAccountType(
+    const chromeos::account_manager::AccountType& account_type) {
+  switch (account_type) {
+    case chromeos::account_manager::AccountType::ACCOUNT_TYPE_UNSPECIFIED:
+      return mojom::AccountType::kUnspecified;
+    case chromeos::account_manager::AccountType::ACCOUNT_TYPE_GAIA:
+      return mojom::AccountType::kGaia;
+    case chromeos::account_manager::AccountType::ACCOUNT_TYPE_ACTIVE_DIRECTORY:
+      return mojom::AccountType::kActiveDirectory;
+  }
+}
+
+void AccountManagerAsh::FlushMojoForTesting() {
+  observers_.FlushForTesting();
+}
+
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/account_manager_ash.h

@@ -5,21 +5,19 @@
 #ifndef CHROME_BROWSER_CHROMEOS_CROSAPI_ACCOUNT_MANAGER_ASH_H_
 #define CHROME_BROWSER_CHROMEOS_CROSAPI_ACCOUNT_MANAGER_ASH_H_
 
+#include "chromeos/components/account_manager/account_manager.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 
-namespace chromeos {
-class AccountManager;
-}  // namespace chromeos
-
 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 AccountManagerAsh : public mojom::AccountManager,
+                          public chromeos::AccountManager::Observer {
  public:
   AccountManagerAsh(chromeos::AccountManager* account_manager,
                     mojo::PendingReceiver<mojom::AccountManager> receiver);
@@ -31,8 +29,27 @@ class AccountManagerAsh : public mojom::AccountManager {
   void IsInitialized(IsInitializedCallback callback) override;
   void AddObserver(AddObserverCallback callback) override;
 
+  // chromeos::AccountManager::Observer:
+  void OnTokenUpserted(
+      const chromeos::AccountManager::Account& account) override;
+  void OnAccountRemoved(
+      const chromeos::AccountManager::Account& account) override;
+
  private:
   friend class AccountManagerAshTest;
+  friend class TestAccountManagerObserver;
+
+  // Following util functions are static for ease of testing.
+  static chromeos::AccountManager::Account FromMojoAccount(
+      const mojom::AccountPtr& mojom_account);
+  static mojom::AccountPtr ToMojoAccount(
+      const chromeos::AccountManager::Account& account);
+  static chromeos::account_manager::AccountType FromMojoAccountType(
+      const mojom::AccountType& account_type);
+  static mojom::AccountType ToMojoAccountType(
+      const chromeos::account_manager::AccountType& account_type);
+
+  void FlushMojoForTesting();
 
   chromeos::AccountManager* const account_manager_;
   mojo::Receiver<mojom::AccountManager> receiver_;

chrome/browser/chromeos/crosapi/account_manager_ash_unittest.cc

@@ -6,13 +6,17 @@
 
 #include <cstddef>
 #include <memory>
+#include <type_traits>
+#include <utility>
 
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
 #include "chromeos/components/account_manager/account_manager.h"
+#include "chromeos/components/account_manager/tokens.pb.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom-test-utils.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom.h"
+#include "components/prefs/testing_pref_service.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
@@ -21,6 +25,66 @@
 
 namespace crosapi {
 
+namespace {
+
+const char kFakeGaiaId[] = "fake-gaia-id";
+const char kFakeEmail[] = "fake_email@example.com";
+const char kFakeToken[] = "fake-token";
+
+}  // namespace
+
+class TestAccountManagerObserver
+    : public mojom::AccountManagerObserverInterceptorForTesting {
+ public:
+  TestAccountManagerObserver() : receiver_(this) {}
+
+  TestAccountManagerObserver(const TestAccountManagerObserver&) = delete;
+  TestAccountManagerObserver& operator=(const TestAccountManagerObserver&) =
+      delete;
+  ~TestAccountManagerObserver() override = default;
+
+  void Observe(
+      mojom::AccountManagerAsyncWaiter* const account_manager_async_waiter) {
+    mojo::PendingReceiver<mojom::AccountManagerObserver> receiver;
+    account_manager_async_waiter->AddObserver(&receiver);
+    receiver_.Bind(std::move(receiver));
+  }
+
+  int GetNumOnTokenUpsertedCalls() { return num_token_upserted_calls_; }
+
+  chromeos::AccountManager::Account GetLastUpsertedAccount() {
+    return last_upserted_account_;
+  }
+
+  int GetNumOnAccountRemovedCalls() { return num_account_removed_calls_; }
+
+  chromeos::AccountManager::Account GetLastRemovedAccount() {
+    return last_removed_account_;
+  }
+
+ private:
+  // mojom::AccountManagerObserverInterceptorForTesting override:
+  AccountManagerObserver* GetForwardingInterface() override { return this; }
+
+  // mojom::AccountManagerObserverInterceptorForTesting override:
+  void OnTokenUpserted(mojom::AccountPtr account) override {
+    ++num_token_upserted_calls_;
+    last_upserted_account_ = AccountManagerAsh::FromMojoAccount(account);
+  }
+
+  // mojom::AccountManagerObserverInterceptorForTesting override:
+  void OnAccountRemoved(mojom::AccountPtr account) override {
+    ++num_account_removed_calls_;
+    last_removed_account_ = AccountManagerAsh::FromMojoAccount(account);
+  }
+
+  int num_token_upserted_calls_ = 0;
+  int num_account_removed_calls_ = 0;
+  chromeos::AccountManager::Account last_upserted_account_;
+  chromeos::AccountManager::Account last_removed_account_;
+  mojo::Receiver<mojom::AccountManagerObserver> receiver_;
+};
+
 class AccountManagerAshTest : public ::testing::Test {
  public:
   AccountManagerAshTest() = default;
@@ -39,11 +103,15 @@ class AccountManagerAshTest : public ::testing::Test {
 
   void RunAllPendingTasks() { task_environment_.RunUntilIdle(); }
 
+  void FlushMojoForTesting() { account_manager_ash_->FlushMojoForTesting(); }
+
   // Returns |true| if initialization was successful.
   bool InitializeAccountManager() {
     base::RunLoop run_loop;
     account_manager_.InitializeInEphemeralMode(
         test_url_loader_factory_.GetSafeWeakWrapper(), run_loop.QuitClosure());
+    account_manager_.SetPrefService(&pref_service_);
+    account_manager_.RegisterPrefs(pref_service_.registry());
     run_loop.Run();
     return account_manager_.IsInitialized();
   }
@@ -54,10 +122,13 @@ class AccountManagerAshTest : public ::testing::Test {
     return account_manager_async_waiter_.get();
   }
 
+  chromeos::AccountManager* account_manager() { return &account_manager_; }
+
  private:
   base::test::SingleThreadTaskEnvironment task_environment_;
 
   network::TestURLLoaderFactory test_url_loader_factory_;
+  TestingPrefServiceSimple pref_service_;
   chromeos::AccountManager account_manager_;
   mojo::Remote<mojom::AccountManager> remote_;
   std::unique_ptr<AccountManagerAsh> account_manager_ash_;
@@ -96,4 +167,38 @@ TEST_F(AccountManagerAshTest,
   EXPECT_EQ(0, GetNumObservers());
 }
 
+TEST_F(AccountManagerAshTest, LacrosObserversAreNotifiedOnAccountUpdates) {
+  const chromeos::AccountManager::AccountKey kTestAccountKey{
+      kFakeGaiaId, chromeos::account_manager::AccountType::ACCOUNT_TYPE_GAIA};
+  ASSERT_TRUE(InitializeAccountManager());
+  TestAccountManagerObserver observer;
+  observer.Observe(account_manager_async_waiter());
+  ASSERT_EQ(1, GetNumObservers());
+
+  EXPECT_EQ(0, observer.GetNumOnTokenUpsertedCalls());
+  account_manager()->UpsertAccount(kTestAccountKey, kFakeEmail, kFakeToken);
+  FlushMojoForTesting();
+  EXPECT_EQ(1, observer.GetNumOnTokenUpsertedCalls());
+  EXPECT_EQ(kTestAccountKey, observer.GetLastUpsertedAccount().key);
+  EXPECT_EQ(kFakeEmail, observer.GetLastUpsertedAccount().raw_email);
+}
+
+TEST_F(AccountManagerAshTest, LacrosObserversAreNotifiedOnAccountRemovals) {
+  const chromeos::AccountManager::AccountKey kTestAccountKey{
+      kFakeGaiaId, chromeos::account_manager::AccountType::ACCOUNT_TYPE_GAIA};
+  ASSERT_TRUE(InitializeAccountManager());
+  TestAccountManagerObserver observer;
+  observer.Observe(account_manager_async_waiter());
+  ASSERT_EQ(1, GetNumObservers());
+  account_manager()->UpsertAccount(kTestAccountKey, kFakeEmail, kFakeToken);
+  FlushMojoForTesting();
+
+  EXPECT_EQ(0, observer.GetNumOnAccountRemovedCalls());
+  account_manager()->RemoveAccount(kTestAccountKey);
+  FlushMojoForTesting();
+  EXPECT_EQ(1, observer.GetNumOnAccountRemovedCalls());
+  EXPECT_EQ(kTestAccountKey, observer.GetLastRemovedAccount().key);
+  EXPECT_EQ(kFakeEmail, observer.GetLastRemovedAccount().raw_email);
+}
+
 }  // namespace crosapi