[Signin] Move Dice code from the AccountReconcilor

This CL moves some of Dice specific code from the `AccountReconcilor` to
`DiceAccountReconcilorDelegate`.

Change-Id: Idd0bdeb2939aa930d7dd8348bd80937b2b66ee3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3627578
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001061}

chrome/browser/signin/account_reconcilor_factory.cc

@@ -212,7 +212,8 @@ AccountReconcilorFactory::CreateAccountReconcilorDelegate(Profile* profile) {
 
     case signin::AccountConsistencyMethod::kDice:
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
-      return std::make_unique<signin::DiceAccountReconcilorDelegate>();
+      return std::make_unique<signin::DiceAccountReconcilorDelegate>(
+          IdentityManagerFactory::GetForProfile(profile));
 #else
       NOTREACHED();
       return nullptr;

chrome/browser/signin/dice_response_handler_unittest.cc

@@ -117,7 +117,8 @@ class DiceResponseHandlerTest : public testing::Test,
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     AboutSigninInternals::RegisterPrefs(pref_service_.registry());
     auto account_reconcilor_delegate =
-        std::make_unique<signin::DiceAccountReconcilorDelegate>();
+        std::make_unique<signin::DiceAccountReconcilorDelegate>(
+            identity_manager());
     account_reconcilor_ = std::make_unique<AccountReconcilor>(
         identity_test_env_.identity_manager(), &signin_client_,
         std::move(account_reconcilor_delegate));

components/signin/core/browser/BUILD.gn

@@ -114,7 +114,6 @@ source_set("unit_tests") {
     "account_investigator_unittest.cc",
     "account_reconcilor_delegate_unittest.cc",
     "account_reconcilor_unittest.cc",
-    "dice_account_reconcilor_delegate_unittest.cc",
     "signin_error_controller_unittest.cc",
     "signin_header_helper_unittest.cc",
     "signin_investigator_unittest.cc",
@@ -165,10 +164,6 @@ source_set("unit_tests") {
       "mirror_landing_account_reconcilor_delegate_unittest.cc",
     ]
   }
-
-  if (!enable_dice_support) {
-    sources -= [ "dice_account_reconcilor_delegate_unittest.cc" ]
-  }
 }
 
 if (is_android) {

components/signin/core/browser/account_reconcilor.cc

@@ -60,49 +60,6 @@ std::vector<gaia::ListedAccount> FilterUnverifiedAccounts(
   return verified_gaia_accounts;
 }
 
-// Revokes tokens for all accounts in chrome_accounts but the primary account.
-// Returns true if tokens were revoked, and false if the function did nothing.
-bool RevokeAllSecondaryTokens(
-    signin::IdentityManager* identity_manager,
-    signin::AccountReconcilorDelegate::RevokeTokenOption revoke_option,
-    const CoreAccountId& primary_account,
-    signin_metrics::SourceForRefreshTokenOperation source) {
-  bool token_revoked = false;
-  if (revoke_option ==
-      AccountReconcilorDelegate::RevokeTokenOption::kDoNotRevoke)
-    return false;
-  for (const CoreAccountInfo& account_info :
-       identity_manager->GetAccountsWithRefreshTokens()) {
-    CoreAccountId account = account_info.account_id;
-    if (account == primary_account)
-      continue;
-    bool should_revoke = false;
-    switch (revoke_option) {
-      case AccountReconcilorDelegate::RevokeTokenOption::kRevokeIfInError:
-        if (identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
-                account)) {
-          VLOG(1) << "Revoke token for " << account;
-          should_revoke = true;
-        }
-        break;
-      case AccountReconcilorDelegate::RevokeTokenOption::kRevoke:
-        VLOG(1) << "Revoke token for " << account;
-        should_revoke = true;
-        break;
-      case AccountReconcilorDelegate::RevokeTokenOption::kDoNotRevoke:
-        NOTREACHED();
-        break;
-    }
-    if (should_revoke) {
-      token_revoked = true;
-      VLOG(1) << "Revoke token for " << account;
-      auto* accounts_mutator = identity_manager->GetAccountsMutator();
-      accounts_mutator->RemoveAccount(account, source);
-    }
-  }
-  return token_revoked;
-}
-
 // Pick the account will become first after this reconcile is finished.
 CoreAccountId PickFirstGaiaAccount(
     const signin::MultiloginParameters& parameters,
@@ -461,30 +418,23 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
   DCHECK(!log_out_in_progress_);
   DCHECK_EQ(AccountReconcilorState::ACCOUNT_RECONCILOR_RUNNING, state_);
 
-  bool primary_has_error =
-      identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
-          primary_account);
-
   const signin::MultiloginParameters kLogoutParameters(
       gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER,
       std::vector<CoreAccountId>());
 
