AccessTokenFetcher: Add ability to wait for refresh token availability

Sync would like to use one AccessTokenFetcher instance to
fetch access tokens both for the primary account and for secondary
accounts. For the primary account, they would like to wait for the
refresh token to be available. In order to support this use case,
AccessTokenFetcher needs a Mode parameter. This CL adds that Mode
parameter.

Note that this Mode parameter is *not identical* semantically to the
Mode parameter of PrimaryAccountAccessTokenFetcher: the former's
|kWaitUntilAvailable| mode makes an access token request once there
is an account that is both (a) primary and (b) has a refresh token
available, while the latter's new |kWaitUntilRefreshTokenAvailable|
mode simply waits until there is a refresh token available for the
given account (which may no longer be primary at that point, even if
it was at the time of creating the AccessTokenFetcher). This semantic
distinction is called out explicitly in the comments on
PrimaryAccountAccessTokenFetcher::Mode. Sync has indicated that the
AccessTokenFetcher semantics suffice for their use case.

Bug: 840703, 729547
Change-Id: Iff39a24ef74945b3e9fa22ad961c3cd73a401e51
Reviewed-on: https://chromium-review.googlesource.com/1158831
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582535}

components/gcm_driver/account_tracker.cc

@@ -226,7 +226,8 @@ void AccountIdFetcher::Start() {
   access_token_fetcher_ = identity_manager_->CreateAccessTokenFetcherForAccount(
       account_key_, "gaia_account_tracker", scopes,
       base::BindOnce(&AccountIdFetcher::AccessTokenFetched,
-                     base::Unretained(this)));
+                     base::Unretained(this)),
+      identity::AccessTokenFetcher::Mode::kImmediate);
 }
 
 void AccountIdFetcher::AccessTokenFetched(

components/gcm_driver/gcm_account_tracker.cc

@@ -307,7 +307,8 @@ void GCMAccountTracker::GetToken(AccountInfos::iterator& account_iter) {
           account_iter->first, kGCMAccountTrackerName, scopes,
           base::BindOnce(
               &GCMAccountTracker::OnAccessTokenFetchCompleteForAccount,
-              base::Unretained(this), account_iter->first));
+              base::Unretained(this), account_iter->first),
+          identity::AccessTokenFetcher::Mode::kImmediate);
 
   DCHECK(pending_token_requests_.count(account_iter->first) == 0);
   pending_token_requests_.emplace(account_iter->first,

components/invalidation/impl/profile_identity_provider.cc

@@ -43,7 +43,8 @@ AccessTokenFetcherAdaptor::AccessTokenFetcherAdaptor(
   access_token_fetcher_ = identity_manager->CreateAccessTokenFetcherForAccount(
       active_account_id, oauth_consumer_name, scopes,
       base::BindOnce(&AccessTokenFetcherAdaptor::HandleTokenRequestCompletion,
-                     base::Unretained(this)));
+                     base::Unretained(this)),
+      identity::AccessTokenFetcher::Mode::kImmediate);
 }
 
 void AccessTokenFetcherAdaptor::HandleTokenRequestCompletion(

services/identity/public/cpp/access_token_fetcher.cc

@@ -15,12 +15,46 @@ AccessTokenFetcher::AccessTokenFetcher(
     const std::string& oauth_consumer_name,
     OAuth2TokenService* token_service,
     const OAuth2TokenService::ScopeSet& scopes,
-    TokenCallback callback)
+    TokenCallback callback,
+    Mode mode)
     : OAuth2TokenService::Consumer(oauth_consumer_name),
       account_id_(account_id),
       token_service_(token_service),
       scopes_(scopes),
