[Lacros] Add observers to AccountManagerFacade

Adds AccountManagerFacade::Observer interface along with
{Add/Remove}Observer methods, corresponding implementation in
AccountManagerFacadeImpl and some tests.

Bug: 1117472
Change-Id: Ia84f37703af56b8f22683d9a97cfc2dddb5579ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560122
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845349}

components/account_manager_core/account_manager_facade.cc

@@ -17,6 +17,9 @@ AccountManagerFacade::AccountAdditionResult::AccountAdditionResult(
     : status(status), error(error) {}
 AccountManagerFacade::AccountAdditionResult::~AccountAdditionResult() = default;
 
+AccountManagerFacade::Observer::Observer() = default;
+AccountManagerFacade::Observer::~Observer() = default;
+
 AccountManagerFacade::AccountManagerFacade() = default;
 AccountManagerFacade::~AccountManagerFacade() = default;
 

components/account_manager_core/account_manager_facade.h

@@ -9,6 +9,7 @@
 
 #include "base/callback.h"
 #include "base/component_export.h"
+#include "base/observer_list_types.h"
 #include "components/account_manager_core/account.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 
@@ -21,6 +22,20 @@ namespace account_manager {
 // Use |GetAccountManagerFacade()| to get an instance of this class.
 class COMPONENT_EXPORT(ACCOUNT_MANAGER_CORE) AccountManagerFacade {
  public:
+  // Observer interface to get notifications about changes in the account list.
+  class Observer : public base::CheckedObserver {
+   public:
+    Observer();
+    Observer(const Observer&) = delete;
+    Observer& operator=(const Observer&) = delete;
+    ~Observer() override;
+
+    // Invoked when an account is added or updated.
+    virtual void OnAccountUpserted(const AccountKey& account) = 0;
+    // Invoked when an account is removed.
+    virtual void OnAccountRemoved(const AccountKey& account) = 0;
+  };
+
   // The source UI surface used for launching the account addition /
   // re-authentication dialog. This should be as specific as possible.
   // These values are persisted to logs. Entries should not be renumbered and
@@ -83,6 +98,11 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER_CORE) AccountManagerFacade {
   // pipe to |AccountManager| is disconnected.
   virtual bool IsInitialized() = 0;
 
+  // Registers an observer. Ensures the observer wasn't already registered.
+  virtual void AddObserver(Observer* observer) = 0;
+  // Unregisters an observer that was registered using AddObserver.
+  virtual void RemoveObserver(Observer* observer) = 0;
+
   // Launches account addition dialog and calls the `callback` with the result.
   // If `result` is `kSuccess`, the added account will be passed to the
   // callback. Otherwise `account` will be set to `base::nullopt`.

components/account_manager_core/account_manager_facade_impl.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "components/account_manager_core/account_manager_util.h"
 
 namespace {
 
@@ -38,6 +39,14 @@ bool AccountManagerFacadeImpl::IsInitialized() {
   return is_initialized_;
 }
 
+void AccountManagerFacadeImpl::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void AccountManagerFacadeImpl::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 void AccountManagerFacadeImpl::ShowAddAccountDialog(
     const AccountAdditionSource& source,
     base::OnceCallback<void(const AccountAdditionResult& result)> callback) {
@@ -81,7 +90,29 @@ void AccountManagerFacadeImpl::OnInitialized(bool is_initialized) {
 void AccountManagerFacadeImpl::OnTokenUpserted(
     crosapi::mojom::AccountPtr account) {
   is_initialized_ = true;
+
+  base::Optional<account_manager::Account> maybe_account =
+      account_manager::FromMojoAccount(account);
+  if (!maybe_account) {
+    LOG(WARNING) << "Can't unmarshal account of type: "
+                 << account->key->account_type;
+    return;
+  }
+  for (auto& observer : observer_list_) {
+    observer.OnAccountUpserted(maybe_account->key);
+  }
 }
 
 void AccountManagerFacadeImpl::OnAccountRemoved(
-    crosapi::mojom::AccountPtr account) {}
+    crosapi::mojom::AccountPtr account) {
+  base::Optional<account_manager::Account> maybe_account =
+      account_manager::FromMojoAccount(account);
+  if (!maybe_account) {
+    LOG(WARNING) << "Can't unmarshal account of type: "
+                 << account->key->account_type;
+    return;
+  }
+  for (auto& observer : observer_list_) {
+    observer.OnAccountRemoved(maybe_account->key);
+  }
+}

components/account_manager_core/account_manager_facade_impl.h

@@ -9,6 +9,7 @@
 
 #include "base/component_export.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom.h"
 #include "components/account_manager_core/account_manager_facade.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -29,6 +30,8 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER_CORE) AccountManagerFacadeImpl
 
   // AccountManagerFacade overrides:
   bool IsInitialized() override;
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
   void ShowAddAccountDialog(
       const AccountAdditionSource& source,
       base::OnceCallback<void(const AccountAdditionResult& result)> callback)
@@ -51,6 +54,7 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER_CORE) AccountManagerFacadeImpl
   bool is_initialized_ = false;
   std::unique_ptr<mojo::Receiver<crosapi::mojom::AccountManagerObserver>>
       receiver_;
+  base::ObserverList<Observer> observer_list_;
 
   base::WeakPtrFactory<AccountManagerFacadeImpl> weak_factory_{this};
 };

components/account_manager_core/account_manager_facade_impl_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "components/account_manager_core/account_manager_facade_impl.h"
 
+#include "base/test/gmock_callback_support.h"
 #include "base/test/task_environment.h"
 #include "chromeos/crosapi/mojom/account_manager.mojom.h"
 #include "components/account_manager_core/account.h"
@@ -13,6 +14,7 @@
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -50,12 +52,38 @@ class FakeAccountManager : public crosapi::mojom::AccountManager {
     }
   }
 
+  void NotifyOnAccountRemovedObservers(
+      const account_manager::Account& account) {
+    for (auto& observer : observers_) {
+      observer->OnAccountRemoved(ToMojoAccount(account));
+    }
+  }
+
  private:
   bool is_initialized_{false};
   mojo::ReceiverSet<crosapi::mojom::AccountManager> receivers_;
   mojo::RemoteSet<crosapi::mojom::AccountManagerObserver> observers_;
 };
 
+class MockObserver : public account_manager::AccountManagerFacade::Observer {
+ public:
+  MockObserver() = default;
+  MockObserver(const MockObserver&) = delete;
+  MockObserver& operator=(const MockObserver&) = delete;
+  ~MockObserver() override = default;
+
+  MOCK_METHOD(void,
+              OnAccountUpserted,
+              (const account_manager::AccountKey& account),
+              (override));
+  MOCK_METHOD(void,
+              OnAccountRemoved,
+              (const account_manager::AccountKey& account),
+              (override));
+};
+
+constexpr char kTestAccountEmail[] = "test@gmail.com";
+
 }  // namespace
 
 class AccountManagerFacadeImplTest : public testing::Test {
@@ -97,4 +125,51 @@ TEST_F(AccountManagerFacadeImplTest, FacadeIsUninitializedByDefault) {
   EXPECT_FALSE(account_manager_facade->IsInitialized());
 }
 
-// TODO(https://crbug.com/1117472): Add more tests after implementing observers.
+TEST_F(AccountManagerFacadeImplTest,
+       UninitializedFacadeIsInitializedOnFirstTokenUpsertedNotification) {
+  std::unique_ptr<AccountManagerFacadeImpl> account_manager_facade =
+      CreateFacade();
+  ASSERT_FALSE(account_manager_facade->IsInitialized());
+
+  testing::StrictMock<MockObserver> observer;
+  account_manager_facade->AddObserver(&observer);
+
+  base::RunLoop run_loop;
+  EXPECT_CALL(observer, OnAccountUpserted(testing::_))
+      .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+  account_manager().NotifyOnTokenUpsertedObservers(
+      account_manager::CreateTestGaiaAccount(kTestAccountEmail));
+  run_loop.Run();
+
+  EXPECT_TRUE(account_manager_facade->IsInitialized());
+}
+
+TEST_F(AccountManagerFacadeImplTest, OnTokenUpsertedIsPropagatedToObservers) {
+  std::unique_ptr<AccountManagerFacadeImpl> account_manager_facade =
+      CreateFacade();
+  testing::StrictMock<MockObserver> observer;
+  account_manager_facade->AddObserver(&observer);
+
+  account_manager::Account account =
+      account_manager::CreateTestGaiaAccount(kTestAccountEmail);
+  base::RunLoop run_loop;
+  EXPECT_CALL(observer, OnAccountUpserted(account.key))
+      .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+  account_manager().NotifyOnTokenUpsertedObservers(account);
+  run_loop.Run();
+}
+
+TEST_F(AccountManagerFacadeImplTest, OnAccountRemovedIsPropagatedToObservers) {
+  std::unique_ptr<AccountManagerFacadeImpl> account_manager_facade =
+      CreateFacade();
+  testing::StrictMock<MockObserver> observer;
+  account_manager_facade->AddObserver(&observer);
+
+  account_manager::Account account =
+      account_manager::CreateTestGaiaAccount(kTestAccountEmail);
+  base::RunLoop run_loop;
+  EXPECT_CALL(observer, OnAccountRemoved(account.key))
+      .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+  account_manager().NotifyOnAccountRemovedObservers(account);
+  run_loop.Run();
+}