Move RequestParameters from O2TS to OAuth2AccessTokenManager

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

It moves RequestParameters to OAuth2AccessTokenManager and
TokenCache type definition which uses RequestParameters
also is moved.

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

google_apis/gaia/oauth2_access_token_manager.cc

@@ -16,6 +16,32 @@
 
 int OAuth2AccessTokenManager::max_fetch_retry_num_ = 5;
 
+OAuth2AccessTokenManager::RequestParameters::RequestParameters(
+    const std::string& client_id,
+    const CoreAccountId& account_id,
+    const OAuth2TokenService::ScopeSet& scopes)
+    : client_id(client_id), account_id(account_id), scopes(scopes) {}
+
+OAuth2AccessTokenManager::RequestParameters::RequestParameters(
+    const RequestParameters& other) = default;
+
+OAuth2AccessTokenManager::RequestParameters::~RequestParameters() {}
+
+bool OAuth2AccessTokenManager::RequestParameters::operator<(
+    const RequestParameters& p) const {
+  if (client_id < p.client_id)
+    return true;
+  else if (p.client_id < client_id)
+    return false;
+
+  if (account_id < p.account_id)
+    return true;
+  else if (p.account_id < account_id)
+    return false;
+
+  return scopes < p.scopes;
+}
+
 // Class that fetches an OAuth2 access token for a given account id and set of
 // scopes.
 //