-  const bool should_revoke_tokens =
-      delegate_->ShouldRevokeTokensBeforeMultilogin(
-          chrome_accounts, primary_account, gaia_accounts, first_execution_,
-          primary_has_error);
+  const bool tokens_revoked =
+      delegate_->RevokeSecondaryTokensBeforeMultiloginIfNeeded(
+          chrome_accounts, gaia_accounts, first_execution_);
 
   DCHECK(is_reconcile_started_);
   signin::MultiloginParameters parameters_for_multilogin;
-  if (should_revoke_tokens) {
+  if (tokens_revoked) {
     // Set parameters for logout for deleting cookies.
     parameters_for_multilogin = kLogoutParameters;
-    RevokeAllSecondaryTokens(
-        identity_manager_,
-        AccountReconcilorDelegate::RevokeTokenOption::kRevoke, primary_account,
-        signin_metrics::SourceForRefreshTokenOperation::
-            kAccountReconcilor_Reconcile);
   } else {
+    bool primary_has_error =
+        identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
+            primary_account);
     parameters_for_multilogin = delegate_->CalculateParametersForMultilogin(
         chrome_accounts, primary_account, gaia_accounts, first_execution_,
         primary_has_error);
@@ -593,13 +543,7 @@ void AccountReconcilor::OnAccountsInCookieUpdated(
   // completely remove them from Chrome.
   // Revoking the token for the primary account is not supported (it should be
   // signed out or put to auth error state instead).
-  // TODO(https://crbug.com/1122551): Move to |DiceAccountReconcilorDelegate|.
-  AccountReconcilorDelegate::RevokeTokenOption revoke_option =
-      delegate_->ShouldRevokeSecondaryTokensBeforeReconcile(
-          verified_gaia_accounts);
-  RevokeAllSecondaryTokens(identity_manager_, revoke_option, primary_account,
-                           signin_metrics::SourceForRefreshTokenOperation::
-                               kAccountReconcilor_GaiaCookiesUpdated);
+  delegate_->RevokeSecondaryTokensBeforeReconcileIfNeeded();
 
   std::vector<CoreAccountId> chrome_accounts =
       LoadValidAccountsFromTokenService();
@@ -619,33 +563,8 @@ void AccountReconcilor::OnAccountsInCookieUpdated(
 }
 
 void AccountReconcilor::OnAccountsCookieDeletedByUserAction() {
-  if (!delegate_->ShouldRevokeTokensOnCookieDeleted())
-    return;
-
-  // This code is only used with DiceAccountReconcilorDelegate and should thus
-  // use sync account.
-  // TODO(https://crbug.com/1122551): Move to |DiceAccountReconcilorDelegate|.
-  DCHECK_EQ(delegate_->GetConsentLevelForPrimaryAccount(), ConsentLevel::kSync);
-
-  CoreAccountId primary_account =
-      identity_manager_->GetPrimaryAccountId(ConsentLevel::kSync);
-  // Revoke secondary tokens.
-  RevokeAllSecondaryTokens(
-      identity_manager_, AccountReconcilorDelegate::RevokeTokenOption::kRevoke,
-      primary_account,
-      signin_metrics::SourceForRefreshTokenOperation::
-          kAccountReconcilor_GaiaCookiesDeletedByUser);
-  if (primary_account.empty())
-    return;
-  if (identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
-          primary_account) ||
-      synced_data_deletion_in_progress_count_ == 0) {
-    // Invalidate the primary token, but do not revoke it.
-    auto* accounts_mutator = identity_manager_->GetAccountsMutator();
-    accounts_mutator->InvalidateRefreshTokenForPrimaryAccount(
-        signin_metrics::SourceForRefreshTokenOperation::
-            kAccountReconcilor_GaiaCookiesDeletedByUser);
-  }
+  delegate_->OnAccountsCookieDeletedByUserAction(
+      synced_data_deletion_in_progress_count_ != 0);
 }
 
 std::vector<CoreAccountId>
@@ -877,7 +796,6 @@ bool AccountReconcilor::CookieNeedsUpdate(
               gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER);
     return true;
   }
