Move StartRequest*() from OAuth2TokenService to OAuth2AccessTokenManager

This CL is a part of moving access token management to
OAuth2AccessTokenManager.

It moves StartRequest*() APIs which returns OAuth2TokenService::Request
except StartRequestForMultilogin(). With this CL, OAuth2TokenService
calls StartRequest*() APIs through OAuth2AccessTokenManager.

OAuth2AccessTokenManager gets OAuth2TokenServiceDelegate through
OAuth2TokenService. Once OAuth2AccessTokenManagerDelegate is ready,
it should be replaced with OAuth2AccessTokenManagerDelegate.

Bug: 967598
Change-Id: I33523c08560bfbb8169f0a197b424c190639bebb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1667244
Commit-Queue: Julie Jeongeun Kim <jkim@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670871}

google_apis/gaia/oauth2_access_token_manager.cc

@@ -4,12 +4,18 @@
 
 #include "google_apis/gaia/oauth2_access_token_manager.h"
 
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
+#include "google_apis/gaia/gaia_urls.h"
+#include "google_apis/gaia/oauth2_token_service_delegate.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 OAuth2AccessTokenManager::OAuth2AccessTokenManager(
-    OAuth2TokenService* token_service)
-    : token_service_(token_service) {
+    OAuth2TokenService* token_service,
+    OAuth2TokenServiceDelegate* delegate)
+    : token_service_(token_service), delegate_(delegate) {
   DCHECK(token_service_);
+  DCHECK(delegate_);
 }
 
 OAuth2AccessTokenManager::~OAuth2AccessTokenManager() = default;
@@ -24,6 +30,41 @@ void OAuth2AccessTokenManager::RemoveDiagnosticsObserver(
   diagnostics_observer_list_.RemoveObserver(observer);
 }
 
+std::unique_ptr<OAuth2TokenService::Request>
+OAuth2AccessTokenManager::StartRequest(
+    const CoreAccountId& account_id,
+    const OAuth2TokenService::ScopeSet& scopes,
+    OAuth2TokenService::Consumer* consumer) {
+  return StartRequestForClientWithContext(
+      account_id, delegate_->GetURLLoaderFactory(),
+      GaiaUrls::GetInstance()->oauth2_chrome_client_id(),
+      GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer);
+}
+
+std::unique_ptr<OAuth2TokenService::Request>
+OAuth2AccessTokenManager::StartRequestForClient(
+    const CoreAccountId& account_id,
+    const std::string& client_id,
+    const std::string& client_secret,
+    const OAuth2TokenService::ScopeSet& scopes,
+    OAuth2TokenService::Consumer* consumer) {
+  return StartRequestForClientWithContext(
+      account_id, delegate_->GetURLLoaderFactory(), client_id, client_secret,
+      scopes, consumer);
+}
+
+std::unique_ptr<OAuth2TokenService::Request>
+OAuth2AccessTokenManager::StartRequestWithContext(
+    const CoreAccountId& account_id,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    const OAuth2TokenService::ScopeSet& scopes,
+    OAuth2TokenService::Consumer* consumer) {
+  return StartRequestForClientWithContext(
+      account_id, url_loader_factory,
+      GaiaUrls::GetInstance()->oauth2_chrome_client_id(),
+      GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer);
+}
+
 void OAuth2AccessTokenManager::RegisterTokenResponse(
     const std::string& client_id,
     const CoreAccountId& account_id,
@@ -76,6 +117,71 @@ void OAuth2AccessTokenManager::ClearCacheForAccount(
   }
 }
 
