[NTP Snippets] Have RemoteSuggestionsFetcherImpl use IdentityManager

This CL converts RemoteSuggestionsFetcherImpl from using
//components/signin to using the Identity Service client library.
The conversion is straightforward:

- Get info of primary account via IdentityManager rather than
SigninManager.
- Create PrimaryAccountAccessTokenFetcher via IdentityManager rather
than directly.

I tested the relevant behavior manually by going to the NTP for a
signed-in and syncing test user on Android and confirming that their
content suggestions showed up after this change identically to before
this change.

A followup change will convert the test to use IdentityTestEnvironment.

TBR=gambard@chromium.org

Bug: 798413
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If639f55969f70edbaff758598f95e0d74406b492
Reviewed-on: https://chromium-review.googlesource.com/870313Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530135}

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/ntp_snippets/dependent_features.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/suggestions/image_decoder_impl.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ -338,8 +339,8 @@ void RegisterArticleProviderIfEnabled(ContentSuggestionsService* service,
   }
 
   PrefService* pref_service = profile->GetPrefs();
-  OAuth2TokenService* token_service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
+  identity::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
   UrlLanguageHistogram* language_histogram =
       UrlLanguageHistogramFactory::GetForBrowserContext(profile);
 
@@ -375,8 +376,7 @@ void RegisterArticleProviderIfEnabled(ContentSuggestionsService* service,
   }
 #endif  // BUILDFLAG(ENABLE_OFFLINE_PAGES)
   auto suggestions_fetcher = base::MakeUnique<RemoteSuggestionsFetcherImpl>(
-      signin_manager, token_service, request_context, pref_service,
-      language_histogram,
+      identity_manager, request_context, pref_service, language_histogram,
       base::Bind(
           &data_decoder::SafeJsonParser::Parse,
           content::ServiceManagerConnection::GetForProcess()->GetConnector()),
@@ -454,6 +454,7 @@ ContentSuggestionsServiceFactory::ContentSuggestionsServiceFactory()
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(BookmarkModelFactory::GetInstance());
   DependsOn(HistoryServiceFactory::GetInstance());
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(LargeIconServiceFactory::GetInstance());
 #if BUILDFLAG(ENABLE_OFFLINE_PAGES)
   DependsOn(OfflinePageModelFactory::GetInstance());

components/ntp_snippets/BUILD.gn

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

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc

@@ -15,9 +15,9 @@
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
 #include "components/ntp_snippets/user_classifier.h"
-#include "components/signin/core/browser/signin_manager_base.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
+#include "services/identity/public/cpp/identity_manager.h"
 #include "services/identity/public/cpp/primary_account_access_token_fetcher.h"
 
 using language::UrlLanguageHistogram;
@@ -124,8 +124,7 @@ void FilterCategories(FetchedCategoriesVector* categories,
 }  // namespace
 
 RemoteSuggestionsFetcherImpl::RemoteSuggestionsFetcherImpl(
-    SigninManagerBase* signin_manager,
-    OAuth2TokenService* token_service,
+    identity::IdentityManager* identity_manager,
     scoped_refptr<URLRequestContextGetter> url_request_context_getter,
     PrefService* pref_service,
     UrlLanguageHistogram* language_histogram,
@@ -133,8 +132,7 @@ RemoteSuggestionsFetcherImpl::RemoteSuggestionsFetcherImpl(
     const GURL& api_endpoint,
     const std::string& api_key,
     const UserClassifier* user_classifier)
-    : signin_manager_(signin_manager),
-      token_service_(token_service),
+    : identity_manager_(identity_manager),
       url_request_context_getter_(std::move(url_request_context_getter)),
       language_histogram_(language_histogram),
       parse_json_callback_(parse_json_callback),
@@ -179,7 +177,7 @@ void RemoteSuggestionsFetcherImpl::FetchSnippets(
       .SetUrlRequestContextGetter(url_request_context_getter_)
       .SetUserClassifier(*user_classifier_);
 
-  if (signin_manager_->IsAuthenticated()) {
+  if (identity_manager_->HasPrimaryAccount()) {
     // Signed-in: get OAuth token --> fetch suggestions.
     pending_requests_.emplace(std::move(builder), std::move(callback));
     StartTokenRequest();
@@ -211,7 +209,7 @@ void RemoteSuggestionsFetcherImpl::FetchSnippetsAuthenticated(
     const std::string& oauth_access_token) {
   // TODO(jkrcal, treib): Add unit-tests for authenticated fetches.
   builder.SetUrl(fetch_url_)
-      .SetAuthentication(signin_manager_->GetAuthenticatedAccountId(),
+      .SetAuthentication(identity_manager_->GetPrimaryAccountInfo().account_id,
                          base::StringPrintf(kAuthorizationRequestHeaderFormat,
                                             oauth_access_token.c_str()));
   StartRequest(std::move(builder), std::move(callback));
@@ -234,8 +232,8 @@ void RemoteSuggestionsFetcherImpl::StartTokenRequest() {
   }
 
   OAuth2TokenService::ScopeSet scopes{kContentSuggestionsApiScope};
-  token_fetcher_ = std::make_unique<identity::PrimaryAccountAccessTokenFetcher>(
-      "ntp_snippets", signin_manager_, token_service_, scopes,
+  token_fetcher_ = identity_manager_->CreateAccessTokenFetcherForPrimaryAccount(
+      "ntp_snippets", scopes,
       base::BindOnce(&RemoteSuggestionsFetcherImpl::AccessTokenFetchFinished,
                      base::Unretained(this)),
       identity::PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable);

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h

@@ -20,15 +20,14 @@
 #include "components/ntp_snippets/remote/request_params.h"
 #include "net/url_request/url_request_context_getter.h"
 
-class OAuth2TokenService;
 class PrefService;
-class SigninManagerBase;
 
 namespace base {
 class Value;
 }  // namespace base
 
 namespace identity {
+class IdentityManager;
 class PrimaryAccountAccessTokenFetcher;
 }
 
@@ -43,8 +42,7 @@ class UserClassifier;
 class RemoteSuggestionsFetcherImpl : public RemoteSuggestionsFetcher {
  public:
   RemoteSuggestionsFetcherImpl(
-      SigninManagerBase* signin_manager,
-      OAuth2TokenService* token_service,
+      identity::IdentityManager* identity_manager,
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
       PrefService* pref_service,
       language::UrlLanguageHistogram* language_histogram,
@@ -90,8 +88,7 @@ class RemoteSuggestionsFetcherImpl : public RemoteSuggestionsFetcher {
                      const std::string& error_details);
 
   // Authentication for signed-in users.
-  SigninManagerBase* signin_manager_;
-  OAuth2TokenService* token_service_;
+  identity::IdentityManager* identity_manager_;
 
   std::unique_ptr<identity::PrimaryAccountAccessTokenFetcher> token_fetcher_;
 

components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc

@@ -10,6 +10,7 @@
 
 #include "base/containers/circular_deque.h"
 #include "base/json/json_reader.h"
+#include "base/message_loop/message_loop.h"
 #include "base/optional.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/test_mock_time_task_runner.h"
@@ -34,6 +35,7 @@
 #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 "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -59,7 +61,7 @@ const char kTestChromeContentSuggestionsSignedOutUrl[] =
 const char kTestChromeContentSuggestionsSignedInUrl[] =
     "https://chromecontentsuggestions-pa.googleapis.com/v1/suggestions/fetch";
 
-const char kTestEmail[] = "foo@bar.com";
+const char kTestAccount[] = "foo@bar.com";
 
 // Artificial time delay for JSON parsing.
 const int64_t kTestJsonParsingLatencyMs = 20;
@@ -291,18 +293,25 @@ class RemoteSuggestionsFetcherImplTestBase
     scoped_refptr<net::TestURLRequestContextGetter> request_context_getter =
         new net::TestURLRequestContextGetter(mock_task_runner_.get());
 
-    if (fake_token_service_)
+    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>(
-        utils_.fake_signin_manager(), fake_token_service_.get(),
-        std::move(request_context_getter), utils_.pref_service(), nullptr,
-        base::Bind(&ParseJsonDelayed),
+        identity_manager_.get(), std::move(request_context_getter),
+        utils_.pref_service(), nullptr, base::BindRepeating(&ParseJsonDelayed),
         GetFetchEndpoint(version_info::Channel::STABLE), api_key,
         user_classifier_.get());
 
@@ -311,26 +320,24 @@ class RemoteSuggestionsFetcherImplTestBase
   }
 
   void SignIn() {
-#if defined(OS_CHROMEOS)
-    utils_.fake_signin_manager()->SignIn(kTestEmail);
-#else
-    utils_.fake_signin_manager()->SignIn(kTestEmail, "user", "password");
-#endif
+    identity_manager_->SetPrimaryAccountSynchronouslyForTests(kTestAccount,
+                                                              kTestAccount, "");
   }
 
   void IssueRefreshToken() {
-    fake_token_service_->GetDelegate()->UpdateCredentials(kTestEmail, "token");
+    fake_token_service_->GetDelegate()->UpdateCredentials(kTestAccount,
+                                                          "token");
   }
 
   void IssueOAuth2Token() {
-    fake_token_service_->IssueAllTokensForAccount(kTestEmail, "access_token",
+    fake_token_service_->IssueAllTokensForAccount(kTestAccount, "access_token",
                                                   base::Time::Max());
   }
 
   void CancelOAuth2TokenRequests() {
     fake_token_service_->IssueErrorForAllPendingRequestsForAccount(
-        kTestEmail, GoogleServiceAuthError(
-                        GoogleServiceAuthError::State::REQUEST_CANCELED));
+        kTestAccount, GoogleServiceAuthError(
+                          GoogleServiceAuthError::State::REQUEST_CANCELED));
   }
 
   RemoteSuggestionsFetcher::SnippetsAvailableCallback
@@ -408,6 +415,7 @@ class RemoteSuggestionsFetcherImplTestBase
   // 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_;

ios/chrome/browser/ntp_snippets/BUILD.gn

@@ -33,5 +33,6 @@ source_set("ntp_snippets") {
     "//ios/chrome/common",
     "//ios/web",
     "//net",
+    "//services/identity/public/cpp",
   ]
 }

ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc

@@ -15,6 +15,7 @@
 #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/signin/oauth2_token_service_factory.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 
@@ -46,6 +47,7 @@ IOSChromeContentSuggestionsServiceFactory::
     : BrowserStateKeyedServiceFactory(
           "ContentSuggestionsService",
           BrowserStateDependencyManager::GetInstance()) {
+  DependsOn(IdentityManagerFactory::GetInstance());
   DependsOn(ios::HistoryServiceFactory::GetInstance());
   DependsOn(IOSChromeLargeIconServiceFactory::GetInstance());
   DependsOn(OAuth2TokenServiceFactory::GetInstance());

ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc

@@ -46,7 +46,7 @@
 #include "ios/chrome/browser/history/history_service_factory.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
-#include "ios/chrome/browser/signin/oauth2_token_service_factory.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/common/channel_info.h"
 #include "ios/web/public/browser_state.h"
@@ -162,9 +162,8 @@ void RegisterRemoteSuggestionsProvider(ContentSuggestionsService* service,
   PrefService* prefs = chrome_browser_state->GetPrefs();
   SigninManager* signin_manager =
       ios::SigninManagerFactory::GetForBrowserState(chrome_browser_state);
-
-  OAuth2TokenService* token_service =
-      OAuth2TokenServiceFactory::GetForBrowserState(chrome_browser_state);
+  identity::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForBrowserState(chrome_browser_state);
   scoped_refptr<net::URLRequestContextGetter> request_context =
       browser_state->GetRequestContext();
   base::FilePath database_dir(
@@ -179,8 +178,8 @@ void RegisterRemoteSuggestionsProvider(ContentSuggestionsService* service,
                                 : google_apis::GetNonStableAPIKey();
   }
   auto suggestions_fetcher = base::MakeUnique<RemoteSuggestionsFetcherImpl>(
-      signin_manager, token_service, request_context, prefs, nullptr,
-      base::Bind(&ParseJson), GetFetchEndpoint(GetChannel()), api_key,
+      identity_manager, request_context, prefs, nullptr,
+      base::BindRepeating(&ParseJson), GetFetchEndpoint(GetChannel()), api_key,
       service->user_classifier());
 
   // This pref is also used for logging. If it is changed, change it in the