Commit b30f7f82 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

IdentityManager: Remove caching of accounts with refresh tokens

This CL streamlines IdentityManager's interaction with
AccountTrackerService when firing updates for accounts with refresh
tokens. Rather than caching state locally in response to
PO2TS::DiagnosticsClient callbacks, IdentityManager simply obtains
the AccountInfo from AccountTracker in the PO2TS observer callback
itself. This CL eliminates the need for any cached state in
IdentityManager related to accounts with refresh tokens.

The motivation is to enable conversion of the codebase to
IdentityManager as smoothly and painlessly as possible, as explained in
detail in crbug.com/883722.

Bug: 883722
Change-Id: Ic13d5be5a5f17411c167d19da7f8444e6ec257bc
Reviewed-on: https://chromium-review.googlesource.com/1228197
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593142}
parent 13303bdb
......@@ -51,38 +51,9 @@ IdentityManager::IdentityManager(
token_service_(token_service),
account_tracker_service_(account_tracker_service),
gaia_cookie_manager_service_(gaia_cookie_manager_service) {
// Initialize the state of accounts with refresh tokens.
// |account_id| is moved into |accounts_with_refresh_tokens_|.
// Do not change this to "const std::string&".
for (std::string account_id : token_service->GetAccounts()) {
AccountInfo account_info =
account_tracker_service_->GetAccountInfo(account_id);
// In the context of supervised users, the ProfileOAuth2TokenService is used
// without the AccountTrackerService being used. This is the only case in
// which the AccountTrackerService will potentially not know about the
// account. In this context, |account_id| is always set to
// kSupervisedUserPseudoEmail.
// TODO(860492): Remove this special case once supervised user support is
// removed.
DCHECK(!account_info.IsEmpty() || account_id == kSupervisedUserPseudoEmail);
if (account_id == kSupervisedUserPseudoEmail && account_info.IsEmpty()) {
// Populate the information manually to maintain the invariant that the
// account ID, gaia ID, and email are always set.
account_info.account_id = account_id;
account_info.email = kSupervisedUserPseudoEmail;
account_info.gaia = kSupervisedUserPseudoGaiaID;
}
accounts_with_refresh_tokens_.emplace(std::move(account_id),
std::move(account_info));
}
signin_manager_->AddObserver(this);
token_service_->AddDiagnosticsObserver(this);
token_service_->AddObserver(this);
token_service_->set_diagnostics_client(this);
gaia_cookie_manager_service_->AddObserver(this);
}
......@@ -90,7 +61,6 @@ IdentityManager::~IdentityManager() {
signin_manager_->RemoveObserver(this);
token_service_->RemoveObserver(this);
token_service_->RemoveDiagnosticsObserver(this);
token_service_->set_diagnostics_client(nullptr);
gaia_cookie_manager_service_->RemoveObserver(this);
}
......@@ -248,10 +218,7 @@ void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) {
}
}
void IdentityManager::WillFireOnRefreshTokenAvailable(
const std::string& account_id,
bool is_valid) {
DCHECK(!pending_token_available_state_);
void IdentityManager::OnRefreshTokenAvailable(const std::string& account_id) {
AccountInfo account_info =
account_tracker_service_->GetAccountInfo(account_id);
......@@ -271,94 +238,25 @@ void IdentityManager::WillFireOnRefreshTokenAvailable(
account_info.gaia = kSupervisedUserPseudoGaiaID;
}
// The account might have already been present (e.g., this method can fire on
// updating an invalid token to a valid one or vice versa); in this case we
// sanity-check that the cached account info has the expected values.
auto iterator = accounts_with_refresh_tokens_.find(account_id);
if (iterator != accounts_with_refresh_tokens_.end()) {
DCHECK_EQ(account_info.gaia, iterator->second.gaia);
DCHECK(gaia::AreEmailsSame(account_info.email, iterator->second.email));
} else {
auto insertion_result = accounts_with_refresh_tokens_.emplace(
account_id, std::move(account_info));
DCHECK(insertion_result.second);
iterator = insertion_result.first;
}
PendingTokenAvailableState pending_token_available_state;
pending_token_available_state.account_info = iterator->second;
pending_token_available_state.refresh_token_is_valid = is_valid;
pending_token_available_state_ = std::move(pending_token_available_state);
}
void IdentityManager::WillFireOnRefreshTokenRevoked(
const std::string& account_id) {
DCHECK(!pending_token_revoked_info_);
auto iterator = accounts_with_refresh_tokens_.find(account_id);
if (iterator == accounts_with_refresh_tokens_.end()) {
// A corner case exists wherein the token service revokes tokens while
// loading tokens during initial startup. This is the only case in which it
// is expected that we could receive this notification without having
// previously received a notification that this account was available. In
// this case, we simply do not forward on the notification, for the
// following reasons: (1) We may not have a fully-populated |account_info|
// to send as the argument. (2) Sending the notification breaks clients'
// expectations that IdentityManager will only fire RefreshTokenRemoved
// notifications for accounts that it previously knew about.
DCHECK(!token_service_->AreAllCredentialsLoaded());
return;
}
accounts_with_refresh_tokens_.erase(iterator);
pending_token_revoked_info_ =
account_tracker_service_->GetAccountInfo(account_id);
// In the context of supervised users, the ProfileOAuth2TokenService is used
// without the AccountTrackerService being used. This is the only case in
// which the AccountTrackerService will potentially not know about the
// account. In this context, |account_id| is always set to
// kSupervisedUserPseudoEmail.
// TODO(860492): Remove this special case once supervised user support is
// removed.
DCHECK(!pending_token_revoked_info_->IsEmpty() ||
account_id == kSupervisedUserPseudoEmail);
if (account_id == kSupervisedUserPseudoEmail &&
pending_token_revoked_info_->IsEmpty()) {
// Populate the information manually to maintain the invariant that the
// account ID, gaia ID, and email are always set.
pending_token_revoked_info_->account_id = account_id;
pending_token_revoked_info_->email = account_id;
pending_token_revoked_info_->gaia = kSupervisedUserPseudoGaiaID;
// Compute the validity of the new refresh token: PO2TS sets an account's
// refresh token to be invalid (error CREDENTIALS_REJECTED_BY_CLIENT) if the
// user signs out of that account on the web.
// TODO(blundell): Hide this logic inside PO2TS.
bool is_valid = true;
GoogleServiceAuthError token_error = token_service_->GetAuthError(account_id);
if (token_error == GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_CLIENT)) {
is_valid = false;
}
}
void IdentityManager::OnRefreshTokenAvailable(const std::string& account_id) {
DCHECK(pending_token_available_state_);
DCHECK_EQ(pending_token_available_state_->account_info.account_id,
account_id);
// Move the state out of |pending_token_available_state_| in case any observer
// callbacks fired below result in mutations of refresh tokens.
AccountInfo account_info =
std::move(pending_token_available_state_->account_info);
bool refresh_token_is_valid =
pending_token_available_state_->refresh_token_is_valid;
pending_token_available_state_.reset();
for (auto& observer : observer_list_) {
observer.OnRefreshTokenUpdatedForAccount(account_info,
refresh_token_is_valid);
observer.OnRefreshTokenUpdatedForAccount(account_info, is_valid);
}
}
void IdentityManager::OnRefreshTokenRevoked(const std::string& account_id) {
// TODO(883722): Remove this state entirely, as it now serves no purpose.
pending_token_revoked_info_.reset();
for (auto& observer : observer_list_) {
observer.OnRefreshTokenRemovedForAccount(account_id);
}
......
......@@ -43,7 +43,6 @@ namespace identity {
// Gives access to information about the user's Google identities. See
// ./README.md for detailed documentation.
class IdentityManager : public SigninManagerBase::Observer,
public ProfileOAuth2TokenService::DiagnosticsClient,
public OAuth2TokenService::DiagnosticsObserver,
public OAuth2TokenService::Observer,
public GaiaCookieManagerService::Observer {
......@@ -207,11 +206,6 @@ class IdentityManager : public SigninManagerBase::Observer,
void RemoveDiagnosticsObserver(DiagnosticsObserver* observer);
private:
struct PendingTokenAvailableState {
AccountInfo account_info;
bool refresh_token_is_valid = false;
};
// These clients need to call SetPrimaryAccountSynchronouslyForTests().
friend AccountInfo SetPrimaryAccount(SigninManagerBase* signin_manager,
IdentityManager* identity_manager,
......@@ -247,11 +241,6 @@ class IdentityManager : public SigninManagerBase::Observer,
void GoogleSigninSucceeded(const AccountInfo& account_info) override;
void GoogleSignedOut(const AccountInfo& account_info) override;
// ProfileOAuth2TokenService::DiagnosticsClient:
void WillFireOnRefreshTokenAvailable(const std::string& account_id,
bool is_valid) override;
void WillFireOnRefreshTokenRevoked(const std::string& account_id) override;
// OAuth2TokenService::Observer:
void OnRefreshTokenAvailable(const std::string& account_id) override;
void OnRefreshTokenRevoked(const std::string& account_id) override;
......@@ -277,17 +266,6 @@ class IdentityManager : public SigninManagerBase::Observer,
AccountTrackerService* account_tracker_service_;
GaiaCookieManagerService* gaia_cookie_manager_service_;
// The latest (cached) value of the accounts with refresh tokens.
using AccountIDToAccountInfoMap = std::map<std::string, AccountInfo>;
AccountIDToAccountInfoMap accounts_with_refresh_tokens_;
// Info that is cached from the PO2TS::DiagnosticsClient callbacks in order to
// forward on to the observers of this class in the corresponding
// O2TS::Observer callbacks (the information is not directly available at the
// time of receiving the O2TS::Observer callbacks).
base::Optional<PendingTokenAvailableState> pending_token_available_state_;
base::Optional<AccountInfo> pending_token_revoked_info_;
// Lists of observers.
// Makes sure lists are empty on destruction.
base::ObserverList<Observer, true>::Unchecked observer_list_;
......
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