Commit 16f7249b authored by David Roger's avatar David Roger Committed by Commit Bot

[signin] Support for empty accounts in MultiLoginParams in UPDATE mode

An empty account list in multilogin is not supported by Gaia.
In this CL we replace this by a Logout call,
which is the recommended way to do the same thing.

Change-Id: I61a3be41be35e27293efbe8cf7a9d3319a5a7d75
Reviewed-on: https://chromium-review.googlesource.com/c/1488874
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635537}
parent e70a035a
...@@ -126,13 +126,10 @@ bool AccountsNeedUpdate( ...@@ -126,13 +126,10 @@ bool AccountsNeedUpdate(
const signin::MultiloginParameters& parameters, const signin::MultiloginParameters& parameters,
const std::vector<gaia::ListedAccount>& existing_accounts) { const std::vector<gaia::ListedAccount>& existing_accounts) {
if (parameters.mode == if (parameters.mode ==
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER) { gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
// In UPDATE mode accounts_to_send are guaranteed to be not empty. !existing_accounts.empty() && !parameters.accounts_to_send.empty() &&
DCHECK(!parameters.accounts_to_send.empty()); existing_accounts[0].id != parameters.accounts_to_send[0]) {
if (existing_accounts.empty()) // In UPDATE mode update is needed if first accounts don't match.
return true;
// In UPDATE mode update is needed id syncing account is not first.
if (existing_accounts[0].id != parameters.accounts_to_send[0])
return true; return true;
} }
// Maybe some accounts in cookies are not valid and need refreshing. // Maybe some accounts in cookies are not valid and need refreshing.
...@@ -502,6 +499,16 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( ...@@ -502,6 +499,16 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
DCHECK(is_reconcile_started_); DCHECK(is_reconcile_started_);
if (AccountsNeedUpdate(parameters_for_multilogin, gaia_accounts)) { if (AccountsNeedUpdate(parameters_for_multilogin, gaia_accounts)) {
if (parameters_for_multilogin.mode ==
gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER &&
parameters_for_multilogin.accounts_to_send.empty()) {
// UPDATE mode does not support empty list of accounts, call logout
// instead.
PerformLogoutAllAccountsAction();
gaia_accounts.clear();
OnSetAccountsInCookieCompleted(GoogleServiceAuthError::AuthErrorNone());
DCHECK(!is_reconcile_started_);
} else {
// Reconcilor has to do some calls to gaia. is_reconcile_started_ is true // Reconcilor has to do some calls to gaia. is_reconcile_started_ is true
// and any StartReconcile() calls that are made in the meantime will be // and any StartReconcile() calls that are made in the meantime will be
// aborted until OnSetAccountsInCookieCompleted is called and // aborted until OnSetAccountsInCookieCompleted is called and
...@@ -509,7 +516,9 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( ...@@ -509,7 +516,9 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
set_accounts_in_progress_ = true; set_accounts_in_progress_ = true;
PerformSetCookiesAction(parameters_for_multilogin); PerformSetCookiesAction(parameters_for_multilogin);
DCHECK(is_reconcile_started_); DCHECK(is_reconcile_started_);
}
} else { } else {
// Nothing to do, accounts already match.
OnSetAccountsInCookieCompleted(GoogleServiceAuthError::AuthErrorNone()); OnSetAccountsInCookieCompleted(GoogleServiceAuthError::AuthErrorNone());
DCHECK(!is_reconcile_started_); DCHECK(!is_reconcile_started_);
} }
......
...@@ -223,6 +223,7 @@ class AccountReconcilor : public KeyedService, ...@@ -223,6 +223,7 @@ class AccountReconcilor : public KeyedService,
DelegateTimeoutIsNotCalled); DelegateTimeoutIsNotCalled);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
DelegateTimeoutIsNotCalledIfTimeoutIsNotReached); DelegateTimeoutIsNotCalledIfTimeoutIsNotReached);
FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, MultiloginLogout);
void set_timer_for_testing(std::unique_ptr<base::OneShotTimer> timer); void set_timer_for_testing(std::unique_ptr<base::OneShotTimer> timer);
......
...@@ -2797,3 +2797,45 @@ TEST_F(AccountReconcilorTest, LockDestructionOrder) { ...@@ -2797,3 +2797,45 @@ TEST_F(AccountReconcilorTest, LockDestructionOrder) {
DeleteReconcilor(); DeleteReconcilor();
// |lock| is destroyed after the reconcilor, this should not crash. // |lock| is destroyed after the reconcilor, this should not crash.
} }
// Checks that multilogin with empty list of accounts in UPDATE mode is changed
// into a Logout call.
TEST_F(AccountReconcilorTest, MultiloginLogout) {
// Delegate implementation always returning UPDATE mode with no accounts.
class MultiloginLogoutDelegate : public signin::AccountReconcilorDelegate {
bool IsReconcileEnabled() const override { return true; }
bool IsAccountConsistencyEnforced() const override { return true; }
std::vector<std::string> GetChromeAccountsForReconcile(
const std::vector<std::string>& chrome_accounts,
const std::string& primary_account,
const std::vector<gaia::ListedAccount>& gaia_accounts,
const gaia::MultiloginMode mode) const override {
return {};
}
gaia::MultiloginMode CalculateModeForReconcile(
const std::vector<gaia::ListedAccount>& gaia_accounts,
const std::string primary_account,
bool first_execution,
bool primary_has_error) const override {
return gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER;
}
};
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kUseMultiloginEndpoint);
MockAccountReconcilor* reconcilor =
GetMockReconcilor(std::make_unique<MultiloginLogoutDelegate>());
signin::SetListAccountsResponseOneAccount("user@gmail.com", "123456",
&test_url_loader_factory_);
// Logout call to Gaia.
EXPECT_CALL(*reconcilor, PerformLogoutAllAccountsAction());
// No multilogin call.
EXPECT_CALL(*reconcilor, PerformSetCookiesAction(testing::_)).Times(0);
reconcilor->StartReconcile();
ASSERT_TRUE(reconcilor->is_reconcile_started_);
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(reconcilor->is_reconcile_started_);
ASSERT_EQ(signin_metrics::ACCOUNT_RECONCILOR_OK, reconcilor->GetState());
}
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