+std::unique_ptr<OAuth2TokenService::Request>
+OAuth2AccessTokenManager::StartRequestForClientWithContext(
+    const CoreAccountId& account_id,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    const std::string& client_id,
+    const std::string& client_secret,
+    const OAuth2TokenService::ScopeSet& scopes,
+    OAuth2TokenService::Consumer* consumer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  std::unique_ptr<OAuth2TokenService::RequestImpl> request(
+      new OAuth2TokenService::RequestImpl(account_id, consumer));
+  for (auto& observer : diagnostics_observer_list_)
+    observer.OnAccessTokenRequested(account_id, consumer->id(), scopes);
+
+  if (!delegate_->RefreshTokenIsAvailable(account_id)) {
+    GoogleServiceAuthError error(GoogleServiceAuthError::USER_NOT_SIGNED_UP);
+
+    for (auto& observer : diagnostics_observer_list_) {
+      observer.OnFetchAccessTokenComplete(account_id, consumer->id(), scopes,
+                                          error, base::Time());
+    }
+
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&OAuth2TokenService::RequestImpl::InformConsumer,
+                       request->AsWeakPtr(), error,
+                       OAuth2AccessTokenConsumer::TokenResponse()));
+    return std::move(request);
+  }
+
+  OAuth2TokenService::RequestParameters request_parameters(client_id,
+                                                           account_id, scopes);
+  const OAuth2AccessTokenConsumer::TokenResponse* token_response =
+      GetCachedTokenResponse(request_parameters);
+  if (token_response && token_response->access_token.length()) {
+    InformConsumerWithCachedTokenResponse(token_response, request.get(),
+                                          request_parameters);
+  } else {
+    token_service_->FetchOAuth2Token(request.get(), account_id,
+                                     url_loader_factory, client_id,
+                                     client_secret, scopes);
+  }
+  return std::move(request);
+}
+
+void OAuth2AccessTokenManager::InformConsumerWithCachedTokenResponse(
+    const OAuth2AccessTokenConsumer::TokenResponse* cache_token_response,
+    OAuth2TokenService::RequestImpl* request,
+    const OAuth2TokenService::RequestParameters& request_parameters) {
+  DCHECK(cache_token_response && cache_token_response->access_token.length());
+  for (auto& observer : diagnostics_observer_list_) {
+    observer.OnFetchAccessTokenComplete(
+        request_parameters.account_id, request->GetConsumerId(),
+        request_parameters.scopes, GoogleServiceAuthError::AuthErrorNone(),
+        cache_token_response->expiration_time);
+  }
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&OAuth2TokenService::RequestImpl::InformConsumer,
+                     request->AsWeakPtr(),
+                     GoogleServiceAuthError(GoogleServiceAuthError::NONE),
+                     *cache_token_response));
+}
+
 bool OAuth2AccessTokenManager::RemoveCachedTokenResponse(
     const OAuth2TokenService::RequestParameters& request_parameters,
     const std::string& token_to_remove) {

google_apis/gaia/oauth2_access_token_manager.h

@@ -17,14 +17,47 @@ class OAuth2AccessTokenManager {
  public:
   // TODO(https://crbug.com/967598): Remove |token_service| parameter once
   // OAuth2AccessTokenManager fully manages access tokens independently of
-  // OAuth2TokenService.
-  explicit OAuth2AccessTokenManager(OAuth2TokenService* token_service);
+  // OAuth2TokenService and replace |delegate| with
+  // OAuth2AccessTokenManagerDelegate.
+  explicit OAuth2AccessTokenManager(OAuth2TokenService* token_service,
+                                    OAuth2TokenServiceDelegate* delegate);
   virtual ~OAuth2AccessTokenManager();
 
   // Add or remove observers of this token manager.
   void AddDiagnosticsObserver(AccessTokenDiagnosticsObserver* observer);
   void RemoveDiagnosticsObserver(AccessTokenDiagnosticsObserver* observer);
 
+  // Checks in the cache for a valid access token for a specified |account_id|
+  // and |scopes|, and if not found starts a request for an OAuth2 access token
+  // using the OAuth2 refresh token maintained by this instance for that
+  // |account_id|. The caller owns the returned Request.
+  // |scopes| is the set of scopes to get an access token for, |consumer| is
+  // the object that will be called back with results if the returned request
+  // is not deleted.
+  std::unique_ptr<OAuth2TokenService::Request> StartRequest(
+      const CoreAccountId& account_id,
+      const OAuth2TokenService::ScopeSet& scopes,
+      OAuth2TokenService::Consumer* consumer);
+
+  // This method does the same as |StartRequest| except it uses |client_id| and
+  // |client_secret| to identify OAuth client app instead of using
+  // Chrome's default values.
+  std::unique_ptr<OAuth2TokenService::Request> StartRequestForClient(
+      const CoreAccountId& account_id,
+      const std::string& client_id,
+      const std::string& client_secret,
+      const OAuth2TokenService::ScopeSet& scopes,
+      OAuth2TokenService::Consumer* consumer);
+
+  // This method does the same as |StartRequest| except it uses the
+  // URLLoaderfactory given by |url_loader_factory| instead of using the one
+  // returned by |GetURLLoaderFactory| implemented by the delegate.
+  std::unique_ptr<OAuth2TokenService::Request> StartRequestWithContext(
+      const CoreAccountId& account_id,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      const OAuth2TokenService::ScopeSet& scopes,
+      OAuth2TokenService::Consumer* consumer);
+
   // Add a new entry to the cache.
   void RegisterTokenResponse(
       const std::string& client_id,
@@ -54,6 +87,23 @@ class OAuth2AccessTokenManager {
 
   OAuth2TokenService::TokenCache& token_cache() { return token_cache_; }
 
+  // This method does the same as |StartRequestWithContext| except it
+  // uses |client_id| and |client_secret| to identify OAuth
+  // client app instead of using Chrome's default values.
+  std::unique_ptr<OAuth2TokenService::Request> StartRequestForClientWithContext(
+      const CoreAccountId& account_id,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      const std::string& client_id,
+      const std::string& client_secret,
+      const OAuth2TokenService::ScopeSet& scopes,
+      OAuth2TokenService::Consumer* consumer);
+
+  // Posts a task to fire the Consumer callback with the cached token response.
+  void InformConsumerWithCachedTokenResponse(
+      const OAuth2AccessTokenConsumer::TokenResponse* token_response,
+      OAuth2TokenService::RequestImpl* request,
+      const OAuth2TokenService::RequestParameters& client_scopes);
+
   // Removes an access token for the given set of scopes from the cache.
   // Returns true if the entry was removed, otherwise false.
   bool RemoveCachedTokenResponse(
@@ -68,6 +118,9 @@ class OAuth2AccessTokenManager {
   // TODO(https://crbug.com/967598): Remove this once OAuth2AccessTokenManager
   // fully manages access tokens independently of OAuth2TokenService.
   OAuth2TokenService* token_service_;
+  // TODO(https://crbug.com/967598): Replace it with
+  // OAuth2AccessTokenManagerDelegate.
+  OAuth2TokenServiceDelegate* delegate_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

google_apis/gaia/oauth2_token_service.cc

@@ -394,7 +394,8 @@ OAuth2TokenService::OAuth2TokenService(
     : delegate_(std::move(delegate)), all_credentials_loaded_(false) {
   DCHECK(delegate_);
   AddObserver(this);
-  token_manager_ = std::make_unique<OAuth2AccessTokenManager>(this);
+  token_manager_ =
+      std::make_unique<OAuth2AccessTokenManager>(this, delegate_.get());
 }
 
 OAuth2TokenService::~OAuth2TokenService() {
@@ -479,10 +480,7 @@ std::unique_ptr<OAuth2TokenService::Request> OAuth2TokenService::StartRequest(
     const CoreAccountId& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     OAuth2TokenService::Consumer* consumer) {
-  return StartRequestForClientWithContext(
-      account_id, delegate_->GetURLLoaderFactory(),
-      GaiaUrls::GetInstance()->oauth2_chrome_client_id(),
-      GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer);
+  return token_manager_->StartRequest(account_id, scopes, consumer);
 }
 
 std::unique_ptr<OAuth2TokenService::Request>
@@ -492,9 +490,8 @@ OAuth2TokenService::StartRequestForClient(
     const std::string& client_secret,
     const OAuth2TokenService::ScopeSet& scopes,
     OAuth2TokenService::Consumer* consumer) {
-  return StartRequestForClientWithContext(account_id, GetURLLoaderFactory(),
-                                          client_id, client_secret, scopes,
-                                          consumer);
+  return token_manager_->StartRequestForClient(account_id, client_id,
+                                               client_secret, scopes, consumer);
 }
 
 scoped_refptr<network::SharedURLLoaderFactory>
@@ -508,54 +505,8 @@ OAuth2TokenService::StartRequestWithContext(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     const ScopeSet& scopes,
     Consumer* consumer) {
-  return StartRequestForClientWithContext(
-      account_id, url_loader_factory,
-      GaiaUrls::GetInstance()->oauth2_chrome_client_id(),
-      GaiaUrls::GetInstance()->oauth2_chrome_client_secret(), scopes, consumer);
-}
-
-std::unique_ptr<OAuth2TokenService::Request>
-OAuth2TokenService::StartRequestForClientWithContext(
-    const CoreAccountId& account_id,
-    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-    const std::string& client_id,
-    const std::string& client_secret,
-    const ScopeSet& scopes,
-    Consumer* consumer) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  std::unique_ptr<RequestImpl> request(new RequestImpl(account_id, consumer));
-  for (auto& observer : token_manager_->diagnostics_observer_list_)
-    observer.OnAccessTokenRequested(account_id, consumer->id(), scopes);
-
-  if (!RefreshTokenIsAvailable(account_id)) {
-    GoogleServiceAuthError error(GoogleServiceAuthError::USER_NOT_SIGNED_UP);
-
-    for (auto& observer : token_manager_->diagnostics_observer_list_) {
-      observer.OnFetchAccessTokenComplete(account_id, consumer->id(), scopes,
-                                          error, base::Time());
-    }
-
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(&RequestImpl::InformConsumer, request->AsWeakPtr(),
-                       error, OAuth2AccessTokenConsumer::TokenResponse()));
-    return std::move(request);
-  }
-
-  RequestParameters request_parameters(client_id,
-                                       account_id,
-                                       scopes);
-  const OAuth2AccessTokenConsumer::TokenResponse* token_response =
-      GetCachedTokenResponse(request_parameters);
-  if (token_response && token_response->access_token.length()) {
-    InformConsumerWithCachedTokenResponse(token_response, request.get(),
-                                          request_parameters);
-  } else {
-    FetchOAuth2Token(request.get(), account_id, url_loader_factory, client_id,
-                     client_secret, scopes);
-  }
-  return std::move(request);
+  return token_manager_->StartRequestWithContext(account_id, url_loader_factory,
+                                                 scopes, consumer);
 }
 
 void OAuth2TokenService::FetchOAuth2Token(
@@ -591,24 +542,6 @@ OAuth2TokenService::CreateAccessTokenFetcher(
                                              consumer);
 }
 
-void OAuth2TokenService::InformConsumerWithCachedTokenResponse(
-    const OAuth2AccessTokenConsumer::TokenResponse* cache_token_response,
-    RequestImpl* request,
-    const RequestParameters& request_parameters) {
-  DCHECK(cache_token_response && cache_token_response->access_token.length());
-  for (auto& observer : token_manager_->diagnostics_observer_list_) {
-    observer.OnFetchAccessTokenComplete(
-        request_parameters.account_id, request->GetConsumerId(),
-        request_parameters.scopes, GoogleServiceAuthError::AuthErrorNone(),
-        cache_token_response->expiration_time);
-  }
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&RequestImpl::InformConsumer, request->AsWeakPtr(),
-                     GoogleServiceAuthError(GoogleServiceAuthError::NONE),
-                     *cache_token_response));
-}
-
 bool OAuth2TokenService::AreAllCredentialsLoaded() const {
   return all_credentials_loaded_;
 }

google_apis/gaia/oauth2_token_service.h

@@ -158,6 +158,7 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
   // |scopes| is the set of scopes to get an access token for, |consumer| is
   // the object that will be called back with results if the returned request
   // is not deleted. Virtual for mocking.
+  // Deprecated. It's moved to OAuth2AccessTokenManager.
   virtual std::unique_ptr<Request> StartRequest(const CoreAccountId& account_id,
                                                 const ScopeSet& scopes,
                                                 Consumer* consumer);
@@ -172,6 +173,7 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
   // This method does the same as |StartRequest| except it uses |client_id| and
   // |client_secret| to identify OAuth client app instead of using
   // Chrome's default values.
+  // Deprecated. It's moved to OAuth2AccessTokenManager.
   std::unique_ptr<Request> StartRequestForClient(
       const CoreAccountId& account_id,
       const std::string& client_id,
@@ -182,6 +184,7 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
   // This method does the same as |StartRequest| except it uses the
   // URLLoaderfactory given by |url_loader_factory| instead of using the one
   // returned by |GetURLLoaderFactory| implemented by derived classes.
+  // Deprecated. It's moved to OAuth2AccessTokenManager.
   std::unique_ptr<Request> StartRequestWithContext(
       const CoreAccountId& account_id,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
@@ -261,6 +264,10 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
   GetAccessTokenDiagnosticsObservers();
 
  protected:
+  // TODO(https://crbug.com/967598): Remove this once OAuth2AccessTokenManager
+  // fully manages access tokens independently of OAuth2TokenService.
+  friend class OAuth2AccessTokenManager;
+
   // Implements a cancelable |OAuth2TokenService::Request|, which should be
   // operated on the UI thread.
   // TODO(davidroche): move this out of header file.
@@ -354,23 +361,6 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
   // |StartRequest| method.
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() const;
 
-  // This method does the same as |StartRequestWithContext| except it
-  // uses |client_id| and |client_secret| to identify OAuth
-  // client app instead of using Chrome's default values.
-  std::unique_ptr<Request> StartRequestForClientWithContext(
-      const CoreAccountId& account_id,
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-      const std::string& client_id,
-      const std::string& client_secret,
-      const ScopeSet& scopes,
-      Consumer* consumer);
-
-  // Posts a task to fire the Consumer callback with the cached token response.
-  void InformConsumerWithCachedTokenResponse(
-      const OAuth2AccessTokenConsumer::TokenResponse* token_response,
-      RequestImpl* request,
-      const RequestParameters& client_scopes);
-
   // Returns a currently valid OAuth2 access token for the given set of scopes,
   // or NULL if none have been cached. Note the user of this method should
   // ensure no entry with the same |client_scopes| is added before the usage of