Commit 924db74d authored by Kush Sinha's avatar Kush Sinha Committed by Commit Bot

Remove usages of IdentityManager::LegacySeedAccountInfo

It was required that |IdentityManager::LegacySeedAccountInfo| be called
before calling |chromeos::AccountManager::UpsertAccount| because Chrome
OS Account Manager's |Observer|s needed to translate
|chromeos::AccountManager::AccountKey| to other formats (raw email and
canonical email) and relied on
|AccountTrackerService| / |AccountMapperUtil| for doing so.

This is no longer required:
  - Chrome OS Account Manager can now maintain the mapping from
    AccountKey to email id by itself (https://crrev.com/c/1452003) and
    can notify its |Observer|s about this information
    (https://crrev.com/c/1477691). All new account additions to Chrome
    OS Account Manager are now guaranteed to have an email id associated
    with them in Account Manager itself.
  - AccountMapperUtil has now been deleted (https://crrev.com/c/1483024).

Hence, remove the temporal coupling between
|IdentityManager::LegacySeedAccountInfo| and
|chromeos::AccountManager::UpsertAccount| in all production call-sites.
This seeding of AccountInfo will now be handled within
|ChromeOSOAuth2TokenServiceDelegate::OnTokenUpserted|, just after the
account gets added to |chromeos::AccountManager|.

Bug: 922026
Change-Id: I77cdaa93ef34fc400f00a09aee8d9d847ef0fc48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1495551Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638096}
parent e41113dd
......@@ -114,14 +114,6 @@ class AccountMigrationBaseStep : public AccountMigrationRunner::Step {
return;
}
// |IdentityManager::LegacySeedAccountInfo| must be called before
// |AccountManager::UpsertToken|. |AccountManager| observers will need to
// translate |AccountManager::AccountKey| to other formats using
// |IdentityManager| and hence |IdentityManager| should be updated first.
AccountInfo account_info;
account_info.email = email;
account_info.gaia = gaia_id;
identity_manager_->LegacySeedAccountInfo(account_info);
account_manager_->UpsertAccount(
AccountManager::AccountKey{
gaia_id, account_manager::AccountType::ACCOUNT_TYPE_GAIA},
......
......@@ -314,7 +314,7 @@ void ChromeOSOAuth2TokenServiceDelegate::OnTokenUpserted(
account_manager_->UpdateEmail(account.key, email);
}
std::string account_id = account_tracker_service_->PickAccountIdForAccount(
std::string account_id = account_tracker_service_->SeedAccountInfo(
account.key.id /* gaia_id */, email);
DCHECK(!account_id.empty());
......
......@@ -12,7 +12,6 @@
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/ui/webui/signin/inline_login_handler.h"
#include "chromeos/account_manager/account_manager.h"
#include "chromeos/account_manager/account_manager_factory.h"
......@@ -33,15 +32,13 @@ namespace {
class SigninHelper : public GaiaAuthConsumer {
public:
SigninHelper(
Profile* profile,
chromeos::AccountManager* account_manager,
const base::RepeatingClosure& close_dialog_closure,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const std::string& gaia_id,
const std::string& email,
const std::string& auth_code)
: profile_(profile),
account_manager_(account_manager),
: account_manager_(account_manager),
close_dialog_closure_(close_dialog_closure),
email_(email),
gaia_auth_fetcher_(this,
......@@ -57,22 +54,18 @@ class SigninHelper : public GaiaAuthConsumer {
// GaiaAuthConsumer overrides.
void OnClientOAuthSuccess(const ClientOAuthResult& result) override {
// TODO(sinhak): Do not depend on Profile unnecessarily. A Profile should
// call |IdentityManagerFactory| for the list of accounts it wants to
// pull from |AccountManager|, not the other way round. Remove this when we
// release multi Profile on Chrome OS and have the infra in place to do
// this.
// Account info needs to be seeded before the OAuth2TokenService chain can
// use it. Do this before anything else.
AccountInfo account_info;
account_info.gaia = account_key_.id;
account_info.email = email_;
// TODO(crbug.com/922026): SigninHelper and InlineLoginHandlerChromeOS
// must be refactored to remove this use of LegacySeedAccountInfo. It should
// be possible to replace these two calls with a call to
// |AccountsMutator::AddOrUpdateAccount()|.
IdentityManagerFactory::GetForProfile(profile_)->LegacySeedAccountInfo(
account_info);
// Flow of control after this call:
// |AccountManager::UpsertAccount| updates / inserts the account and calls
// its |Observer|s, one of which is |ChromeOSOAuth2TokenServiceDelegate|.
// |ChromeOSOAuth2TokenServiceDelegate::OnTokenUpserted| seeds the Gaia id
// and email id for this account in |AccountTrackerService| and invokes
// |FireRefreshTokenAvailable|. This causes the account to propagate
// throughout the Identity Service chain, including in
// |AccountFetcherService|. |AccountFetcherService::OnRefreshTokenAvailable|
// invokes |AccountTrackerService::StartTrackingAccount|, triggers a fetch
// for the account information from Gaia and updates this information into
// |AccountTrackerService|. At this point the account will be fully added to
// the system.
account_manager_->UpsertAccount(account_key_, email_, result.refresh_token);
close_dialog_closure_.Run();
......@@ -86,8 +79,6 @@ class SigninHelper : public GaiaAuthConsumer {
}
private:
// A non-owning pointer to Profile.
Profile* const profile_;
// A non-owning pointer to Chrome OS AccountManager.
chromeos::AccountManager* const account_manager_;
// A closure to close the hosting dialog window.
......@@ -147,7 +138,7 @@ void InlineLoginHandlerChromeOS::CompleteLogin(const std::string& email,
->GetAccountManager(profile->GetPath().value());
// SigninHelper deletes itself after its work is done.
new SigninHelper(profile, account_manager, close_dialog_closure_,
new SigninHelper(account_manager, close_dialog_closure_,
account_manager->GetUrlLoaderFactory(), gaia_id, email,
auth_code);
}
......
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