[Signin] Cleanup reconcilor behavior while tokens are not loaded

Bug: 765106
Change-Id: I996d81b304dc114f589de1a9aa0c64a7e7266e58
Reviewed-on: https://chromium-review.googlesource.com/671243
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503411}

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -319,6 +319,7 @@ TEST_F(AccountReconcilorTest, ProfileAlreadyConnected) {
 }
 
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
+
 // Tests that the AccountReconcilor is enabled when Dice is enabled.
 TEST_F(AccountReconcilorTest, EnabledWithDice) {
   signin::ScopedAccountConsistencyDice scoped_dice;
@@ -478,6 +479,34 @@ TEST_F(AccountReconcilorTest, DiceLastKnownFirstAccount) {
 
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
+// Tests that reconcile cannot start before the tokens are loaded, and is
+// automatically started when tokens are loaded.
+TEST_F(AccountReconcilorTest, TokensNotLoaded) {
+  const std::string account_id =
+      ConnectProfileToAccount("12345", "user@gmail.com");
+  cookie_manager_service()->SetListAccountsResponseNoAccounts();
+  token_service()->set_all_credentials_loaded_for_testing(false);
+
+  AccountReconcilor* reconcilor = GetMockReconcilor();
+  reconcilor->StartReconcile();
+
+#if !defined(OS_CHROMEOS)
+  // No reconcile when tokens are not loaded, except on ChromeOS where reconcile
+  // can start as long as the token service is not empty.
+  ASSERT_FALSE(reconcilor->is_reconcile_started_);
+  // When tokens are loaded, reconcile starts automatically.
+  token_service()->LoadCredentials(account_id);
+#endif
+
+  EXPECT_CALL(*GetMockReconcilor(), PerformMergeAction(account_id));
+  ASSERT_TRUE(reconcilor->is_reconcile_started_);
+  base::RunLoop().RunUntilIdle();
+  SimulateAddAccountToCookieCompleted(reconcilor, account_id,
+                                      GoogleServiceAuthError::AuthErrorNone());
+  ASSERT_FALSE(reconcilor->is_reconcile_started_);
+  ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_OK, reconcilor->GetState());
+}
+
 TEST_F(AccountReconcilorTest, GetAccountsFromCookieSuccess) {
   const std::string account_id =
       ConnectProfileToAccount("12345", "user@gmail.com");

chrome/browser/signin/dice_browsertest.cc

@@ -505,7 +505,7 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, MAYBE_Reauth) {
 
   // Start from a signed-in state.
   SetupSignedInAccounts();
-  EXPECT_EQ(2, reconcilor_started_count_);
+  EXPECT_EQ(1, reconcilor_started_count_);
 
   // Navigate to Gaia and sign in again with the main account.
   NavigateToURL(kSigninURL);
@@ -526,7 +526,7 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, MAYBE_Reauth) {
 
   EXPECT_EQ(1, reconcilor_blocked_count_);
   WaitForReconcilorUnblockedCount(1);
-  EXPECT_EQ(3, reconcilor_started_count_);
+  EXPECT_EQ(2, reconcilor_started_count_);
 }
 
 // Checks that the Dice signout flow works and deletes all tokens.

components/signin/core/browser/account_reconcilor.cc

@@ -240,6 +240,10 @@ void AccountReconcilor::OnEndBatchChanges() {
   StartReconcile();
 }
 
+void AccountReconcilor::OnRefreshTokensLoaded() {
+  StartReconcile();
+}
+
 void AccountReconcilor::GoogleSigninSucceeded(const std::string& account_id,
                                               const std::string& username) {
   VLOG(1) << "AccountReconcilor::GoogleSigninSucceeded: signed in";
@@ -280,6 +284,9 @@ void AccountReconcilor::PerformLogoutAllAccountsAction() {
 }
 
 void AccountReconcilor::StartReconcile() {
+  if (is_reconcile_started_)
+    return;
+
   if (IsReconcileBlocked()) {
     VLOG(1) << "AccountReconcilor::StartReconcile: "
             << "Reconcile is blocked, scheduling for later.";
@@ -288,18 +295,22 @@ void AccountReconcilor::StartReconcile() {
     return;
   }
 
-  for (auto& observer : observer_list_)
-    observer.OnStartReconcile();
-
-  reconcile_start_time_ = base::Time::Now();
 
   if (!IsEnabled() || !client_->AreSigninCookiesAllowed()) {
     VLOG(1) << "AccountReconcilor::StartReconcile: !enabled or no cookies";
     return;
   }
 
-  if (is_reconcile_started_)
+  // Do not reconcile if tokens are not loaded yet.
+  if (!IsTokenServiceReady()) {
+    VLOG(1)
+        << "AccountReconcilor::StartReconcile: token service *not* ready yet.";
     return;
+  }
+
+  reconcile_start_time_ = base::Time::Now();
+  for (auto& observer : observer_list_)
+    observer.OnStartReconcile();
 
   // Reset state for validating gaia cookie.
   gaia_accounts_.clear();
@@ -343,11 +354,6 @@ void AccountReconcilor::OnGaiaAccountsInCookieUpdated(
           << "Error was " << error.ToString();
   if (error.state() == GoogleServiceAuthError::NONE) {
     gaia_accounts_ = accounts;
-
-    // It is possible that O2RT is not available at this moment.
-    if (!IsTokenServiceReady())
-      return;
-
     is_reconcile_started_ ? FinishReconcile() : StartReconcile();
   } else {
     if (is_reconcile_started_)

components/signin/core/browser/account_reconcilor.h

@@ -117,6 +117,7 @@ class AccountReconcilor : public KeyedService,
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, DiceLastKnownFirstAccount);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
                            DiceReconcileReuseGaiaFirstAccount);
+  FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, TokensNotLoaded);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
                            StartReconcileCookiesDisabled);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
@@ -208,6 +209,7 @@ class AccountReconcilor : public KeyedService,
 
   // Overriden from OAuth2TokenService::Observer.
   void OnEndBatchChanges() override;
+  void OnRefreshTokensLoaded() override;
 
   // Overriden from SigninManagerBase::Observer.
   void GoogleSigninSucceeded(const std::string& account_id,

components/signin/core/browser/profile_oauth2_token_service.h

@@ -61,6 +61,10 @@ class ProfileOAuth2TokenService : public OAuth2TokenService,
   // is no such instance.
   const net::BackoffEntry* GetDelegateBackoffEntry();
 
+  void set_all_credentials_loaded_for_testing(bool loaded) {
+    all_credentials_loaded_ = loaded;
+  }
+
  private:
   void OnRefreshTokenAvailable(const std::string& account_id) override;
   void OnRefreshTokenRevoked(const std::string& account_id) override;