[NTP Snippets] RemoteSuggestionsFetcherImpl test uses IdentityTestEnv

This change is a followup to
https://chromium-review.googlesource.com/c/chromium/src/+/870313. It
changes the RemoteSuggestionsFetcherImpl unittest to use
IdentityTestEnvironment now that the production code is using
IdentityManager.

This CL also adds an API to IdentityTestEnvironment that is needed for
the conversion.

Bug: 798413
Change-Id: Ie2ec495c32d86130ee93bdb56b92de9cf93f9500
Reviewed-on: https://chromium-review.googlesource.com/870594
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530475}

components/ntp_snippets/BUILD.gn

@@ -251,6 +251,7 @@ source_set("unit_tests") {
     "//google_apis/gcm",
     "//net:test_support",
     "//services/identity/public/cpp",
+    "//services/identity/public/cpp:test_support",
     "//testing/gtest",
     "//third_party/icu/",
     "//ui/gfx:test_support",

components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc

@@ -27,15 +27,12 @@
 #include "components/ntp_snippets/remote/test_utils.h"
 #include "components/ntp_snippets/user_classifier.h"
 #include "components/prefs/testing_pref_service.h"
-#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
-#include "components/signin/core/browser/fake_signin_manager.h"
 #include "components/variations/entropy_provider.h"
 #include "components/variations/variations_params_manager.h"
-#include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_test_util.h"
-#include "services/identity/public/cpp/identity_manager.h"
+#include "services/identity/public/cpp/identity_test_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -261,9 +258,7 @@ void ParseJsonDelayed(const std::string& json,
 
 }  // namespace
 
-class RemoteSuggestionsFetcherImplTestBase
-    : public testing::Test,
-      public OAuth2TokenService::DiagnosticsObserver {
+class RemoteSuggestionsFetcherImplTestBase : public testing::Test {
  public:
   explicit RemoteSuggestionsFetcherImplTestBase(const GURL& gurl)
       : default_variation_params_(
@@ -283,8 +278,6 @@ class RemoteSuggestionsFetcherImplTestBase
   }
 
   ~RemoteSuggestionsFetcherImplTestBase() override {
-    if (fake_token_service_)
-      fake_token_service_->RemoveDiagnosticsObserver(this);
   }
 
   void ResetFetcher() { ResetFetcherWithAPIKey(kAPIKey); }
@@ -293,25 +286,10 @@ class RemoteSuggestionsFetcherImplTestBase
     scoped_refptr<net::TestURLRequestContextGetter> request_context_getter =
         new net::TestURLRequestContextGetter(mock_task_runner_.get());
 
-    if (fake_token_service_) {
-      fake_token_service_->RemoveDiagnosticsObserver(this);
-      identity_manager_.reset();
-    }
-
-    fake_token_service_ = std::make_unique<FakeProfileOAuth2TokenService>(
-        std::make_unique<FakeOAuth2TokenServiceDelegate>(
-            request_context_getter.get()));
-
-    fake_token_service_->AddDiagnosticsObserver(this);
-
-    // TODO(blundell): Convert this test to use IdentityTestEnvironment once
-    // that infrastructure lands in the codebase.
-    identity_manager_ = std::make_unique<identity::IdentityManager>(
-        utils_.fake_signin_manager(), fake_token_service_.get());
-
     fetcher_ = std::make_unique<RemoteSuggestionsFetcherImpl>(
-        identity_manager_.get(), std::move(request_context_getter),
-        utils_.pref_service(), nullptr, base::BindRepeating(&ParseJsonDelayed),
+        identity_test_env_.identity_manager(),
+        std::move(request_context_getter), utils_.pref_service(), nullptr,
+        base::BindRepeating(&ParseJsonDelayed),
         GetFetchEndpoint(version_info::Channel::STABLE), api_key,
         user_classifier_.get());
 
@@ -320,24 +298,8 @@ class RemoteSuggestionsFetcherImplTestBase
   }
 
   void SignIn() {
-    identity_manager_->SetPrimaryAccountSynchronouslyForTests(kTestAccount,
-                                                              kTestAccount, "");
-  }
-
-  void IssueRefreshToken() {
-    fake_token_service_->GetDelegate()->UpdateCredentials(kTestAccount,
-                                                          "token");
-  }
-
-  void IssueOAuth2Token() {
-    fake_token_service_->IssueAllTokensForAccount(kTestAccount, "access_token",
-                                                  base::Time::Max());
-  }
-
-  void CancelOAuth2TokenRequests() {
-    fake_token_service_->IssueErrorForAllPendingRequestsForAccount(
-        kTestAccount, GoogleServiceAuthError(
-                          GoogleServiceAuthError::State::REQUEST_CANCELED));
+    identity_test_env_.MakePrimaryAccountAvailable(kTestAccount, kTestAccount,
+                                                   "token");
   }
 
   RemoteSuggestionsFetcher::SnippetsAvailableCallback
@@ -387,23 +349,12 @@ class RemoteSuggestionsFetcherImplTestBase
     fake_url_fetcher_factory_->SetFakeResponse(test_url_, response_data,
                                                response_code, status);
   }
-  void set_on_access_token_request_callback(base::OnceClosure callback) {
-    on_access_token_request_callback_ = std::move(callback);
-  }
 
  protected:
   std::map<std::string, std::string> default_variation_params_;
+  identity::IdentityTestEnvironment identity_test_env_;
 
  private:
-  // OAuth2TokenService::DiagnosticsObserver:
-  void OnAccessTokenRequested(
-      const std::string& account_id,
-      const std::string& consumer_id,
-      const OAuth2TokenService::ScopeSet& scopes) override {
-    if (on_access_token_request_callback_)
-      std::move(on_access_token_request_callback_).Run();
-  }
-
   // TODO(tzik): Remove |clock_| after updating GetMockTickClock to own the
   // instance. http://crbug.com/789079
   std::unique_ptr<base::Clock> clock_;
@@ -414,14 +365,11 @@ class RemoteSuggestionsFetcherImplTestBase
   FailingFakeURLFetcherFactory failing_url_fetcher_factory_;
   // Initialized lazily in SetFakeResponse().
   std::unique_ptr<net::FakeURLFetcherFactory> fake_url_fetcher_factory_;
-  std::unique_ptr<FakeProfileOAuth2TokenService> fake_token_service_;
-  std::unique_ptr<identity::IdentityManager> identity_manager_;
   std::unique_ptr<RemoteSuggestionsFetcherImpl> fetcher_;
   std::unique_ptr<UserClassifier> user_classifier_;
   MockSnippetsAvailableCallback mock_callback_;
   const GURL test_url_;
   base::HistogramTester histogram_tester_;
-  base::OnceClosure on_access_token_request_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsFetcherImplTestBase);
 };
@@ -497,11 +445,7 @@ TEST_F(RemoteSuggestionsSignedOutFetcherTest, ShouldFetchSuccessfully) {
 }
 
 TEST_F(RemoteSuggestionsSignedInFetcherTest, ShouldFetchSuccessfully) {
-  base::RunLoop run_loop;
-  set_on_access_token_request_callback(run_loop.QuitClosure());
-
   SignIn();
-  IssueRefreshToken();
 
   const std::string kJsonStr =
       "{\"categories\" : [{"
@@ -531,9 +475,9 @@ TEST_F(RemoteSuggestionsSignedInFetcherTest, ShouldFetchSuccessfully) {
   fetcher().FetchSnippets(test_params(),
                           ToSnippetsAvailableCallback(&mock_callback()));
 
-  run_loop.Run();
+  identity_test_env_.WaitForAccessTokenRequestAndRespondWithToken(
+      "access_token", base::Time::Max());
 
-  IssueOAuth2Token();
   // Wait for the fake response.
   FastForwardUntilNoTasksRemain();
 
@@ -548,11 +492,7 @@ TEST_F(RemoteSuggestionsSignedInFetcherTest, ShouldFetchSuccessfully) {
 }
 
 TEST_F(RemoteSuggestionsSignedInFetcherTest, ShouldRetryWhenOAuthCancelled) {
-  base::RunLoop run_loop;
-  set_on_access_token_request_callback(run_loop.QuitClosure());
-
   SignIn();
-  IssueRefreshToken();
 
   const std::string kJsonStr =
       "{\"categories\" : [{"
@@ -582,20 +522,15 @@ TEST_F(RemoteSuggestionsSignedInFetcherTest, ShouldRetryWhenOAuthCancelled) {
   fetcher().FetchSnippets(test_params(),
                           ToSnippetsAvailableCallback(&mock_callback()));
 
-  // Wait for the first access token request to be made.
-  run_loop.Run();
-
-  // Before cancelling the outstanding access token request, prepare to wait for
-  // the second access token request to be made in response to the cancellation.
-  base::RunLoop run_loop2;
-  set_on_access_token_request_callback(run_loop2.QuitClosure());
-
-  CancelOAuth2TokenRequests();
+  // Cancel the first access token request that's made.
+  identity_test_env_.WaitForAccessTokenRequestAndRespondWithError(
+      GoogleServiceAuthError(GoogleServiceAuthError::State::REQUEST_CANCELED));
 
-  // Wait for the second access token request to be made.
-  run_loop2.Run();
+  // RemoteSuggestionsFetcher should retry fetching an access token if the first
+  // attempt is cancelled. Respond with a valid access token on the retry.
+  identity_test_env_.WaitForAccessTokenRequestAndRespondWithToken(
+      "access_token", base::Time::Max());
 
-  IssueOAuth2Token();
   // Wait for the fake response.
   FastForwardUntilNoTasksRemain();
 

services/identity/public/cpp/identity_test_environment.cc

@@ -109,6 +109,14 @@ void IdentityTestEnvironment::SetAutomaticIssueOfAccessTokens(bool grant) {
       grant);
 }
 
+void IdentityTestEnvironment::WaitForAccessTokenRequestAndRespondWithToken(
+    const std::string& token,
+    const base::Time& expiration) {
+  WaitForAccessTokenRequest();
+  internals_->token_service()->IssueTokenForAllPendingRequests(token,
+                                                               expiration);
+}
+
 void IdentityTestEnvironment::WaitForAccessTokenRequestAndRespondWithError(
     const GoogleServiceAuthError& error) {
   WaitForAccessTokenRequest();

services/identity/public/cpp/identity_test_environment.h

@@ -32,6 +32,14 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
   // an access token value of "access_token".
   void SetAutomaticIssueOfAccessTokens(bool grant);
 
+  // Waits for an access token request to occur and issues |token| in response.
+  // NOTE: The implementation currently issues tokens in response to *all*
+  // pending access token requests. If you need finer granularity, contact
+  // blundell@chromium.org
+  void WaitForAccessTokenRequestAndRespondWithToken(
+      const std::string& token,
+      const base::Time& expiration);
+
   // Waits for an access token request to occur and issues |error| in response.
   // NOTE: The implementation currently issues errors in response to *all*
   // pending access token requests. If you need finer granularity, contact