Revert "IdentityManager: Add APIs for querying state of refresh tokens"

This reverts commit cb05af1b.

Reason for revert: Causes a DCHECK followed by a crash. http://crbug.com/859697

Original change's description:
> IdentityManager: Add APIs for querying state of refresh tokens
> 
> This CL adds APIs to IdentityManager for obtaining the set of all
> accounts with refresh tokens and querying whether the primary account
> is available with a refresh token.
> 
> The design follows that of IdentityManager's caching of the primary
> account information:
> - IdentityManager initializes its state with the current state of
>   ProfileOAuth2TokenService.
> - IdentityManager updates this state in response to notifications from
>   PO2TS that an account's refresh token was updated/removed.
> 
> TBR=bsazonov@chromium.org
> 
> Bug: 806774
> Change-Id: Idc4a37a15ced2e7d32f012a50e4e0d1fdb1aecdf
> Reviewed-on: https://chromium-review.googlesource.com/1098668
> Commit-Queue: Colin Blundell <blundell@chromium.org>
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571451}

TBR=blundell@chromium.org,msarda@chromium.org,sdefresne@chromium.org,treib@chromium.org,bsazonov@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 806774, 859697
Change-Id: I636d458101a5a579202dc4683faddfc0643cb19c
Reviewed-on: https://chromium-review.googlesource.com/1123119Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572048}

chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java

@@ -156,14 +156,6 @@ public class OAuth2TokenServiceIntegrationTest {
             // Adding an observer should not lead to a callback.
             Assert.assertEquals(0, mObserver.getRevokedCallCount());
 
-            // First seed the account state (some native classes make the assumption that
-            // a notification that a token was revoked for a given account was preceded by a
-            // notifictation that that account was available).
-            mOAuth2TokenService.fireRefreshTokenAvailable(TEST_ACCOUNT1);
-            mOAuth2TokenService.fireRefreshTokenAvailable(TEST_ACCOUNT2);
-
-            Assert.assertEquals(2, mObserver.getAvailableCallCount());
-
             // An observer should be called with the correct account.
             mOAuth2TokenService.fireRefreshTokenRevoked(TEST_ACCOUNT1);
             Assert.assertEquals(1, mObserver.getRevokedCallCount());
@@ -174,9 +166,8 @@ public class OAuth2TokenServiceIntegrationTest {
             mOAuth2TokenService.fireRefreshTokenRevoked(TEST_ACCOUNT2);
             Assert.assertEquals(1, mObserver.getRevokedCallCount());
 
-            // No other observer interface method should have been called after the initial seeding
-            // of the accounts.
-            Assert.assertEquals(2, mObserver.getAvailableCallCount());
+            // No other observer interface method should ever have been called.
+            Assert.assertEquals(0, mObserver.getAvailableCallCount());
             Assert.assertEquals(0, mObserver.getLoadedCallCount());
         });
     }

services/identity/public/cpp/identity_manager.cc