-      callback_(std::move(callback)) {
+      mode_(mode),
+      callback_(std::move(callback)),
+      token_service_observer_(this) {
+  if (mode_ == Mode::kImmediate || IsRefreshTokenAvailable()) {
+    StartAccessTokenRequest();
+    return;
+  }
+
+  // Start observing the IdentityManager. This observer will be removed either
+  // when a refresh token is obtained and an access token request is started or
+  // when this object is destroyed.
+  token_service_observer_.Add(token_service_);
+}
+
+AccessTokenFetcher::~AccessTokenFetcher() {}
+
+bool AccessTokenFetcher::IsRefreshTokenAvailable() const {
+  DCHECK_EQ(Mode::kWaitUntilRefreshTokenAvailable, mode_);
+
+  return token_service_->RefreshTokenIsAvailable(account_id_);
+}
+
+void AccessTokenFetcher::StartAccessTokenRequest() {
+  DCHECK(mode_ == Mode::kImmediate || IsRefreshTokenAvailable());
+
+  // By the time of starting an access token request, we should no longer be
+  // listening for signin-related events.
+  DCHECK(!token_service_observer_.IsObserving(token_service_));
+
+  // Note: We might get here even in cases where we know that there's no refresh
+  // token. We're requesting an access token anyway, so that the token service
+  // will generate an appropriate error code that we can return to the client.
+  DCHECK(!access_token_request_);
+
   // TODO(843510): Consider making the request to ProfileOAuth2TokenService
   // asynchronously once there are no direct clients of PO2TS (i.e., PO2TS is
   // used only by this class and IdentityManager).
@@ -28,7 +62,17 @@ AccessTokenFetcher::AccessTokenFetcher(
       token_service_->StartRequest(account_id_, scopes_, this);
 }
 
-AccessTokenFetcher::~AccessTokenFetcher() {}
+void AccessTokenFetcher::OnRefreshTokenAvailable(
+    const std::string& account_id) {
+  DCHECK_EQ(Mode::kWaitUntilRefreshTokenAvailable, mode_);
+
+  if (!IsRefreshTokenAvailable())
+    return;
+
+  token_service_observer_.Remove(token_service_);
+
+  StartAccessTokenRequest();
+}
 
 void AccessTokenFetcher::OnGetTokenSuccess(
     const OAuth2TokenService::Request* request,

services/identity/public/cpp/access_token_fetcher.h

@@ -10,6 +10,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 #include "services/identity/public/cpp/access_token_info.h"
@@ -22,6 +23,14 @@ namespace identity {
 class AccessTokenFetcher : public OAuth2TokenService::Observer,
                            public OAuth2TokenService::Consumer {
  public:
+  // Specifies how this instance should behave:
+  // |kImmediate|: Makes one-shot immediate request.
+  // |kWaitUntilRefreshTokenAvailable|: Waits for the account to have a refresh
+  // token before making the request.
+  // Note that using |kWaitUntilRefreshTokenAvailable| can result in waiting
+  // forever if the user is not signed in and doesn't sign in.
+  enum class Mode { kImmediate, kWaitUntilRefreshTokenAvailable };
+
   // Callback for when a request completes (successful or not). On successful
   // requests, |error| is NONE and |access_token_info| contains info of the
   // obtained OAuth2 access token. On failed requests, |error| contains the
@@ -40,11 +49,21 @@ class AccessTokenFetcher : public OAuth2TokenService::Observer,
                      const std::string& oauth_consumer_name,
                      OAuth2TokenService* token_service,
                      const OAuth2TokenService::ScopeSet& scopes,
-                     TokenCallback callback);
+                     TokenCallback callback,
+                     Mode mode);
 
   ~AccessTokenFetcher() override;
 
  private:
+  // Returns true iff a refresh token is available for |account_id_|. Should
+  // only be called in mode |kWaitUntilAvailable|.
+  bool IsRefreshTokenAvailable() const;
+
+  void StartAccessTokenRequest();
+
+  // OAuth2TokenService::Observer implementation.
+  void OnRefreshTokenAvailable(const std::string& account_id) override;
+
   // OAuth2TokenService::Consumer implementation.
   void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
                          const std::string& access_token,
@@ -59,15 +78,19 @@ class AccessTokenFetcher : public OAuth2TokenService::Observer,
   void RunCallbackAndMaybeDie(GoogleServiceAuthError error,
                               AccessTokenInfo access_token_info);
 
-  std::string account_id_;
+  const std::string account_id_;
   OAuth2TokenService* token_service_;
-  OAuth2TokenService::ScopeSet scopes_;
+  const OAuth2TokenService::ScopeSet scopes_;
+  const Mode mode_;
 
   // NOTE: This callback should only be invoked from |RunCallbackAndMaybeDie|,
   // as invoking it has the potential to destroy this object per this class's
   // contract.
   TokenCallback callback_;
 
+  ScopedObserver<OAuth2TokenService, AccessTokenFetcher>
+      token_service_observer_;
+
   std::unique_ptr<OAuth2TokenService::Request> access_token_request_;
 
   DISALLOW_COPY_AND_ASSIGN(AccessTokenFetcher);

services/identity/public/cpp/identity_manager.cc

@@ -220,10 +220,11 @@ IdentityManager::CreateAccessTokenFetcherForAccount(
     const std::string& account_id,
     const std::string& oauth_consumer_name,
     const OAuth2TokenService::ScopeSet& scopes,
-    AccessTokenFetcher::TokenCallback callback) {
+    AccessTokenFetcher::TokenCallback callback,
+    AccessTokenFetcher::Mode mode) {
   return std::make_unique<AccessTokenFetcher>(account_id, oauth_consumer_name,
                                               token_service_, scopes,
-                                              std::move(callback));
+                                              std::move(callback), mode);
 }
 
 void IdentityManager::RemoveAccessTokenFromCache(

services/identity/public/cpp/identity_manager.h

@@ -186,7 +186,8 @@ class IdentityManager : public SigninManagerBase::Observer,
       const std::string& account_id,
       const std::string& oauth_consumer_name,
       const OAuth2TokenService::ScopeSet& scopes,
-      AccessTokenFetcher::TokenCallback callback);
+      AccessTokenFetcher::TokenCallback callback,
+      AccessTokenFetcher::Mode mode);
 
   // If an entry exists in the Identity Service's cache corresponding to the
   // given information, removes that entry; in this case, the next access token

services/identity/public/cpp/identity_manager_unittest.cc

@@ -1173,7 +1173,8 @@ TEST_F(IdentityManagerTest, CreateAccessTokenFetcher) {
   std::unique_ptr<AccessTokenFetcher> token_fetcher =
       identity_manager()->CreateAccessTokenFetcherForAccount(
           identity_manager()->GetPrimaryAccountInfo().account_id,
-          "dummy_consumer", scopes, std::move(callback));
+          "dummy_consumer", scopes, std::move(callback),
+          AccessTokenFetcher::Mode::kImmediate);
   EXPECT_TRUE(token_fetcher);
 }
 
