[signin] Don't clear unconsented primary account on cancelling Sync dialog

The unconsented primary account doesn't necessarily change when the Sync
confirmation dialog is cancelled.
In particular this is the case when PrimaryAccountMutator::ClearPrimaryAccount
is called with RemoveAccountOptions::kKeepAll.

This CL stops resetting the unconsented primary account as part of the signout
process.
The unconsented primary account is recomputed anyway when GoogleSignedOut is
called.


Fixed: 1029701
Change-Id: Id46bb468952efe63170a13872519fcaded076cb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944862
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720502}

components/signin/internal/identity_manager/primary_account_manager.cc

@@ -327,7 +327,7 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
 
   const CoreAccountInfo account_info = GetAuthenticatedAccountInfo();
   client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain);
-  SetPrimaryAccountInternal(CoreAccountInfo(), /*consented_to_sync=*/false);
+  SetPrimaryAccountInternal(account_info, /*consented_to_sync=*/false);
 
   // Revoke all tokens before sending signed_out notification, because there
   // may be components that don't listen for token service events when the
@@ -351,10 +351,8 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
       break;
   }
 
-  for (Observer& observer : observers_) {
+  for (Observer& observer : observers_)
     observer.GoogleSignedOut(account_info);
-    observer.UnconsentedPrimaryAccountChanged(primary_account_info());
-  }
 }
 
 void PrimaryAccountManager::OnRefreshTokensLoaded() {

components/signin/internal/identity_manager/primary_account_manager_unittest.cc

@@ -154,13 +154,16 @@ TEST_F(PrimaryAccountManagerTest, SignOut) {
   EXPECT_FALSE(manager_->IsAuthenticated());
   EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
   EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
+  EXPECT_EQ(main_account_id,
+            manager_->GetUnconsentedPrimaryAccountInfo().account_id);
   // Should not be persisted anymore
   ShutDownManager();
   CreatePrimaryAccountManager();
   EXPECT_FALSE(manager_->IsAuthenticated());
   EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
   EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
-  EXPECT_EQ(CoreAccountInfo(), manager_->GetUnconsentedPrimaryAccountInfo());
+  EXPECT_EQ(main_account_id,
+            manager_->GetUnconsentedPrimaryAccountInfo().account_id);
 }
 
 TEST_F(PrimaryAccountManagerTest, SignOutRevoke) {
@@ -204,8 +207,9 @@ TEST_F(PrimaryAccountManagerTest, SignOutDiceNoRevoke) {
   std::vector<CoreAccountId> expected_tokens = {main_account_id,
                                                 other_account_id};
   EXPECT_EQ(expected_tokens, token_service_.GetAccounts());
-  EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(),
-            manager_->GetAuthenticatedAccountInfo());
+  // Unconsented primary account is not reset.
+  EXPECT_EQ(main_account_id,
+            manager_->GetUnconsentedPrimaryAccountInfo().account_id);
 }
 
 TEST_F(PrimaryAccountManagerTest, SignOutDiceWithError) {

components/signin/public/identity_manager/identity_manager.cc

@@ -505,7 +505,7 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const {
   // On ChromeOS and on mobile platforms, we support only the primary account as
   // the unconsented primary account. By this early return, we avoid an extra
   // request to GAIA that lists cookie accounts.
-  return base::nullopt;
+  return CoreAccountInfo();
 #else
   AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar();
 

components/signin/public/identity_manager/identity_manager_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/containers/flat_set.h"
 #include "base/run_loop.h"
+#include "base/scoped_observer.h"
 #include "base/stl_util.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
@@ -1558,6 +1559,47 @@ TEST_F(
   EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info);
 }
 
+TEST_F(IdentityManagerTest, UnconsentedPrimaryAccountNotChangedOnSignout) {
+  // Setup cookies and token for the main account.
+  CoreAccountInfo account_info;
+  account_info.email = kTestEmail;
+  account_info.gaia = kTestGaiaId;
+  account_info.account_id =
+      identity_manager()->PickAccountIdForAccount(kTestGaiaId, kTestEmail);
+  SetCookieAccounts(identity_manager(), test_url_loader_factory(),
+                    {{account_info.email, account_info.gaia}});
+  SetRefreshTokenForAccount(identity_manager(), account_info.account_id,
+                            "refresh-token");
+  EXPECT_EQ(account_info,
+            identity_manager()->GetUnconsentedPrimaryAccountInfo());
+  EXPECT_EQ(account_info, identity_manager()->GetPrimaryAccountInfo());
+  EXPECT_TRUE(identity_manager()->HasPrimaryAccountWithRefreshToken());
+
+  // Tests that OnUnconsentedPrimaryAccountChanged is never called.
+  class UnconsentedPrimaryAccountObserver : public IdentityManager::Observer {
+   public:
+    void OnUnconsentedPrimaryAccountChanged(
+        const CoreAccountInfo& unconsented_primary_account_info) override {
+      NOTREACHED();
+      called_ = true;
+    }
+    bool called_ = false;
+  };
+  UnconsentedPrimaryAccountObserver observer;
+  ScopedObserver<IdentityManager, IdentityManager::Observer> scoped_observer(
+      &observer);
+  scoped_observer.Add(identity_manager());
+  // Clear primary account but do not delete the account.
+  ClearPrimaryAccount(identity_manager(),
+                      ClearPrimaryAccountPolicy::KEEP_ALL_ACCOUNTS);
+  // Primary account is cleared, but unconsented account is not.
+  EXPECT_FALSE(identity_manager()->HasPrimaryAccount());
+  EXPECT_EQ(account_info,
+            identity_manager()->GetUnconsentedPrimaryAccountInfo());
+  // OnUnconsentedPrimaryAccountChanged was not fired.
+  EXPECT_FALSE(observer.called_);
+}
+
 TEST_F(IdentityManagerTest,
        UnconsentedPrimaryAccountTokenRevokedWithStaleCookies) {
   ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT);