Commit 46a15abd authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Fix DCHECKs in IdentityManager::GetPrimaryAccountInfo

On ChromeOS, IdentityManager::GetPrimaryAccountInfo DCHECKs that its
view of the primary account info matches the one in SigninManagerBase.
However, if the primary account's refresh token gets revoked, then the
account gets removed from the AccountTrackerService, and
SigninManagerBase will only remember the account's ID, but not the
full AccountInfo.
This CL updates the DCHECKs so that they only verify the account ID in
this case.

Bug: 825190, 806775
Change-Id: I6cbde518e064b590109ef8ca165a1106c7ebda83
Reviewed-on: https://chromium-review.googlesource.com/1025094
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553116}
parent c612844d
...@@ -604,10 +604,6 @@ IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, NoPrimaryAccount) { ...@@ -604,10 +604,6 @@ IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, NoPrimaryAccount) {
IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest,
PrimaryAccountHasNoRefreshToken) { PrimaryAccountHasNoRefreshToken) {
std::string primary_account_id = SignIn("primary@example.com"); std::string primary_account_id = SignIn("primary@example.com");
// TODO(treib,blundell): This breaks IdentityManager. RevokeCredentials causes
// the account to be removed from AccountTrackerService (via
// AccountFetcherService::OnRefreshTokenRevoked), which triggers DCHECKs in
// IdentityManager::GetPrimaryAccountInfo.
token_service_->RevokeCredentials(primary_account_id); token_service_->RevokeCredentials(primary_account_id);
EXPECT_TRUE(ExpectGetAccounts({})); EXPECT_TRUE(ExpectGetAccounts({}));
} }
...@@ -2327,7 +2323,6 @@ IN_PROC_BROWSER_TEST_F(OnSignInChangedEventTest, ...@@ -2327,7 +2323,6 @@ IN_PROC_BROWSER_TEST_F(OnSignInChangedEventTest,
AddExpectedEvent(api::identity::OnSignInChanged::Create(account_info, false)); AddExpectedEvent(api::identity::OnSignInChanged::Create(account_info, false));
// Revoke the refresh token and verify that the callback fires. // Revoke the refresh token and verify that the callback fires.
// TODO(treib,blundell): This breaks IdentityManager.
token_service_->RevokeCredentials(primary_account_id); token_service_->RevokeCredentials(primary_account_id);
EXPECT_FALSE(HasExpectedEvent()); EXPECT_FALSE(HasExpectedEvent());
...@@ -2344,7 +2339,6 @@ IN_PROC_BROWSER_TEST_F(OnSignInChangedEventTest, ...@@ -2344,7 +2339,6 @@ IN_PROC_BROWSER_TEST_F(OnSignInChangedEventTest,
std::string primary_account_id = SignIn("primary@example.com"); std::string primary_account_id = SignIn("primary@example.com");
AddExpectedEvent(api::identity::OnSignInChanged::Create(account_info, false)); AddExpectedEvent(api::identity::OnSignInChanged::Create(account_info, false));
// TODO(treib,blundell): This breaks IdentityManager.
token_service_->RevokeCredentials(primary_account_id); token_service_->RevokeCredentials(primary_account_id);
account_info.id = "gaia_id_for_primary@example.com"; account_info.id = "gaia_id_for_primary@example.com";
......
...@@ -41,13 +41,24 @@ AccountInfo IdentityManager::GetPrimaryAccountInfo() { ...@@ -41,13 +41,24 @@ AccountInfo IdentityManager::GetPrimaryAccountInfo() {
// where you are setting the authenticated account info in the SigninManager. // where you are setting the authenticated account info in the SigninManager.
// TODO(blundell): Add the API to do this once we hit the first case and // TODO(blundell): Add the API to do this once we hit the first case and
// document the API to use here. // document the API to use here.
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().account_id, DCHECK_EQ(signin_manager_->GetAuthenticatedAccountId(),
primary_account_info_.account_id); primary_account_info_.account_id);
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().gaia, #if DCHECK_IS_ON()
primary_account_info_.gaia); // Note: If the primary account's refresh token gets revoked, then the account
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().email, // gets removed from AccountTrackerService (via
primary_account_info_.email); // AccountFetcherService::OnRefreshTokenRevoked), and so SigninManager's
#endif // GetAuthenticatedAccountInfo is empty (even though
// GetAuthenticatedAccountId is NOT empty).
if (!signin_manager_->GetAuthenticatedAccountInfo().account_id.empty()) {
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().account_id,
primary_account_info_.account_id);
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().gaia,
primary_account_info_.gaia);
DCHECK_EQ(signin_manager_->GetAuthenticatedAccountInfo().email,
primary_account_info_.email);
}
#endif // DCHECK_IS_ON()
#endif // defined(OS_CHROMEOS)
return primary_account_info_; return primary_account_info_;
} }
......
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