@@ -386,8 +412,8 @@ void OAuth2AccessTokenManager::FetchOAuth2Token(
   // If there is already a pending fetcher for |scopes| and |account_id|,
   // simply register this |request| for those results rather than starting
   // a new fetcher.
-  OAuth2TokenService::RequestParameters request_parameters =
-      OAuth2TokenService::RequestParameters(client_id, account_id, scopes);
+  RequestParameters request_parameters =
+      RequestParameters(client_id, account_id, scopes);
   auto iter = pending_fetchers_.find(request_parameters);
   if (iter != pending_fetchers_.end()) {
     iter->second->AddWaitingRequest(request->AsWeakPtr());
@@ -406,16 +432,15 @@ void OAuth2AccessTokenManager::RegisterTokenResponse(
     const OAuth2AccessTokenConsumer::TokenResponse& token_response) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  token_cache_[OAuth2TokenService::RequestParameters(client_id, account_id,
-                                                     scopes)] = token_response;
+  token_cache_[RequestParameters(client_id, account_id, scopes)] =
+      token_response;
 }
 
 const OAuth2AccessTokenConsumer::TokenResponse*
 OAuth2AccessTokenManager::GetCachedTokenResponse(
-    const OAuth2TokenService::RequestParameters& request_parameters) {
+    const RequestParameters& request_parameters) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  OAuth2TokenService::TokenCache::iterator token_iterator =
-      token_cache_.find(request_parameters);
+  TokenCache::iterator token_iterator = token_cache_.find(request_parameters);
   if (token_iterator == token_cache_.end())
     return nullptr;
   if (token_iterator->second.expiration_time <= base::Time::Now()) {
@@ -438,7 +463,7 @@ void OAuth2AccessTokenManager::ClearCache() {
 void OAuth2AccessTokenManager::ClearCacheForAccount(
     const CoreAccountId& account_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  for (OAuth2TokenService::TokenCache::iterator iter = token_cache_.begin();
+  for (TokenCache::iterator iter = token_cache_.begin();
        iter != token_cache_.end();
        /* iter incremented in body */) {
     if (iter->first.account_id == account_id) {
@@ -468,6 +493,17 @@ void OAuth2AccessTokenManager::CancelRequestsForAccount(
   CancelFetchers(fetchers_to_cancel);
 }
 
+void OAuth2AccessTokenManager::InvalidateAccessTokenImpl(
+    const CoreAccountId& account_id,
+    const std::string& client_id,
+    const OAuth2TokenService::ScopeSet& scopes,
+    const std::string& access_token) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  RemoveCachedTokenResponse(RequestParameters(client_id, account_id, scopes),
+                            access_token);
+  delegate_->InvalidateAccessToken(account_id, client_id, scopes, access_token);
+}
+
 void OAuth2AccessTokenManager::
     set_max_authorization_token_fetch_retries_for_testing(int max_retries) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -478,8 +514,8 @@ size_t OAuth2AccessTokenManager::GetNumPendingRequestsForTesting(
     const std::string& client_id,
     const CoreAccountId& account_id,
     const OAuth2TokenService::ScopeSet& scopes) const {
-  auto iter = pending_fetchers_.find(
-      OAuth2TokenService::RequestParameters(client_id, account_id, scopes));
+  auto iter =
+      pending_fetchers_.find(RequestParameters(client_id, account_id, scopes));
   return iter == pending_fetchers_.end()
              ? 0
              : iter->second->GetWaitingRequestCount();
@@ -525,8 +561,7 @@ OAuth2AccessTokenManager::StartRequestForClientWithContext(
     return std::move(request);
   }
 
-  OAuth2TokenService::RequestParameters request_parameters(client_id,
-                                                           account_id, scopes);
+  RequestParameters request_parameters(client_id, account_id, scopes);
   const OAuth2AccessTokenConsumer::TokenResponse* token_response =
       GetCachedTokenResponse(request_parameters);
   if (token_response && token_response->access_token.length()) {
@@ -547,7 +582,7 @@ OAuth2AccessTokenManager::StartRequestForClientWithContext(
 void OAuth2AccessTokenManager::InformConsumerWithCachedTokenResponse(
     const OAuth2AccessTokenConsumer::TokenResponse* cache_token_response,
     OAuth2TokenService::RequestImpl* request,
-    const OAuth2TokenService::RequestParameters& request_parameters) {
+    const RequestParameters& request_parameters) {
   DCHECK(cache_token_response && cache_token_response->access_token.length());
   for (auto& observer : diagnostics_observer_list_) {
     observer.OnFetchAccessTokenComplete(
@@ -564,11 +599,10 @@ void OAuth2AccessTokenManager::InformConsumerWithCachedTokenResponse(
 }
 
 bool OAuth2AccessTokenManager::RemoveCachedTokenResponse(
-    const OAuth2TokenService::RequestParameters& request_parameters,
+    const RequestParameters& request_parameters,
     const std::string& token_to_remove) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  OAuth2TokenService::TokenCache::iterator token_iterator =
-      token_cache_.find(request_parameters);
+  TokenCache::iterator token_iterator = token_cache_.find(request_parameters);
   if (token_iterator != token_cache_.end() &&
       token_iterator->second.access_token == token_to_remove) {
     for (auto& observer : diagnostics_observer_list_) {
@@ -617,7 +651,7 @@ void OAuth2AccessTokenManager::OnFetchComplete(
   // Then by (2), |fetcher| is recorded in |pending_fetchers_|.
   // Then by (3), |fetcher_| is mapped from its combination of client ID,
   // account ID, and scope set.
-  OAuth2TokenService::RequestParameters request_param(
+  RequestParameters request_param(
       fetcher->GetClientId(), fetcher->GetAccountId(), fetcher->GetScopeSet());
 
   const OAuth2AccessTokenConsumer::TokenResponse* entry =

google_apis/gaia/oauth2_access_token_manager.h

@@ -18,6 +18,25 @@ class OAuth2TokenServiceDelegate;
 // Class that manages requests for OAuth2 access tokens.
 class OAuth2AccessTokenManager {
  public:
+  // The parameters used to fetch an OAuth2 access token.
+  struct RequestParameters {
+    RequestParameters(const std::string& client_id,
+                      const CoreAccountId& account_id,
+                      const OAuth2TokenService::ScopeSet& scopes);
+    RequestParameters(const RequestParameters& other);
+    ~RequestParameters();
+    bool operator<(const RequestParameters& params) const;
+
+    // OAuth2 client id.
+    std::string client_id;
+    // Account id for which the request is made.
+    CoreAccountId account_id;
+    // URL scopes for the requested access token.
+    OAuth2TokenService::ScopeSet scopes;
+  };
+  typedef std::map<RequestParameters, OAuth2AccessTokenConsumer::TokenResponse>
+      TokenCache;
+
   // TODO(https://crbug.com/967598): Remove |token_service| parameter once
   // OAuth2AccessTokenManager fully manages access tokens independently of
   // OAuth2TokenService and replace |delegate| with
@@ -85,7 +104,7 @@ class OAuth2AccessTokenManager {
   // ensure no entry with the same |client_scopes| is added before the usage of
   // the returned entry is done.
   const OAuth2AccessTokenConsumer::TokenResponse* GetCachedTokenResponse(
-      const OAuth2TokenService::RequestParameters& client_scopes);
+      const RequestParameters& client_scopes);
 
   // Clears the internal token cache.
   void ClearCache();
@@ -101,6 +120,13 @@ class OAuth2AccessTokenManager {
   // Cancels all requests related to a given |account_id|.
   void CancelRequestsForAccount(const CoreAccountId& account_id);
 
+  // Invalidates the |access_token| issued for |account_id|, |client_id| and
+  // |scopes|.
+  void InvalidateAccessTokenImpl(const CoreAccountId& account_id,
+                                 const std::string& client_id,
+                                 const OAuth2TokenService::ScopeSet& scopes,
+                                 const std::string& access_token);
+
   void set_max_authorization_token_fetch_retries_for_testing(int max_retries);
 
   // Returns the current number of pending fetchers matching given params.
@@ -117,7 +143,7 @@ class OAuth2AccessTokenManager {
   class Fetcher;
   friend class Fetcher;
 
-  OAuth2TokenService::TokenCache& token_cache() { return token_cache_; }
+  TokenCache& token_cache() { return token_cache_; }
 
   // Create an access token fetcher for the given account id.
   std::unique_ptr<OAuth2AccessTokenFetcher> CreateAccessTokenFetcher(
@@ -140,13 +166,12 @@ class OAuth2AccessTokenManager {
   void InformConsumerWithCachedTokenResponse(
       const OAuth2AccessTokenConsumer::TokenResponse* token_response,
       OAuth2TokenService::RequestImpl* request,
-      const OAuth2TokenService::RequestParameters& client_scopes);
+      const 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(
-      const OAuth2TokenService::RequestParameters& client_scopes,
-      const std::string& token_to_remove);
+  bool RemoveCachedTokenResponse(const RequestParameters& client_scopes,
+                                 const std::string& token_to_remove);
 
   // Called when |fetcher| finishes fetching.
   void OnFetchComplete(Fetcher* fetcher);
@@ -155,7 +180,7 @@ class OAuth2AccessTokenManager {
   void CancelFetchers(std::vector<Fetcher*> fetchers_to_cancel);
 
   // The cache of currently valid tokens.
-  OAuth2TokenService::TokenCache token_cache_;
+  TokenCache token_cache_;
   // List of observers to notify when access token status changes.
   base::ObserverList<AccessTokenDiagnosticsObserver, true>::Unchecked
       diagnostics_observer_list_;
@@ -167,8 +192,7 @@ class OAuth2AccessTokenManager {
   OAuth2TokenServiceDelegate* delegate_;
   // A map from fetch parameters to a fetcher that is fetching an OAuth2 access
   // token using these parameters.
-  std::map<OAuth2TokenService::RequestParameters, std::unique_ptr<Fetcher>>
-      pending_fetchers_;
+  std::map<RequestParameters, std::unique_ptr<Fetcher>> pending_fetchers_;
   // Maximum number of retries in fetching an OAuth2 access token.
   static int max_fetch_retry_num_;
 

google_apis/gaia/oauth2_token_service.cc

@@ -23,33 +23,6 @@
 #include "google_apis/gaia/oauth2_token_service_delegate.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
-OAuth2TokenService::RequestParameters::RequestParameters(
-    const std::string& client_id,
-    const CoreAccountId& account_id,
-    const ScopeSet& scopes)
-    : client_id(client_id), account_id(account_id), scopes(scopes) {}
-
-OAuth2TokenService::RequestParameters::RequestParameters(
-    const RequestParameters& other) = default;
-
-OAuth2TokenService::RequestParameters::~RequestParameters() {
-}
-
-bool OAuth2TokenService::RequestParameters::operator<(
-    const RequestParameters& p) const {
-  if (client_id < p.client_id)
-    return true;
-  else if (p.client_id < client_id)
-    return false;
-
-  if (account_id < p.account_id)
-    return true;
-  else if (p.account_id < account_id)
-    return false;
-
-  return scopes < p.scopes;
-}
-
 OAuth2TokenService::RequestImpl::RequestImpl(
     const CoreAccountId& account_id,
     OAuth2TokenService::Consumer* consumer)
@@ -116,8 +89,8 @@ OAuth2TokenService::GetAccessTokenDiagnosticsObservers() {
   return token_manager_->diagnostics_observer_list_;
 }
 
-OAuth2TokenService::TokenCache& OAuth2TokenService::token_cache() {
-  return token_manager_->token_cache();
+int OAuth2TokenService::GetTokenCacheCount() {
+  return token_manager_->token_cache().size();
 }
 
 void OAuth2TokenService::AddObserver(OAuth2TokenServiceObserver* observer) {
@@ -273,9 +246,8 @@ void OAuth2TokenService::InvalidateAccessTokenImpl(
     const ScopeSet& scopes,
     const std::string& access_token) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  token_manager_->RemoveCachedTokenResponse(
-      RequestParameters(client_id, account_id, scopes), access_token);
-  delegate_->InvalidateAccessToken(account_id, client_id, scopes, access_token);
+  token_manager_->InvalidateAccessTokenImpl(account_id, client_id, scopes,
+                                            access_token);
 }
 
 void OAuth2TokenService::OnRefreshTokensLoaded() {

google_apis/gaia/oauth2_token_service.h

@@ -91,25 +91,6 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
     std::string id_;
   };
 
-  // The parameters used to fetch an OAuth2 access token.
-  struct RequestParameters {
-    RequestParameters(const std::string& client_id,
-                      const CoreAccountId& account_id,
-                      const ScopeSet& scopes);
-    RequestParameters(const RequestParameters& other);
-    ~RequestParameters();
-    bool operator<(const RequestParameters& params) const;
-
-    // OAuth2 client id.
-    std::string client_id;
-    // Account id for which the request is made.
-    CoreAccountId account_id;
-    // URL scopes for the requested access token.
-    ScopeSet scopes;
-  };
-  typedef std::map<RequestParameters, OAuth2AccessTokenConsumer::TokenResponse>
-      TokenCache;
-
   explicit OAuth2TokenService(
       std::unique_ptr<OAuth2TokenServiceDelegate> delegate);
   ~OAuth2TokenService() override;
@@ -230,7 +211,7 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
 
   // TODO(https://crbug.com/967598): Remove this. It's opened only for
   // OAuth2TokenServiceTest.
-  OAuth2TokenService::TokenCache& token_cache();
+  int GetTokenCacheCount();
 
   const base::ObserverList<AccessTokenDiagnosticsObserver, true>::Unchecked&
   GetAccessTokenDiagnosticsObservers();
@@ -313,8 +294,9 @@ class OAuth2TokenService : public OAuth2TokenServiceObserver {
       const ScopeSet& scopes);
 
   // Invalidates the |access_token| issued for |account_id|, |client_id| and
-  // |scopes|. Virtual so it can be overriden for tests and for platform-
-  // specifc behavior.
+  // |scopes|. Virtual so it can be overridden for tests and for platform-
+  // specific behavior.
+  // Deprecated. It's moved to OAuth2AccessTokenManager.
   virtual void InvalidateAccessTokenImpl(const CoreAccountId& account_id,
                                          const std::string& client_id,
                                          const ScopeSet& scopes,

google_apis/gaia/oauth2_token_service_unittest.cc

@@ -16,6 +16,7 @@
 #include "google_apis/gaia/oauth2_access_token_consumer.h"
 #include "google_apis/gaia/oauth2_access_token_fetcher_immediate_error.h"
 #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h"
+#include "google_apis/gaia/oauth2_access_token_manager.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 #include "google_apis/gaia/oauth2_token_service_test_util.h"
 #include "net/http/http_status_code.h"
@@ -770,15 +771,15 @@ TEST_F(OAuth2TokenServiceTest, RequestParametersOrderTest) {
 
   const CoreAccountId account_id0("0");
   const CoreAccountId account_id1("1");
-  OAuth2TokenService::RequestParameters params[] = {
-      OAuth2TokenService::RequestParameters("0", account_id0, set_0),
-      OAuth2TokenService::RequestParameters("0", account_id0, set_1),
-      OAuth2TokenService::RequestParameters("0", account_id1, set_0),
-      OAuth2TokenService::RequestParameters("0", account_id1, set_1),
-      OAuth2TokenService::RequestParameters("1", account_id0, set_0),
-      OAuth2TokenService::RequestParameters("1", account_id0, set_1),
-      OAuth2TokenService::RequestParameters("1", account_id1, set_0),
-      OAuth2TokenService::RequestParameters("1", account_id1, set_1),
+  OAuth2AccessTokenManager::RequestParameters params[] = {
+      OAuth2AccessTokenManager::RequestParameters("0", "0", set_0),
+      OAuth2AccessTokenManager::RequestParameters("0", "0", set_1),
+      OAuth2AccessTokenManager::RequestParameters("0", "1", set_0),
+      OAuth2AccessTokenManager::RequestParameters("0", "1", set_1),
+      OAuth2AccessTokenManager::RequestParameters("1", "0", set_0),
+      OAuth2AccessTokenManager::RequestParameters("1", "0", set_1),
+      OAuth2AccessTokenManager::RequestParameters("1", "1", set_0),
+      OAuth2AccessTokenManager::RequestParameters("1", "1", set_1),
   };
 
   for (size_t i = 0; i < base::size(params); i++) {
@@ -811,11 +812,11 @@ TEST_F(OAuth2TokenServiceTest, UpdateClearsCache) {
   EXPECT_EQ(1, consumer_.number_of_successful_tokens_);
   EXPECT_EQ(0, consumer_.number_of_errors_);
   EXPECT_EQ("token", consumer_.last_token_);
-  EXPECT_EQ(1, (int)oauth2_service_->token_cache().size());
+  EXPECT_EQ(1, oauth2_service_->GetTokenCacheCount());
 
   oauth2_service_->ClearCache();
 
-  EXPECT_EQ(0, (int)oauth2_service_->token_cache().size());
+  EXPECT_EQ(0, oauth2_service_->GetTokenCacheCount());
   oauth2_service_->GetFakeOAuth2TokenServiceDelegate()->UpdateCredentials(
       account_id, "refreshToken");
   SimulateOAuthTokenResponse(GetValidTokenResponse("another token", 3600));
@@ -824,7 +825,7 @@ TEST_F(OAuth2TokenServiceTest, UpdateClearsCache) {
   EXPECT_EQ(2, consumer_.number_of_successful_tokens_);
   EXPECT_EQ(0, consumer_.number_of_errors_);
   EXPECT_EQ("another token", consumer_.last_token_);
-  EXPECT_EQ(1, (int)oauth2_service_->token_cache().size());
+  EXPECT_EQ(1, oauth2_service_->GetTokenCacheCount());
 }
 
 TEST_F(OAuth2TokenServiceTest, FixRequestErrorIfPossible) {