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.

This CL is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1098668 with
the following addition:

In https://chromium-review.googlesource.com/c/chromium/src/+/1098668,
we made the assumption that when IdentityManager receives a
notification from ProfileOAuth2TokenService that a token will be
revoked, it had previously known about that account (either because
the account was available at IdentityManager startup or because
IdentityManager previously received a notification from PO2TS that a
refresh token had been made available for the account). However, as
demonstrated in the linked bug, it turns out that this assumption is
not valid in one case: while loading tokens during startup, PO2TS
sometimes revokes tokens without having previously made them available.
This violation of IdentityManager's assumption causes a crash.

It is not feasible at the current time to change this behavior on the
part of PO2TS, as AccountTrackerService should be made aware of these
events so that it removes the accounts. However, this situation poses
a challenge for IdentityManager: Should it forward on the callback to
its own observers or not? In this CL, we fix the crash and additionally
nail down the semantics that IdentityManager sends token removed
callbacks only for accounts of which it previously knew the existence.
The reasons for making this choice on the semantics are twofold:

(1) Sending an observer callback that a token was removed for an account
that was not previously present in
IdentityManager::GetAccountsWithRefreshTokens() seems like a violation of
the principle of least surprise.
(2) It's not clear that AccountTrackerService will always know about the
account in this case, in which case we wouldn't have the full information
to forward on in the observer callback.

This does mean that
IdentityManager::Observer::OnRefreshTokenRemovedForAccount() is not
strictly identical semantically to
ProfileOAuth2TokenService::Observer::OnRefreshTokenRevoked(). However, we
strongly believe that the only consumer that needs to know about
revocations in this corner case is AccountTrackerService, which will not
be converted to depend on IdentityManager as it lives below
IdentityManager.

TBR=bsazonov@chromium.org

Bug: 806774
Change-Id: I9f77525e26825ab418b15c7d1c2a8e4f8e3b4910
Reviewed-on: https://chromium-review.googlesource.com/1124320
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572185}

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

@@ -156,6 +156,14 @@ 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());
@@ -166,8 +174,9 @@ public class OAuth2TokenServiceIntegrationTest {
             mOAuth2TokenService.fireRefreshTokenRevoked(TEST_ACCOUNT2);
             Assert.assertEquals(1, mObserver.getRevokedCallCount());
 
-            // No other observer interface method should ever have been called.
-            Assert.assertEquals(0, mObserver.getAvailableCallCount());
+            // No other observer interface method should have been called after the initial seeding
+            // of the accounts.
+            Assert.assertEquals(2, mObserver.getAvailableCallCount());
             Assert.assertEquals(0, mObserver.getLoadedCallCount());
         });
     }

services/identity/public/cpp/identity_manager.cc

@@ -14,7 +14,20 @@ 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_)
@@ -83,6 +96,29 @@ 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,
@@ -197,6 +233,20 @@ 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);
   }
@@ -204,9 +254,27 @@ void IdentityManager::WillFireOnRefreshTokenAvailable(
 
 void IdentityManager::WillFireOnRefreshTokenRevoked(
     const std::string& account_id) {
+  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);
+
   AccountInfo account_info =
       account_tracker_service_->GetAccountInfo(account_id);
   DCHECK(!account_info.IsEmpty());
+
   for (auto& observer : observer_list_) {
     observer.OnRefreshTokenRemovedForAccount(account_info);
   }

services/identity/public/cpp/identity_manager.h

@@ -120,6 +120,20 @@ 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,
@@ -218,6 +232,10 @@ 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_;