Commit f959ae2e authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Commit Bot

Remove primary_account_id_ property from PO2TSIOSDelegate

The sole reason for this property to exists is to prevent fetching
the list of accounts from SSO library before the primary account
has been set.

However, current design is that we want to load all accounts from
the SSO library before setting the primary account, so remove this
member variable whose purpose is no lost.

See design at: https://docs.google.com/document/d/1_NknywZB7UCQdCiCxEPQU85lXouid_aMxdzujEJSiEM/view

Bug: 957887
Change-Id: If0411f36a30c845951a75be745da3dd93e296050
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662548
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670034}
parent 901ad70b
...@@ -54,17 +54,6 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { ...@@ -54,17 +54,6 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
void ReloadAccountsFromSystem( void ReloadAccountsFromSystem(
const CoreAccountId& primary_account_id) override; const CoreAccountId& primary_account_id) override;
// Reloads accounts from the provider. Fires |OnRefreshTokenAvailable| for
// each new account. Fires |OnRefreshTokenRevoked| for each account that was
// removed.
// It expects that there is already a primary account id.
void ReloadCredentials();
// Sets the primary account and then reloads the accounts from the provider.
// Should be called when the user signs in to a new account.
// |primary_account_id| must not be an empty string.
void ReloadCredentials(const CoreAccountId& primary_account_id);
// Adds |account_id| to |accounts_| if it does not exist or udpates // Adds |account_id| to |accounts_| if it does not exist or udpates
// the auth error state of |account_id| if it exists. Fires // the auth error state of |account_id| if it exists. Fires
// |OnRefreshTokenAvailable| if the account info is updated. // |OnRefreshTokenAvailable| if the account info is updated.
...@@ -79,6 +68,14 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { ...@@ -79,6 +68,14 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
friend class ProfileOAuth2TokenServiceIOSDelegateTest; friend class ProfileOAuth2TokenServiceIOSDelegateTest;
FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest, FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
LoadRevokeCredentialsClearsExcludedAccounts); LoadRevokeCredentialsClearsExcludedAccounts);
FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
ReloadCredentials);
FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
ReloadCredentialsWithPrimaryAccountId);
FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
UpdateAuthErrorAfterRevokeCredentials);
FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
GetAuthError);
struct AccountStatus { struct AccountStatus {
GoogleServiceAuthError last_auth_error; GoogleServiceAuthError last_auth_error;
...@@ -88,12 +85,14 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { ...@@ -88,12 +85,14 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
// to information about the account. // to information about the account.
typedef std::map<CoreAccountId, AccountStatus> AccountStatusMap; typedef std::map<CoreAccountId, AccountStatus> AccountStatusMap;
// Reloads accounts from the provider. Fires |OnRefreshTokenAvailable| for
// each new account. Fires |OnRefreshTokenRevoked| for each account that was
// removed.
void ReloadCredentials();
// Clears exclude secondary accounts preferences. // Clears exclude secondary accounts preferences.
void ClearExcludedSecondaryAccounts(); void ClearExcludedSecondaryAccounts();
// The primary account id.
CoreAccountId primary_account_id_;
// Info about the existing accounts. // Info about the existing accounts.
AccountStatusMap accounts_; AccountStatusMap accounts_;
...@@ -103,7 +102,7 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate { ...@@ -103,7 +102,7 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
base::ThreadChecker thread_checker_; base::ThreadChecker thread_checker_;
// The client with which this instance was initialied, or NULL. // The client with which this instance was initialied, or NULL.
SigninClient* client_; SigninClient* client_ = nullptr;
std::unique_ptr<ProfileOAuth2TokenServiceIOSProvider> provider_; std::unique_ptr<ProfileOAuth2TokenServiceIOSProvider> provider_;
AccountTrackerService* account_tracker_service_; AccountTrackerService* account_tracker_service_;
......
...@@ -186,27 +186,14 @@ void ProfileOAuth2TokenServiceIOSDelegate::LoadCredentials( ...@@ -186,27 +186,14 @@ void ProfileOAuth2TokenServiceIOSDelegate::LoadCredentials(
return; return;
} }
ReloadCredentials(primary_account_id); ReloadCredentials();
set_load_credentials_state(LOAD_CREDENTIALS_FINISHED_WITH_SUCCESS); set_load_credentials_state(LOAD_CREDENTIALS_FINISHED_WITH_SUCCESS);
FireRefreshTokensLoaded(); FireRefreshTokensLoaded();
} }
void ProfileOAuth2TokenServiceIOSDelegate::ReloadCredentials(
const CoreAccountId& primary_account_id) {
DCHECK(!primary_account_id.empty());
DCHECK(primary_account_id_.empty() ||
primary_account_id_ == primary_account_id);
primary_account_id_ = primary_account_id;
ReloadCredentials();
}
void ProfileOAuth2TokenServiceIOSDelegate::ReloadCredentials() { void ProfileOAuth2TokenServiceIOSDelegate::ReloadCredentials() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
if (primary_account_id_.empty()) {
// Avoid loading the credentials if there is no primary account id.
return;
}
// Get the list of new account ids. // Get the list of new account ids.
std::set<std::string> new_account_ids; std::set<std::string> new_account_ids;
...@@ -266,7 +253,6 @@ void ProfileOAuth2TokenServiceIOSDelegate::RevokeAllCredentials() { ...@@ -266,7 +253,6 @@ void ProfileOAuth2TokenServiceIOSDelegate::RevokeAllCredentials() {
RemoveAccount(accountStatus.first); RemoveAccount(accountStatus.first);
DCHECK_EQ(0u, accounts_.size()); DCHECK_EQ(0u, accounts_.size());
primary_account_id_ = CoreAccountId();
ClearExcludedSecondaryAccounts(); ClearExcludedSecondaryAccounts();
} }
...@@ -277,10 +263,8 @@ void ProfileOAuth2TokenServiceIOSDelegate::AddAccountFromSystem( ...@@ -277,10 +263,8 @@ void ProfileOAuth2TokenServiceIOSDelegate::AddAccountFromSystem(
void ProfileOAuth2TokenServiceIOSDelegate::ReloadAccountsFromSystem( void ProfileOAuth2TokenServiceIOSDelegate::ReloadAccountsFromSystem(
const CoreAccountId& primary_account_id) { const CoreAccountId& primary_account_id) {
if (primary_account_id.empty()) DCHECK(!primary_account_id.empty());
return; ReloadCredentials();
ReloadCredentials(primary_account_id);
} }
OAuth2AccessTokenFetcher* OAuth2AccessTokenFetcher*
......
...@@ -200,27 +200,11 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, ReloadCredentials) { ...@@ -200,27 +200,11 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, ReloadCredentials) {
oauth2_delegate_->RefreshTokenIsAvailable(GetAccountId(account4))); oauth2_delegate_->RefreshTokenIsAvailable(GetAccountId(account4)));
} }
TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
ReloadCredentialsIgnoredIfNoPrimaryAccountId) {
ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x");
ProviderAccount account2 = fake_provider_->AddAccount("gaia_2", "email_2@x");
oauth2_delegate_->ReloadCredentials();
EXPECT_EQ(0, token_available_count_);
EXPECT_EQ(0, tokens_loaded_count_);
EXPECT_EQ(0, token_revoked_count_);
EXPECT_EQ(0U, oauth2_delegate_->GetAccounts().size());
EXPECT_FALSE(
oauth2_delegate_->RefreshTokenIsAvailable(GetAccountId(account1)));
EXPECT_FALSE(
oauth2_delegate_->RefreshTokenIsAvailable(GetAccountId(account2)));
}
TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
ReloadCredentialsWithPrimaryAccountId) { ReloadCredentialsWithPrimaryAccountId) {
ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x"); ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x");
ProviderAccount account2 = fake_provider_->AddAccount("gaia_2", "email_2@x"); ProviderAccount account2 = fake_provider_->AddAccount("gaia_2", "email_2@x");
oauth2_delegate_->ReloadCredentials(GetAccountId(account1)); oauth2_delegate_->ReloadCredentials();
EXPECT_EQ(2, token_available_count_); EXPECT_EQ(2, token_available_count_);
EXPECT_EQ(0, tokens_loaded_count_); EXPECT_EQ(0, tokens_loaded_count_);
...@@ -285,7 +269,7 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, StartRequestFailure) { ...@@ -285,7 +269,7 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, StartRequestFailure) {
TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
UpdateAuthErrorAfterRevokeCredentials) { UpdateAuthErrorAfterRevokeCredentials) {
ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x"); ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x");
oauth2_delegate_->ReloadCredentials(GetAccountId(account1)); oauth2_delegate_->ReloadCredentials();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
ResetObserverCounts(); ResetObserverCounts();
...@@ -303,7 +287,7 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, ...@@ -303,7 +287,7 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, GetAuthError) { TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, GetAuthError) {
// Accounts have no error by default. // Accounts have no error by default.
ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x"); ProviderAccount account1 = fake_provider_->AddAccount("gaia_1", "email_1@x");
oauth2_delegate_->ReloadCredentials(GetAccountId(account1)); oauth2_delegate_->ReloadCredentials();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(), EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
oauth2_delegate_->GetAuthError("gaia_1")); oauth2_delegate_->GetAuthError("gaia_1"));
......
...@@ -198,7 +198,9 @@ class WebViewSyncControllerObserverBridge ...@@ -198,7 +198,9 @@ class WebViewSyncControllerObserverBridge
} }
- (void)reloadCredentials { - (void)reloadCredentials {
_identityManager->LegacyReloadAccountsFromSystem(); if (_currentIdentity != nil) {
_identityManager->LegacyReloadAccountsFromSystem();
}
} }
#pragma mark - Internal Methods #pragma mark - Internal Methods
......
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