Move Cancel{AllRequests,RequestsForAccount} from PO2TStest to O2ATMtest

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

It is specifically a step toward folding O2TS into PO2TS
now that O2TS is only one subclass of PO2TS.

It removes CancelAllRequests and CancelRequestsForAccount
from PO2TS since these are only used by PO2TS and it could
call these methods through |token_manager_| instead of
having helper methods.

It moves CancelAllRequests and CancelRequestsForAccount
from ProfileOAuth2TokenServiceTest
to OAuth2AccessTokenManagerTest since
OAuth2AccessTokenManager manages requests.

It adds AddAccount to FakeOAuth2AccessTokenManagerDelegate
to handle multiple accounts.

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

components/signin/internal/identity_manager/profile_oauth2_token_service.cc

@@ -253,7 +253,7 @@ void ProfileOAuth2TokenService::SetRefreshTokenRevokedFromSourceCallback(
 }
 
 void ProfileOAuth2TokenService::Shutdown() {
-  CancelAllRequests();
+  token_manager_->CancelAllRequests();
   GetDelegate()->Shutdown();
 }
 
@@ -287,7 +287,7 @@ void ProfileOAuth2TokenService::RevokeAllCredentials(
     SourceForRefreshTokenOperation source) {
   base::AutoReset<SourceForRefreshTokenOperation> auto_reset(
       &update_refresh_token_source_, source);
-  CancelAllRequests();
+  token_manager_->CancelAllRequests();
   ClearCache();
   GetDelegate()->RevokeAllCredentials();
 }
@@ -372,7 +372,7 @@ void ProfileOAuth2TokenService::OnRefreshTokenAvailable(
     is_valid = false;
   }
 
-  CancelRequestsForAccount(account_id);
+  token_manager_->CancelRequestsForAccount(account_id);
   ClearCacheForAccount(account_id);
 
   signin_metrics::RecordRefreshTokenUpdatedFromSource(
@@ -389,7 +389,7 @@ void ProfileOAuth2TokenService::OnRefreshTokenRevoked(
   // If this was the last token, recreate the device ID.
   RecreateDeviceIdIfNeeded();
 
-  CancelRequestsForAccount(account_id);
+  token_manager_->CancelRequestsForAccount(account_id);
   ClearCacheForAccount(account_id);
 
   signin_metrics::RecordRefreshTokenRevokedFromSource(
@@ -425,15 +425,6 @@ void ProfileOAuth2TokenService::ClearCacheForAccount(
   token_manager_->ClearCacheForAccount(account_id);
 }
 
-void ProfileOAuth2TokenService::CancelAllRequests() {
-  token_manager_->CancelAllRequests();
-}
-
-void ProfileOAuth2TokenService::CancelRequestsForAccount(
-    const CoreAccountId& account_id) {
-  token_manager_->CancelRequestsForAccount(account_id);
-}
-
 bool ProfileOAuth2TokenService::HasLoadCredentialsFinishedWithNoErrors() {
   switch (GetDelegate()->load_credentials_state()) {
     case signin::LoadCredentialsState::LOAD_CREDENTIALS_NOT_STARTED:

components/signin/internal/identity_manager/profile_oauth2_token_service.h

@@ -277,12 +277,6 @@ class ProfileOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
   // used to request the tokens.
   void ClearCacheForAccount(const CoreAccountId& account_id);
 
-  // Cancels all requests that are currently in progress.
-  void CancelAllRequests();
-
-  // Cancels all requests related to a given |account_id|.
-  void CancelRequestsForAccount(const CoreAccountId& account_id);
-
   // Creates a new device ID if there are no accounts, or if the current device
   // ID is empty.
   void RecreateDeviceIdIfNeeded();
@@ -304,9 +298,6 @@ class ProfileOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
       signin_metrics::SourceForRefreshTokenOperation::kUnknown;
 
   FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceTest, UpdateClearsCache);
-  FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceTest, CancelAllRequests);
-  FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceTest,
-                           CancelRequestsForAccount);
   FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceTest,
                            SameScopesRequestedForDifferentClients);
 

components/signin/internal/identity_manager/profile_oauth2_token_service_unittest.cc

@@ -644,65 +644,6 @@ TEST_F(ProfileOAuth2TokenServiceTest, InvalidateToken) {
   EXPECT_EQ("token2", consumer_.last_token_);
 }
 
-TEST_F(ProfileOAuth2TokenServiceTest, CancelAllRequests) {
-  oauth2_service_->GetDelegate()->UpdateCredentials(account_id_,
-                                                    "refreshToken");
-  std::unique_ptr<OAuth2AccessTokenManager::Request> request(
-      oauth2_service_->StartRequest(
-          account_id_, OAuth2AccessTokenManager::ScopeSet(), &consumer_));
-  const CoreAccountId account_id_2("account_id_2");
-  oauth2_service_->GetDelegate()->UpdateCredentials(account_id_2,
-                                                    "refreshToken2");
-  std::unique_ptr<OAuth2AccessTokenManager::Request> request2(
-      oauth2_service_->StartRequest(
-          account_id_, OAuth2AccessTokenManager::ScopeSet(), &consumer_));
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
-  EXPECT_EQ(0, consumer_.number_of_errors_);
-
-  oauth2_service_->CancelAllRequests();
-
-  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
-  EXPECT_EQ(2, consumer_.number_of_errors_);
-}
-
-TEST_F(ProfileOAuth2TokenServiceTest, CancelRequestsForAccount) {
-  OAuth2AccessTokenManager::ScopeSet scope_set_1;
-  scope_set_1.insert("scope1");
-  scope_set_1.insert("scope2");
-  OAuth2AccessTokenManager::ScopeSet scope_set_2(scope_set_1.begin(),
-                                                 scope_set_1.end());
-  scope_set_2.insert("scope3");
-
-  oauth2_service_->GetDelegate()->UpdateCredentials(account_id_,
-                                                    "refreshToken");
-  std::unique_ptr<OAuth2AccessTokenManager::Request> request1(
-      oauth2_service_->StartRequest(account_id_, scope_set_1, &consumer_));
-  std::unique_ptr<OAuth2AccessTokenManager::Request> request2(
-      oauth2_service_->StartRequest(account_id_, scope_set_2, &consumer_));
-
-  const CoreAccountId account_id_2("account_id_2");
-  oauth2_service_->GetDelegate()->UpdateCredentials(account_id_2,
-                                                    "refreshToken2");
-  std::unique_ptr<OAuth2AccessTokenManager::Request> request3(
-      oauth2_service_->StartRequest(account_id_2, scope_set_1, &consumer_));
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
-  EXPECT_EQ(0, consumer_.number_of_errors_);
-
-  oauth2_service_->CancelRequestsForAccount(account_id_);
-
-  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
-  EXPECT_EQ(2, consumer_.number_of_errors_);
-
-  oauth2_service_->CancelRequestsForAccount(account_id_2);
-
-  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
-  EXPECT_EQ(3, consumer_.number_of_errors_);
-}
-
 TEST_F(ProfileOAuth2TokenServiceTest, SameScopesRequestedForDifferentClients) {
   std::string client_id_1("client1");
   std::string client_secret_1("secret1");

google_apis/gaia/oauth2_access_token_manager_unittest.cc

@@ -35,13 +35,16 @@ class FakeOAuth2AccessTokenManagerDelegate
       const CoreAccountId& account_id,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       OAuth2AccessTokenConsumer* consumer) override {
-    EXPECT_EQ(CoreAccountId(kTestAccountId), account_id);
+    EXPECT_NE(account_ids_to_refresh_tokens_.find(account_id),
+              account_ids_to_refresh_tokens_.end());
     return std::make_unique<OAuth2AccessTokenFetcherImpl>(
-        consumer, url_loader_factory, "fake_refresh_token");
+        consumer, url_loader_factory,
+        account_ids_to_refresh_tokens_[account_id]);
   }
 
   bool HasRefreshToken(const CoreAccountId& account_id) const override {
-    return CoreAccountId(kTestAccountId) == account_id;
+    return account_ids_to_refresh_tokens_.find(account_id) !=
+           account_ids_to_refresh_tokens_.end();
   }
 
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory()
@@ -49,8 +52,13 @@ class FakeOAuth2AccessTokenManagerDelegate
     return shared_factory_;
   }
 
+  void AddAccount(CoreAccountId id, std::string refresh_token) {
+    account_ids_to_refresh_tokens_[id] = refresh_token;
+  }
+
  private:
   scoped_refptr<network::SharedURLLoaderFactory> shared_factory_;
+  std::map<CoreAccountId, std::string> account_ids_to_refresh_tokens_;
 };
 
 class FakeOAuth2AccessTokenManagerConsumer
@@ -86,12 +94,18 @@ class FakeOAuth2AccessTokenManagerConsumer
 
 }  // namespace
 
+// Any public API surfaces that are wrapped by ProfileOAuth2TokenService are
+// unittested as part of the unittests of that class.
+
 class OAuth2AccessTokenManagerTest : public testing::Test {
  public:
   OAuth2AccessTokenManagerTest()
       : delegate_(&test_url_loader_factory_), token_manager_(&delegate_) {}
 
-  void SetUp() override { account_id_ = CoreAccountId(kTestAccountId); }
+  void SetUp() override {
+    account_id_ = CoreAccountId(kTestAccountId);
+    delegate_.AddAccount(account_id_, "fake_refresh_token");
+  }
 
   void TearDown() override {
     // Makes sure that all the clean up tasks are run. It's required because of
@@ -115,7 +129,7 @@ class OAuth2AccessTokenManagerTest : public testing::Test {
   FakeOAuth2AccessTokenManagerConsumer consumer_;
 };
 
-// Test if StartRequest gets a response properly.
+// Test that StartRequest gets a response properly.
 TEST_F(OAuth2AccessTokenManagerTest, StartRequest) {
   base::RunLoop run_loop;
   consumer_.SetResponseCompletedClosure(run_loop.QuitClosure());
@@ -128,3 +142,56 @@ TEST_F(OAuth2AccessTokenManagerTest, StartRequest) {
   EXPECT_EQ(1, consumer_.number_of_successful_tokens_);
   EXPECT_EQ(0, consumer_.number_of_errors_);
 }
+
+// Test that CancelAllRequests triggers OnGetTokenFailure.
+TEST_F(OAuth2AccessTokenManagerTest, CancelAllRequests) {
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request(
+      token_manager_.StartRequest(
+          account_id_, OAuth2AccessTokenManager::ScopeSet(), &consumer_));
+  const CoreAccountId account_id_2("account_id_2");
+  delegate_.AddAccount(account_id_2, "refreshToken2");
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request2(
+      token_manager_.StartRequest(
+          account_id_2, OAuth2AccessTokenManager::ScopeSet(), &consumer_));
+
+  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
+  EXPECT_EQ(0, consumer_.number_of_errors_);
+
+  token_manager_.CancelAllRequests();
+
+  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
+  EXPECT_EQ(2, consumer_.number_of_errors_);
+}
+
+// Test that CancelRequestsForAccount cancels requests for the specific account.
+TEST_F(OAuth2AccessTokenManagerTest, CancelRequestsForAccount) {
+  OAuth2AccessTokenManager::ScopeSet scope_set_1;
+  scope_set_1.insert("scope1");
+  scope_set_1.insert("scope2");
+  OAuth2AccessTokenManager::ScopeSet scope_set_2(scope_set_1.begin(),
+                                                 scope_set_1.end());
+  scope_set_2.insert("scope3");
+
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request1(
+      token_manager_.StartRequest(account_id_, scope_set_1, &consumer_));
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request2(
+      token_manager_.StartRequest(account_id_, scope_set_2, &consumer_));
+
+  const CoreAccountId account_id_2("account_id_2");
+  delegate_.AddAccount(account_id_2, "refreshToken2");
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request3(
+      token_manager_.StartRequest(account_id_2, scope_set_1, &consumer_));
+
+  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
+  EXPECT_EQ(0, consumer_.number_of_errors_);
+
+  token_manager_.CancelRequestsForAccount(account_id_);
+
+  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
+  EXPECT_EQ(2, consumer_.number_of_errors_);
+
+  token_manager_.CancelRequestsForAccount(account_id_2);
+
+  EXPECT_EQ(0, consumer_.number_of_successful_tokens_);
+  EXPECT_EQ(3, consumer_.number_of_errors_);
+}