@@ -1192,7 +1193,8 @@ TEST_F(IdentityManagerTest, ObserveAccessTokenFetch) {
   std::unique_ptr<AccessTokenFetcher> token_fetcher =
       identity_manager()->CreateAccessTokenFetcherForAccount(
           identity_manager()->GetPrimaryAccountInfo().account_id,
-          "dummy_consumer", scopes, std::move(callback));
+          "dummy_consumer", scopes, std::move(callback),
+          AccessTokenFetcher::Mode::kImmediate);
 
   run_loop.Run();
 

services/identity/public/cpp/primary_account_access_token_fetcher.cc

@@ -55,12 +55,19 @@ void PrimaryAccountAccessTokenFetcher::StartAccessTokenRequest() {
   // will generate an appropriate error code that we can return to the client.
   DCHECK(!access_token_fetcher_);
 
+  // NOTE: This class does not utilize AccessTokenFetcher in its
+  // |kWaitUntilRefreshTokenAvailable| mode because the PAATF semantics specify
+  // that when used in *its* |kWaitUntilAvailable| mode, the access token
+  // request should be started when the account is primary AND has a refresh
+  // token available. AccessTokenFetcher used in
+  // |kWaitUntilRefreshTokenAvailable| mode would guarantee only the latter.
   access_token_fetcher_ = identity_manager_->CreateAccessTokenFetcherForAccount(
       identity_manager_->GetPrimaryAccountInfo().account_id,
       oauth_consumer_name_, scopes_,
       base::BindOnce(
           &PrimaryAccountAccessTokenFetcher::OnAccessTokenFetchComplete,
-          base::Unretained(this)));
+          base::Unretained(this)),
+      AccessTokenFetcher::Mode::kImmediate);
 }
 
 void PrimaryAccountAccessTokenFetcher::OnPrimaryAccountSet(

services/identity/public/cpp/primary_account_access_token_fetcher.h

@@ -28,7 +28,12 @@ class PrimaryAccountAccessTokenFetcher : public IdentityManager::Observer {
   // Specifies how this instance should behave:
   // |kImmediate|: Makes one-shot immediate request.
   // |kWaitUntilAvailable|: Waits for the primary account to be available
-  // before making the request.
+  // before making the request. In particular, "available" is defined as the
+  // moment when (a) there is a primary account and (b) that account has a
+  // refresh token. This semantics is richer than using an AccessTokenFetcher in
+  // kWaitUntilRefreshTokenAvailable mode, as the latter will make a request
+  // once the specified account has a refresh token, regardless of whether it's
+  // the primary account at that point.
   // Note that using |kWaitUntilAvailable| can result in waiting forever
   // if the user is not signed in and doesn't sign in.
   enum class Mode { kImmediate, kWaitUntilAvailable };