Commit 5667a5f1 authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Commit Bot

Re-order method call in AuthenticationService::SignIn

On both Chrome on iOS and iOS WebView, the sign-in workflow consists
of the following operations performed in order:

1.  copying the information about the account from the SSO library
    to the IdentityManager via LegacySeedAccountInfo()

2.  setting the primary account information using the account_id
    acquired from the previous step

3.  synchronizing the list of accounts between the SSO library and
    the IdentityManager via LegacyReloadAccountsFromSystem()

However, AuthenticationService intermingled this with other steps
that are unrelated: clearing the kSigninShouldPromptForSigninAgain
preference and calling PrepareForFirstSyncSetup().

It would become possible to refactor the sign-in workflow on iOS so
that the legacy API calls are internalized into the Identity Service
implementation if those calls where not intermingled.

Looking at what they do, it is safe to move them before the call to
LegacySeedAccountInfo() because LegacySeedPrimaryAccount() does not
result in any callbacks to be invoked, and the implementation does
not depend on the system modified by those two calls.

Bug: 926940
Change-Id: Ie129675ae4a65e9b065885a205c6990468d1649a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569865Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651740}
parent 7a283644
......@@ -319,6 +319,9 @@ void AuthenticationService::SignIn(ChromeIdentity* identity,
->GetChromeIdentityService()
->IsValidIdentity(identity));
SetPromptForSignIn(false);
sync_setup_service_->PrepareForFirstSyncSetup();
// The account info needs to be seeded for the primary account id before
// signing in.
AccountInfo info;
......@@ -336,9 +339,6 @@ void AuthenticationService::SignIn(ChromeIdentity* identity,
if (!old_authenticated_account_id.empty())
CHECK_EQ(new_authenticated_account_id, old_authenticated_account_id);
SetPromptForSignIn(false);
sync_setup_service_->PrepareForFirstSyncSetup();
// Update the SigninManager with the new logged in identity.
auto* account_mutator = identity_manager_->GetPrimaryAccountMutator();
DCHECK(account_mutator);
......
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