[signin] Remove lingering invalid account when signing out from settings

To fix issues with SAML signout, the signout flow was changed to go
through Gaia (http://crbug.com/1068978). However, this does not work when
the account is already invalid (Gaia signout is a no-op in this case).

As a result, when signing out from an invalid account (i.e. in Sync
paused state), the account was not immediately removed from the device,
and would linger until the next reconcilor run.

This CL explicitly removes the account if it's invalid.

Fixed: 1114646
Change-Id: Ibd7b4466ecb7d0ac6951d6b30814ef2e5c521bb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346245Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796430}

chrome/browser/signin/e2e_tests/live_sign_in_test.cc

@@ -119,14 +119,13 @@ class SignInTestObserver : public IdentityManager::Observer,
       return;
     }
 
-    bool has_valid_primary_sync_account = HasValidPrimarySyncAccount();
     switch (primary_sync_account_wait_) {
       case PrimarySyncAccountWait::kWaitForAdded:
-        if (!has_valid_primary_sync_account)
+        if (!HasValidPrimarySyncAccount())
           return;
         break;
       case PrimarySyncAccountWait::kWaitForCleared:
-        if (has_valid_primary_sync_account)
+        if (identity_manager_->HasPrimaryAccount(signin::ConsentLevel::kSync))
           return;
         break;
       case PrimarySyncAccountWait::kNotWait:
@@ -179,6 +178,8 @@ class SignInTestObserver : public IdentityManager::Observer,
 };
 
 // Live tests for SignIn.
+// These tests can be run with:
+// browser_tests --gtest_filter=LiveSignInTest.* --run-live-tests --run-manual
 class LiveSignInTest : public signin::test::LiveTest {
  public:
   LiveSignInTest() = default;
@@ -409,6 +410,31 @@ IN_PROC_BROWSER_TEST_F(LiveSignInTest, MANUAL_TurnOffSync) {
   EXPECT_FALSE(identity_manager()->HasPrimaryAccount());
 }
 
+// In "Sync paused" state, when the primary account is invalid, turns off sync
+// from settings. Checks that the account is removed from Chrome.
+// Regression test for https://crbug.com/1114646
+IN_PROC_BROWSER_TEST_F(LiveSignInTest, MANUAL_TurnOffSyncWhenPaused) {
+  TestAccount test_account_1;
+  CHECK(GetTestAccountsUtil()->GetAccount("TEST_ACCOUNT_1", test_account_1));
+  TurnOnSync(test_account_1, 0);
+
+  // Get in sync paused state.
+  SignOutFromWeb();
+
+  const CoreAccountInfo& primary_account =
+      identity_manager()->GetPrimaryAccountInfo();
+  EXPECT_FALSE(primary_account.IsEmpty());
+  EXPECT_TRUE(gaia::AreEmailsSame(test_account_1.user, primary_account.email));
+  EXPECT_TRUE(sync_service()->IsSyncFeatureEnabled());
+  EXPECT_TRUE(
+      identity_manager()->HasAccountWithRefreshTokenInPersistentErrorState(
+          primary_account.account_id));
+
+  TurnOffSync();
+  EXPECT_TRUE(identity_manager()->GetAccountsWithRefreshTokens().empty());
+  EXPECT_FALSE(identity_manager()->HasPrimaryAccount());
+}
+
 // This test can pass. Marked as manual because it TIMED_OUT on Win7.
 // See crbug.com/1025335.
 // Signs in an account on the web. Goes to the Chrome settings to enable Sync

chrome/browser/ui/webui/settings/people_handler.cc

@@ -702,10 +702,13 @@ void PeopleHandler::HandleSignout(const base::ListValue* args) {
           delete_profile ? signin_metrics::SignoutDelete::DELETED
                          : signin_metrics::SignoutDelete::KEEPING;
 
-      // Do not remove the accounts: the Gaia logout tab will remove them in a
-      // better way (see http://crbug.com/1068978).
+      // Use ClearAccountsAction::kDefault: if the primary account is still
+      // valid, it will be removed by the Gaia logout tab
+      // (see http://crbug.com/1068978). If the account is already invalid, drop
+      // the token now (because it's already invalid on the web, so the Gaia
+      // logout tab won't affect it, see http://crbug.com/1114646).
       identity_manager->GetPrimaryAccountMutator()->ClearPrimaryAccount(
-          signin::PrimaryAccountMutator::ClearAccountsAction::kKeepAll,
+          signin::PrimaryAccountMutator::ClearAccountsAction::kDefault,
           signin_metrics::USER_CLICKED_SIGNOUT_SETTINGS, delete_metric);
     } else {
       DCHECK(!delete_profile)