@@ -14,20 +14,7 @@ IdentityManager::IdentityManager(SigninManagerBase* signin_manager,
     : signin_manager_(signin_manager),
       token_service_(token_service),
       account_tracker_service_(account_tracker_service) {
-  // Initialize the state of the primary account.
   primary_account_info_ = signin_manager_->GetAuthenticatedAccountInfo();
-
-  // 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);
-    DCHECK(!account_info.IsEmpty());
-    accounts_with_refresh_tokens_.emplace(std::move(account_id),
-                                          std::move(account_info));
-  }
-
   signin_manager_->AddObserver(this);
 #if !defined(OS_CHROMEOS)
   SigninManager::FromSigninManagerBase(signin_manager_)
@@ -96,29 +83,6 @@ bool IdentityManager::HasPrimaryAccount() {
   return !primary_account_info_.account_id.empty();
 }
 
-std::vector<AccountInfo> IdentityManager::GetAccountsWithRefreshTokens() {
-  // TODO(blundell): It seems wasteful to construct this vector every time this
-  // method is called, but it also seems bad to maintain the vector as an ivar
-  // along the map.
-  std::vector<AccountInfo> accounts;
-  accounts.reserve(accounts_with_refresh_tokens_.size());
-
-  for (const auto& pair : accounts_with_refresh_tokens_) {
-    accounts.push_back(pair.second);
-  }
-
-  return accounts;
-}
-
-bool IdentityManager::HasAccountWithRefreshToken(
-    const std::string& account_id) {
-  return base::ContainsKey(accounts_with_refresh_tokens_, account_id);
-}
-
-bool IdentityManager::HasPrimaryAccountWithRefreshToken() {
-  return HasAccountWithRefreshToken(GetPrimaryAccountInfo().account_id);
-}
-
 std::unique_ptr<AccessTokenFetcher>
 IdentityManager::CreateAccessTokenFetcherForAccount(
     const std::string& account_id,
@@ -233,20 +197,6 @@ void IdentityManager::WillFireOnRefreshTokenAvailable(
   AccountInfo account_info =
       account_tracker_service_->GetAccountInfo(account_id);
   DCHECK(!account_info.IsEmpty());
-
-  // Insert the account into |accounts_with_refresh_tokens_|.
-  auto insertion_result = accounts_with_refresh_tokens_.emplace(
-      account_id, std::move(account_info));
-
-  // 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.
-  if (!insertion_result.second) {
-    AccountInfo cached_account_info = insertion_result.first->second;
-    DCHECK_EQ(account_info.gaia, cached_account_info.gaia);
-    DCHECK_EQ(account_info.email, cached_account_info.email);
-  }
-
   for (auto& observer : observer_list_) {
     observer.OnRefreshTokenUpdatedForAccount(account_info, is_valid);
   }
@@ -257,11 +207,6 @@ void IdentityManager::WillFireOnRefreshTokenRevoked(
   AccountInfo account_info =
       account_tracker_service_->GetAccountInfo(account_id);
   DCHECK(!account_info.IsEmpty());
-
-  auto iterator = accounts_with_refresh_tokens_.find(account_id);
-  DCHECK(iterator != accounts_with_refresh_tokens_.end());
-  accounts_with_refresh_tokens_.erase(iterator);
-
   for (auto& observer : observer_list_) {
     observer.OnRefreshTokenRemovedForAccount(account_info);
   }

services/identity/public/cpp/identity_manager.h

@@ -120,20 +120,6 @@ class IdentityManager : public SigninManagerBase::Observer,
   // primary account info has a valid account ID.
   bool HasPrimaryAccount();
 
-  // Provides access to the latest cached information of all accounts that have
-  // refresh tokens.
-  // NOTE: The accounts should not be assumed to be in any particular order; in
-  // particular, they are not guaranteed to be in the order in which the
-  // refresh tokens were added.
-  std::vector<AccountInfo> GetAccountsWithRefreshTokens();
-
-  // Returns true if a refresh token exists for |account_id|.
-  bool HasAccountWithRefreshToken(const std::string& account_id);
-
-  // Returns true if (a) the primary account exists, and (b) a refresh token
-  // exists for the primary account.
-  bool HasPrimaryAccountWithRefreshToken();
-
   // Creates an AccessTokenFetcher given the passed-in information.
   std::unique_ptr<AccessTokenFetcher> CreateAccessTokenFetcherForAccount(
       const std::string& account_id,
@@ -232,10 +218,6 @@ class IdentityManager : public SigninManagerBase::Observer,
   // The latest (cached) value of the primary account.
   AccountInfo primary_account_info_;
 
-  // The latest (cached) value of the accounts with refresh tokens.
-  using AccountIDToAccountInfoMap = std::map<std::string, AccountInfo>;
-  AccountIDToAccountInfoMap accounts_with_refresh_tokens_;
-
   // Lists of observers.
   // Makes sure lists are empty on destruction.
   base::ObserverList<Observer, true> observer_list_;