Commit 35b7e3a2 authored by David Roger's avatar David Roger Committed by Commit Bot

[signin] Fix AccountReconcilorState AccountReconcilor::EnableReconcile

As per comments in:
https://chromium-review.googlesource.com/c/chromium/src/+/2001359/5/components/signin/core/browser/account_reconcilor.cc#452

There is a scenario where the reconcilor can be broken:
if EnableReconcile is called as the reconcilor is already running, the
state will be set to SCHEDULED, but is_reconcile_started_ remains true.
From there the reconcilor is in inconsistent state, and triggers a
DCHECK in FinishReconcileWithMultiloginEndpoint:
DCHECK_EQ(AccountReconcilorState::ACCOUNT_RECONCILOR_RUNNING, state_);

Change-Id: I4f7436f94b87b5e34b11b856d0b55bd880813877
Fixed: 1043651
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010795
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarKush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733400}
parent 2a2e0d5c
......@@ -278,10 +278,11 @@ void AccountReconcilor::Initialize(bool start_reconcile_if_tokens_available) {
}
void AccountReconcilor::EnableReconcile() {
SetState(AccountReconcilorState::ACCOUNT_RECONCILOR_SCHEDULED);
RegisterWithAllDependencies();
if (IsIdentityManagerReady())
StartReconcile();
else
SetState(AccountReconcilorState::ACCOUNT_RECONCILOR_SCHEDULED);
}
void AccountReconcilor::DisableReconcile(bool logout_all_accounts) {
......
......@@ -172,6 +172,8 @@ class AccountReconcilor : public KeyedService,
StartReconcileContentSettingsInvalidPattern);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorMirrorTest,
GetAccountsFromCookieSuccess);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorMirrorTest,
EnableReconcileWhileAlreadyRunning);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorMirrorTest,
GetAccountsFromCookieFailure);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorMirrorTest,
......
......@@ -1916,6 +1916,42 @@ TEST_F(AccountReconcilorMirrorTest, GetAccountsFromCookieSuccess) {
ASSERT_EQ(0u, accounts_in_cookie_jar_info.signed_out_accounts.size());
}
// Checks that calling EnableReconcile() while the reconcilor is already running
// doesn't have any effect. Regression test for https://crbug.com/1043651
TEST_F(AccountReconcilorMirrorTest, EnableReconcileWhileAlreadyRunning) {
AccountInfo account_info = ConnectProfileToAccount("user@gmail.com");
const CoreAccountId account_id = account_info.account_id;
signin::SetListAccountsResponseOneAccountWithParams(
{account_info.email, account_info.gaia, false /* valid */,
false /* signed_out */, true /* verified */},
&test_url_loader_factory_);
std::vector<CoreAccountId> accounts_to_send = {account_id};
const signin::MultiloginParameters params(
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER,
accounts_to_send);
EXPECT_CALL(*GetMockReconcilor(), PerformSetCookiesAction(params));
AccountReconcilor* reconcilor = GetMockReconcilor();
ASSERT_TRUE(reconcilor);
ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_SCHEDULED,
reconcilor->GetState());
reconcilor->StartReconcile();
EXPECT_EQ(signin_metrics::ACCOUNT_RECONCILOR_RUNNING, reconcilor->GetState());
reconcilor->EnableReconcile();
EXPECT_EQ(signin_metrics::ACCOUNT_RECONCILOR_RUNNING, reconcilor->GetState());
base::RunLoop().RunUntilIdle();
ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_RUNNING, reconcilor->GetState());
signin::AccountsInCookieJarInfo accounts_in_cookie_jar_info =
identity_test_env()->identity_manager()->GetAccountsInCookieJar();
ASSERT_TRUE(accounts_in_cookie_jar_info.accounts_are_fresh);
ASSERT_EQ(1u, accounts_in_cookie_jar_info.signed_in_accounts.size());
ASSERT_EQ(account_id, accounts_in_cookie_jar_info.signed_in_accounts[0].id);
ASSERT_EQ(0u, accounts_in_cookie_jar_info.signed_out_accounts.size());
}
TEST_F(AccountReconcilorMirrorTest, GetAccountsFromCookieFailure) {
ConnectProfileToAccount("user@gmail.com");
signin::SetListAccountsResponseWithUnexpectedServiceResponse(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment