Bugfix: Account Manager's init callback is never invoked

If Chrome OS Account Manager
  - Has been fully initialized, and
  - Receives an initialization request with a callback,
that callback is never invoked.

Fix this by calling initialization callbacks instead of just early
exiting in the case of duplicate initialization requests.

Bug: 967602
Test: chromeos_components_unittests --gtest_filter="*AccountManager*Test*"
Change-Id: I2d072c50df158c81adc1be497c9891603ac52038
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1722532
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682260}

chromeos/components/account_manager/account_manager.cc

@@ -202,6 +202,7 @@ void AccountManager::Initialize(
     // invocation of |Initialize| matches the one it is currently being called
     // with.
     DCHECK_EQ(home_dir, writer_->path().DirName());
+    std::move(initialization_callback).Run();
     return;
   }
 

chromeos/components/account_manager/account_manager.h

@@ -139,6 +139,8 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
 
   // Same as above except that it accepts an |initialization_callback|, which
   // will be called after Account Manager has been fully initialized.
+  // If Account Manager has already been fully initialized,
+  // |initialization_callback| is called immediately.
   // Note: During initialization, there is no ordering guarantee between
   // |initialization_callback| and Account Manager's observers getting their
   // callbacks.
@@ -261,7 +263,7 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
   class GaiaTokenRevocationRequest;
 
   friend class AccountManagerTest;
-  FRIEND_TEST_ALL_PREFIXES(AccountManagerTest, TestInitialization);
+  FRIEND_TEST_ALL_PREFIXES(AccountManagerTest, TestInitializationCompletes);
   FRIEND_TEST_ALL_PREFIXES(AccountManagerTest, TestTokenPersistence);
   FRIEND_TEST_ALL_PREFIXES(AccountManagerTest,
                            UpdatingAccountEmailShouldNotOverwriteTokens);

chromeos/components/account_manager/account_manager_unittest.cc

@@ -109,11 +109,21 @@ class AccountManagerTest : public testing::Test {
   // parameters.
   void ResetAndInitializeAccountManager() {
     account_manager_ = std::make_unique<AccountManagerSpy>();
-    account_manager_->Initialize(
+    InitializeAccountManager(account_manager_.get(), base::DoNothing());
+  }
+
+  // |account_manager| is a non-owning pointer.
+  void InitializeAccountManager(AccountManager* account_manager,
+                                base::OnceClosure initialization_callback) {
+    account_manager->Initialize(
         tmp_dir_.GetPath(), test_url_loader_factory_.GetSafeWeakWrapper(),
         immediate_callback_runner_, base::SequencedTaskRunnerHandle::Get(),
-        base::DoNothing());
-    account_manager_->SetPrefService(&pref_service_);
+        std::move(initialization_callback));
+    account_manager->SetPrefService(&pref_service_);
+    scoped_task_environment_.RunUntilIdle();
+    EXPECT_EQ(account_manager->init_state_,
+              AccountManager::InitializationState::kInitialized);
+    EXPECT_TRUE(account_manager->IsInitialized());
   }
 
   // Check base/test/scoped_task_environment.h. This must be the first member /
@@ -182,20 +192,39 @@ TEST(AccountManagerKeyTest, TestValidity) {
   EXPECT_TRUE(key3.IsValid());
 }
 
-TEST_F(AccountManagerTest, TestInitialization) {
+TEST_F(AccountManagerTest, TestInitializationCompletes) {
   AccountManager account_manager;
 
   EXPECT_EQ(account_manager.init_state_,
             AccountManager::InitializationState::kNotStarted);
-  account_manager.Initialize(
-      tmp_dir_.GetPath(), test_url_loader_factory_.GetSafeWeakWrapper(),
-      immediate_callback_runner_, base::SequencedTaskRunnerHandle::Get(),
-      base::DoNothing());
-  account_manager.SetPrefService(&pref_service_);
-  scoped_task_environment_.RunUntilIdle();
-  EXPECT_EQ(account_manager.init_state_,
-            AccountManager::InitializationState::kInitialized);
-  EXPECT_TRUE(account_manager.IsInitialized());
+  // Test assertions will be made inside the method.
+  InitializeAccountManager(&account_manager, base::DoNothing());
+}
+
+TEST_F(AccountManagerTest, TestInitializationCallbackIsCalled) {
+  bool init_callback_was_called = false;
+  base::OnceClosure closure = base::BindLambdaForTesting(
+      [&init_callback_was_called]() { init_callback_was_called = true; });
+  AccountManager account_manager;
+  InitializeAccountManager(&account_manager, std::move(closure));
+  ASSERT_TRUE(init_callback_was_called);
+}
+
+// Tests that |AccountManager::Initialize|'s callback parameter is called, if
+// |AccountManager::Initialize| is invoked after Account Manager has been fully
+// initialized.
+TEST_F(AccountManagerTest,
+       TestInitializationCallbackIsCalledIfAccountManagerIsAlreadyInitialized) {
+  // Make sure that Account Manager is fully initialized.
+  AccountManager account_manager;
+  InitializeAccountManager(&account_manager, base::DoNothing());
+
+  // Send a duplicate initialization call.
+  bool init_callback_was_called = false;
+  base::OnceClosure closure = base::BindLambdaForTesting(
+      [&init_callback_was_called]() { init_callback_was_called = true; });
+  InitializeAccountManager(&account_manager, std::move(closure));
+  ASSERT_TRUE(init_callback_was_called);
 }
 
 TEST_F(AccountManagerTest, TestUpsert) {