-
   if (parameters.mode ==
           gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
       !existing_accounts.empty() && !parameters.accounts_to_send.empty() &&

components/signin/core/browser/account_reconcilor_delegate.cc

@@ -51,12 +51,10 @@ AccountReconcilorDelegate::CalculateParametersForMultilogin(
   return {mode, accounts_to_send};
 }
 
-bool AccountReconcilorDelegate::ShouldRevokeTokensBeforeMultilogin(
+bool AccountReconcilorDelegate::RevokeSecondaryTokensBeforeMultiloginIfNeeded(
     const std::vector<CoreAccountId>& chrome_accounts,
-    const CoreAccountId& primary_account,
     const std::vector<gaia::ListedAccount>& gaia_accounts,
-    bool first_execution,
-    bool primary_has_error) const {
+    bool first_execution) {
   return false;
 }
 
@@ -177,15 +175,11 @@ AccountReconcilorDelegate::GetChromeAccountsForReconcile(
   return std::vector<CoreAccountId>();
 }
 
-AccountReconcilorDelegate::RevokeTokenOption
-AccountReconcilorDelegate::ShouldRevokeSecondaryTokensBeforeReconcile(
-    const std::vector<gaia::ListedAccount>& gaia_accounts) {
-  return RevokeTokenOption::kDoNotRevoke;
+void AccountReconcilorDelegate::RevokeSecondaryTokensBeforeReconcileIfNeeded() {
 }
 
-bool AccountReconcilorDelegate::ShouldRevokeTokensOnCookieDeleted() {
-  return false;
-}
+void AccountReconcilorDelegate::OnAccountsCookieDeletedByUserAction(
+    bool synced_data_deletion_in_progress) {}
 
 bool AccountReconcilorDelegate::ShouldRevokeTokensIfNoPrimaryAccount() const {
   return true;

components/signin/core/browser/account_reconcilor_delegate.h

@@ -22,17 +22,6 @@ namespace signin {
 // Base class for AccountReconcilorDelegate.
 class AccountReconcilorDelegate {
  public:
-  // Options for revoking refresh tokens.
-  enum class RevokeTokenOption {
-    // Do not revoke the token.
-    kDoNotRevoke,
-    // Revoke the token if it is in auth error state.
-    kRevokeIfInError,
-    // Revoke the token.
-    // TODO(droger): remove this when Dice is launched.
-    kRevoke
-  };
-
   AccountReconcilorDelegate();
   virtual ~AccountReconcilorDelegate();
 
@@ -59,28 +48,23 @@ class AccountReconcilorDelegate {
       bool first_execution,
       bool primary_has_error) const;
 
-  // Returns whether secondary accounts should be revoked for doing full logout.
-  // Used only for the Multilogin codepath.
-  virtual bool ShouldRevokeTokensBeforeMultilogin(
+  // Revokes secondary tokens if needed based on the platform.
+  // Returns whether tokens has been revoked.
+  virtual bool RevokeSecondaryTokensBeforeMultiloginIfNeeded(
       const std::vector<CoreAccountId>& chrome_accounts,
-      const CoreAccountId& primary_account,
       const std::vector<gaia::ListedAccount>& gaia_accounts,
-      bool first_execution,
-      bool primary_has_error) const;
+      bool first_execution);
 
-  // Returns whether secondary accounts should be revoked at the beginning of
-  // the reconcile.
-  virtual RevokeTokenOption ShouldRevokeSecondaryTokensBeforeReconcile(
-      const std::vector<gaia::ListedAccount>& gaia_accounts);
+  // Revokes secondary accounts if needed.
+  virtual void RevokeSecondaryTokensBeforeReconcileIfNeeded();
 
-  // Returns whether tokens should be revoked when the Gaia cookie has been
-  // explicitly deleted by the user.
-  // If this returns false, tokens will not be revoked. If this returns true,
-  // secondary tokens will be deleted ; and the primary token will be
-  // invalidated unless it has to be kept for critical Sync operations.
-  virtual bool ShouldRevokeTokensOnCookieDeleted();
+  // Called when cookies are deleted by user action.
+  // This might be a no-op or signout the profile or lead to a sync paused state
+  // based on different platforms conditions.
+  virtual void OnAccountsCookieDeletedByUserAction(
+      bool synced_data_deletion_in_progress);
 
-  // Returns whether tokens should be revoked when the primary account is empty
+  // Returns whether tokens should be revoked when the primary account is empty.
   virtual bool ShouldRevokeTokensIfNoPrimaryAccount() const;
 
   // Called when reconcile is finished.

components/signin/core/browser/account_reconcilor_unittest.cc

@@ -138,7 +138,8 @@ class DummyAccountReconcilorWithDelegate : public AccountReconcilor {
         return std::make_unique<signin::AccountReconcilorDelegate>();
       case signin::AccountConsistencyMethod::kDice:
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
-        return std::make_unique<signin::DiceAccountReconcilorDelegate>();
+        return std::make_unique<signin::DiceAccountReconcilorDelegate>(
+            identity_manager);
 #else
         NOTREACHED();
         return nullptr;