[NTP::Push] Support Authenticated subscription requests.

Bug: 735465
Change-Id: I4e1aa8d74be22dd96479f1d5c53e11c89358cfa0
Reviewed-on: https://chromium-review.googlesource.com/562757
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: vitaliii <vitaliii@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487036}

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

@@ -376,18 +376,27 @@ void RegisterForeignSessionsProviderIfEnabled(
   service->RegisterProvider(std::move(provider));
 }
 
-void SubscribeForGCMPushUpdates(PrefService* pref_service,
-                                ContentSuggestionsService* service,
-                                Profile* profile) {
+void SubscribeForGCMPushUpdates(
+    PrefService* pref_service,
+    ContentSuggestionsService* content_suggestions_service,
+    Profile* profile) {
+  // TODO(mamir): Either pass all params from outside or pass only profile and
+  // create them inside the method, but be consistent.
   gcm::GCMDriver* gcm_driver =
       gcm::GCMProfileServiceFactory::GetForProfile(profile)->driver();
 
+  OAuth2TokenService* token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
+
+  SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfile(profile);
+
   scoped_refptr<net::URLRequestContextGetter> request_context =
       content::BrowserContext::GetDefaultStoragePartition(profile)
           ->GetURLRequestContext();
 
   auto subscription_manager = base::MakeUnique<SubscriptionManager>(
-      request_context, pref_service,
+      request_context, pref_service, signin_manager, token_service,
       GetPushUpdatesSubscriptionEndpoint(chrome::GetChannel()),
       GetPushUpdatesUnsubscriptionEndpoint(chrome::GetChannel()));
 
@@ -411,9 +420,10 @@ void SubscribeForGCMPushUpdates(PrefService* pref_service,
   base::FilePath database_dir(
       profile->GetPath().Append(ntp_snippets::kBreakingNewsDatabaseFolder));
   auto provider = base::MakeUnique<BreakingNewsSuggestionsProvider>(
-      service, std::move(handler), base::MakeUnique<base::DefaultClock>(),
+      content_suggestions_service, std::move(handler),
+      base::MakeUnique<base::DefaultClock>(),
       base::MakeUnique<RemoteSuggestionsDatabase>(database_dir, task_runner));
-  service->RegisterProvider(std::move(provider));
+  content_suggestions_service->RegisterProvider(std::move(provider));
 }
 
 }  // namespace

components/ntp_snippets/BUILD.gn

@@ -210,6 +210,7 @@ source_set("unit_tests") {
     "//components/signin/core/common",
     "//components/strings",
     "//components/sync:test_support_driver",
+    "//components/sync_preferences:test_support",
     "//components/sync_sessions",
     "//components/variations:test_support",
     "//components/web_resource:web_resource",

components/ntp_snippets/DEPS

@@ -13,6 +13,7 @@ include_rules = [
   "+components/reading_list",
   "+components/signin",
   "+components/strings/grit/components_strings.h",
+  "+components/sync_preferences/testing_pref_service_syncable.h",
   "+components/sync/driver",
   "+components/url_formatter",
   "+components/variations",

components/ntp_snippets/breaking_news/breaking_news_gcm_app_handler.cc

@@ -60,20 +60,18 @@ void BreakingNewsGCMAppHandler::StopListening() {
   DCHECK_EQ(gcm_driver_->GetAppHandler(kBreakingNewsGCMAppID), this);
   gcm_driver_->RemoveAppHandler(kBreakingNewsGCMAppID);
   on_new_content_callback_ = OnNewContentCallback();
-  // TODO(mamir): Check which token should be used for unsubscription when
-  // handling change in the token.
-  std::string token = pref_service_->GetString(
-      ntp_snippets::prefs::kBreakingNewsGCMSubscriptionTokenCache);
-  subscription_manager_->Unsubscribe(token);
+  subscription_manager_->Unsubscribe();
 }
 
 void BreakingNewsGCMAppHandler::Subscribe() {
-  std::string token = pref_service_->GetString(
-      ntp_snippets::prefs::kBreakingNewsGCMSubscriptionTokenCache);
+  // TODO(mamir): This logic should be moved to the SubscriptionManager.
+  std::string token =
+      pref_service_->GetString(prefs::kBreakingNewsGCMSubscriptionTokenCache);
   // If a token has been already obtained, subscribe directly at the content
-  // suggestions server.
+  // suggestions server. Otherwise, obtain a GCM token first.
   if (!token.empty()) {
-    if (!subscription_manager_->IsSubscribed()) {
+    if (!subscription_manager_->IsSubscribed() ||
+        subscription_manager_->NeedsToResubscribe()) {
       subscription_manager_->Subscribe(token);
     }
     return;
@@ -81,19 +79,19 @@ void BreakingNewsGCMAppHandler::Subscribe() {
 
   instance_id_driver_->GetInstanceID(kBreakingNewsGCMAppID)
       ->GetToken(kBreakingNewsGCMSenderId, kGCMScope,
-                 std::map<std::string, std::string>() /* options */,
+                 /*options=*/std::map<std::string, std::string>(),
                  base::Bind(&BreakingNewsGCMAppHandler::DidSubscribe,
                             weak_ptr_factory_.GetWeakPtr()));
 }
 
-void BreakingNewsGCMAppHandler::DidSubscribe(const std::string& subscription_id,
-                                             InstanceID::Result result) {
+void BreakingNewsGCMAppHandler::DidSubscribe(
+    const std::string& subscription_token,
+    InstanceID::Result result) {
   switch (result) {
     case InstanceID::SUCCESS:
-      pref_service_->SetString(
-          ntp_snippets::prefs::kBreakingNewsGCMSubscriptionTokenCache,
-          subscription_id);
-      subscription_manager_->Subscribe(subscription_id);
+      pref_service_->SetString(prefs::kBreakingNewsGCMSubscriptionTokenCache,
+                               subscription_token);
+      subscription_manager_->Subscribe(subscription_token);
       return;
     case InstanceID::INVALID_PARAMETER:
     case InstanceID::DISABLED:
@@ -112,8 +110,7 @@ void BreakingNewsGCMAppHandler::DidSubscribe(const std::string& subscription_id,
 void BreakingNewsGCMAppHandler::ShutdownHandler() {}
 
 void BreakingNewsGCMAppHandler::OnStoreReset() {
-  pref_service_->ClearPref(
-      ntp_snippets::prefs::kBreakingNewsGCMSubscriptionTokenCache);
+  pref_service_->ClearPref(prefs::kBreakingNewsGCMSubscriptionTokenCache);
 }
 
 void BreakingNewsGCMAppHandler::OnMessage(const std::string& app_id,

components/ntp_snippets/breaking_news/breaking_news_gcm_app_handler.h

@@ -77,7 +77,7 @@ class BreakingNewsGCMAppHandler : public BreakingNewsListener,
   void Subscribe();
 
   // Called after the subscription is obtained from the GCM server.
-  void DidSubscribe(const std::string& subscription_id,
+  void DidSubscribe(const std::string& subscription_token,
                     instance_id::InstanceID::Result result);
 
   // Called after successfully parsing the received suggestion JSON.

components/ntp_snippets/breaking_news/subscription_json_request.cc

@@ -26,13 +26,7 @@ namespace internal {
 
 SubscriptionJsonRequest::SubscriptionJsonRequest() = default;
 
-SubscriptionJsonRequest::~SubscriptionJsonRequest() {
-  if (!request_completed_callback_.is_null()) {
-    std::move(request_completed_callback_)
-        .Run(ntp_snippets::Status(ntp_snippets::StatusCode::TEMPORARY_ERROR,
-                                  "cancelled"));
-  }
-}
+SubscriptionJsonRequest::~SubscriptionJsonRequest() = default;
 
 void SubscriptionJsonRequest::Start(CompletedCallback callback) {
   DCHECK(request_completed_callback_.is_null()) << "Request already running!";
@@ -49,18 +43,15 @@ void SubscriptionJsonRequest::OnURLFetchComplete(const URLFetcher* source) {
 
   if (!status.is_success()) {
     std::move(request_completed_callback_)
-        .Run(ntp_snippets::Status(
-            ntp_snippets::StatusCode::TEMPORARY_ERROR,
-            base::StringPrintf("Internal Error: %d", status.error())));
+        .Run(Status(StatusCode::TEMPORARY_ERROR,
+                    base::StringPrintf("Network Error: %d", status.error())));
   } else if (response != net::HTTP_OK) {
     std::move(request_completed_callback_)
-        .Run(ntp_snippets::Status(
-            ntp_snippets::StatusCode::PERMANENT_ERROR,
-            base::StringPrintf("HTTP Error: %d", response)));
+        .Run(Status(StatusCode::PERMANENT_ERROR,
+                    base::StringPrintf("HTTP Error: %d", response)));
   } else {
     std::move(request_completed_callback_)
-        .Run(ntp_snippets::Status(ntp_snippets::StatusCode::SUCCESS,
-                                  std::string()));
+        .Run(Status(StatusCode::SUCCESS, std::string()));
   }
 }
 
@@ -80,9 +71,9 @@ SubscriptionJsonRequest::Builder::Build() const {
   request->url_fetcher_ = BuildURLFetcher(request.get(), headers, body);
 
   // Log the request for debugging network issues.
-  VLOG(1) << "Sending a subscription request to " << url_ << ":\n"
-          << headers << "\n"
-          << body;
+  DVLOG(1) << "Building a subscription request to " << url_ << ":\n"
+           << headers << "\n"
+           << body;
 
   return request;
 }
@@ -106,10 +97,20 @@ SubscriptionJsonRequest::Builder::SetUrlRequestContextGetter(
   return *this;
 }
 
+SubscriptionJsonRequest::Builder&
+SubscriptionJsonRequest::Builder::SetAuthenticationHeader(
+    const std::string& auth_header) {
+  auth_header_ = auth_header;
+  return *this;
+}
+
 std::string SubscriptionJsonRequest::Builder::BuildHeaders() const {
   HttpRequestHeaders headers;
-  headers.SetHeader("Content-Type", "application/json; charset=UTF-8");
-
+  headers.SetHeader(HttpRequestHeaders::kContentType,
+                    "application/json; charset=UTF-8");
+  if (!auth_header_.empty()) {
+    headers.SetHeader(HttpRequestHeaders::kAuthorization, auth_header_);
+  }
   // Add X-Client-Data header with experiment IDs from field trials.
   // Note: It's OK to pass |is_signed_in| false if it's unknown, as it does
   // not affect transmission of experiments coming from the variations server.

components/ntp_snippets/breaking_news/subscription_json_request.h

@@ -27,10 +27,9 @@ class SubscriptionJsonRequest : public net::URLFetcherDelegate {
  public:
   // A client can expect a message in the status only, if there was any error
   // during the subscription. In successful cases, it will be an empty string.
-  using CompletedCallback =
-      base::OnceCallback<void(const ntp_snippets::Status& status)>;
+  using CompletedCallback = base::OnceCallback<void(const Status& status)>;
 
-  // Builds non-authenticated SubscriptionJsonRequests.
+  // Builds non-authenticated and authenticated SubscriptionJsonRequests.
   class Builder {
    public:
     Builder();
@@ -44,6 +43,7 @@ class SubscriptionJsonRequest : public net::URLFetcherDelegate {
     Builder& SetUrl(const GURL& url);
     Builder& SetUrlRequestContextGetter(
         const scoped_refptr<net::URLRequestContextGetter>& context_getter);
+    Builder& SetAuthenticationHeader(const std::string& auth_header);
 
    private:
     std::string BuildHeaders() const;
@@ -53,18 +53,21 @@ class SubscriptionJsonRequest : public net::URLFetcherDelegate {
         const std::string& headers,
         const std::string& body) const;
 
-    // GCM subscribtion token obtain from GCM driver (instanceID::getToken())
+    // GCM subscription token obtained from GCM driver (instanceID::getToken()).
     std::string token_;
-    // TODO(mamir): Additional fields to be added: country, language
+    // TODO(mamir): Additional fields to be added: country, language.
 
     GURL url_;
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+    std::string auth_header_;
+
+    DISALLOW_COPY_AND_ASSIGN(Builder);
   };
 
   ~SubscriptionJsonRequest() override;
 
-  // Starts an async request. The callback is invoked when the request succeeds,
-  // fails or gets destroyed.
+  // Starts an async request. The callback is invoked when the request succeeds
+  // or fails. The callback is not called if the request is destroyed.
   void Start(CompletedCallback callback);
 
  private:

components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc

@@ -95,8 +95,6 @@ TEST_F(SubscriptionJsonRequestTest, BuildRequest) {
   GURL url("http://valid-url.test");
 
   base::MockCallback<SubscriptionJsonRequest::CompletedCallback> callback;
-  ntp_snippets::Status status(StatusCode::SUCCESS, "initial");
-  EXPECT_CALL(callback, Run(_)).WillOnce(SaveArg<0>(&status));
 
   SubscriptionJsonRequest::Builder builder;
   std::unique_ptr<SubscriptionJsonRequest> request =
@@ -126,13 +124,12 @@ TEST_F(SubscriptionJsonRequestTest, BuildRequest) {
   EXPECT_THAT(url_fetcher->upload_data(), EqualsJSON(expected_body));
 }
 
-TEST_F(SubscriptionJsonRequestTest, InvokesCallbackWhenCancelled) {
+TEST_F(SubscriptionJsonRequestTest, ShouldNotInvokeCallbackWhenCancelled) {
   std::string token = "1234567890";
   GURL url("http://valid-url.test");
 
   base::MockCallback<SubscriptionJsonRequest::CompletedCallback> callback;
-  ntp_snippets::Status status(StatusCode::SUCCESS, "initial");
-  EXPECT_CALL(callback, Run(_)).WillOnce(SaveArg<0>(&status));
+  EXPECT_CALL(callback, Run(_)).Times(0);
 
   SubscriptionJsonRequest::Builder builder;
   std::unique_ptr<SubscriptionJsonRequest> request =
@@ -144,9 +141,6 @@ TEST_F(SubscriptionJsonRequestTest, InvokesCallbackWhenCancelled) {
 
   // Destroy the request before getting any response.
   request.reset();
-
-  EXPECT_EQ(status.code, StatusCode::TEMPORARY_ERROR);
-  EXPECT_EQ(status.message, "cancelled");
 }
 
 TEST_F(SubscriptionJsonRequestTest, SubscribeWithoutErrors) {

components/ntp_snippets/breaking_news/subscription_manager.h

@@ -7,10 +7,13 @@
 
 #include "components/ntp_snippets/breaking_news/subscription_json_request.h"
 #include "components/prefs/pref_registry_simple.h"
+#include "components/signin/core/browser/signin_manager_base.h"
 #include "components/version_info/version_info.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
 
+class AccessTokenFetcher;
+class OAuth2TokenService;
 class PrefRegistrySimple;
 class PrefService;
 
@@ -34,38 +37,67 @@ class SubscriptionManager {
   SubscriptionManager(
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
       PrefService* pref_service,
+      SigninManagerBase* signin_manager,
+      OAuth2TokenService* access_token_service,
       const GURL& subscribe_url,
       const GURL& unsubscribe_url);
 
   ~SubscriptionManager();
 
   void Subscribe(const std::string& token);
-  bool CanSubscribeNow();
-  void Unsubscribe(const std::string& token);
-  bool CanUnsubscribeNow();
+  void Unsubscribe();
   bool IsSubscribed();
 
+  void Resubscribe(const std::string& new_token);
+
+  // Checks if some data that has been used when subscribing has changed. For
+  // example, the user has signed in.
+  bool NeedsToResubscribe();
+
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
  private:
-  std::string subscription_token_;
-  std::string unsubscription_token_;
+  class SigninObserver;
+
+  void SigninStatusChanged();
+
+  void DidSubscribe(const std::string& subscription_token,
+                    bool is_authenticated,
+                    const Status& status);
+  void DidUnsubscribe(const std::string& new_token, const Status& status);
+  void SubscribeInternal(const std::string& subscription_token,
+                         const std::string& access_token);
+
+  // If |new_token| is empty, this will just unsubscribe. If |new_token| is
+  // non-empty, a subscription request with the |new_token| will be started upon
+  // successful unsubscription.
+  void ResubscribeInternal(const std::string& old_token,
+                           const std::string& new_token);
+
+  // |subscription_token| is the token when subscribing after obtaining the
+  // access token.
+  void StartAccessTokenRequest(const std::string& subscription_token);
+  void AccessTokenFetchFinished(const std::string& subscription_token,
+                                const GoogleServiceAuthError& error,
+                                const std::string& access_token);
 
   // Holds the URL request context.
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
 
-  std::unique_ptr<internal::SubscriptionJsonRequest> subscription_request_;
-  std::unique_ptr<internal::SubscriptionJsonRequest> unsubscription_request_;
+  std::unique_ptr<internal::SubscriptionJsonRequest> request_;
+  std::unique_ptr<AccessTokenFetcher> access_token_fetcher_;
 
   PrefService* pref_service_;
 
+  // Authentication for signed-in users.
+  SigninManagerBase* signin_manager_;
+  std::unique_ptr<SigninObserver> signin_observer_;
+  OAuth2TokenService* access_token_service_;
+
   // API endpoint for subscribing and unsubscribing.
   const GURL subscribe_url_;
   const GURL unsubscribe_url_;
 
-  void DidSubscribe(const ntp_snippets::Status& status);
-  void DidUnsubscribe(const ntp_snippets::Status& status);
-
   DISALLOW_COPY_AND_ASSIGN(SubscriptionManager);
 };
 }

components/ntp_snippets/ntp_snippets_constants.cc

@@ -12,6 +12,9 @@ const base::FilePath::CharType kDatabaseFolder[] =
 const base::FilePath::CharType kBreakingNewsDatabaseFolder[] =
     FILE_PATH_LITERAL("NTPBreakingNews");
 
+const char kContentSuggestionsApiScope[] =
+    "https://www.googleapis.com/auth/chrome-content-suggestions";
+
 const char kContentSuggestionsServer[] =
     "https://chromecontentsuggestions-pa.googleapis.com/v1/suggestions/fetch";
 const char kContentSuggestionsStagingServer[] =

components/ntp_snippets/ntp_snippets_constants.h

@@ -16,6 +16,9 @@ extern const base::FilePath::CharType kDatabaseFolder[];
 // TODO(mamir): Check if the same DB can be used.
 extern const base::FilePath::CharType kBreakingNewsDatabaseFolder[];
 
+// OAuth access token scope.
+extern const char kContentSuggestionsApiScope[];
+
 // Server endpoints for fetching snippets.
 extern const char kContentSuggestionsServer[];         // used on stable/beta
 extern const char kContentSuggestionsStagingServer[];  // used on dev/canary

components/ntp_snippets/pref_names.cc

@@ -84,6 +84,9 @@ const char kClickBasedCategoryRankerLastDecayTime[] =
 const char kBreakingNewsSubscriptionDataToken[] =
     "ntp_suggestions.breaking_news_subscription_data.token";
 
+const char kBreakingNewsSubscriptionDataIsAuthenticated[] =
+    "ntp_suggestions.breaking_news_subscription_data.is_authenticated";
+
 const char kBreakingNewsGCMSubscriptionTokenCache[] =
     "ntp_suggestions.breaking_news_gcm_subscription_token_cache";
 

components/ntp_snippets/pref_names.h

@@ -93,10 +93,13 @@ extern const char kClickBasedCategoryRankerLastDecayTime[];
 // The folllowing prefs hold the data used when subscribing for content
 // suggestions via GCM push updates. They are stored in pref such that in case
 // of change (e.g. the token renders invalid), re-subscription is required.
+// They are stored in prefs for persisting them across Chrome restarts.
 ///////////////////////////////////////////////////////////////////////////////
 // The pref name for the subscription token used when subscription for
 // breaking news push updates.
 extern const char kBreakingNewsSubscriptionDataToken[];
+// The pref name for whether the subscription is authenticated or not.
+extern const char kBreakingNewsSubscriptionDataIsAuthenticated[];
 //////////////////////// End of breaking news subscription-related prefs.
 
 // The pref name for the subscription token received from the gcm server. As

components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc

@@ -13,6 +13,7 @@
 #include "base/values.h"
 #include "components/language/core/browser/url_language_histogram.h"
 #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/access_token_fetcher.h"
 #include "components/signin/core/browser/signin_manager_base.h"
@@ -32,8 +33,6 @@ using internal::JsonRequest;
 
 namespace {
 
-const char kContentSuggestionsApiScope[] =
-    "https://www.googleapis.com/auth/chrome-content-suggestions";
 const char kSnippetsServerNonAuthorizedFormat[] = "%s?key=%s";
 const char kAuthorizationRequestHeaderFormat[] = "Bearer %s";
 

components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/features.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
@@ -298,7 +299,13 @@ class RemoteSuggestionsFetcherImplTestBase : public testing::Test {
     fetcher_->SetClockForTesting(mock_task_runner_->GetMockClock());
   }
 
-  void SignIn() { utils_.fake_signin_manager()->SignIn(kTestEmail); }
+  void SignIn() {
+#if defined(OS_CHROMEOS)
+    utils_.fake_signin_manager()->SignIn(kTestEmail);
+#else
+    utils_.fake_signin_manager()->SignIn(kTestEmail, "user", "password");
+#endif
+  }
 
   void IssueRefreshToken() {
     fake_token_service_->GetDelegate()->UpdateCredentials(kTestEmail, "token");

components/ntp_snippets/remote/remote_suggestions_scheduler_impl_unittest.cc

@@ -29,8 +29,8 @@
 #include "components/ntp_snippets/remote/test_utils.h"
 #include "components/ntp_snippets/status.h"
 #include "components/ntp_snippets/user_classifier.h"
-#include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/variations/variations_params_manager.h"
 #include "components/web_resource/web_resource_pref_names.h"
 #include "net/base/network_change_notifier.h"

components/ntp_snippets/remote/remote_suggestions_status_service_unittest.cc

@@ -7,16 +7,17 @@
 #include <memory>
 
 #include "base/memory/ptr_util.h"
+#include "build/build_config.h"
 #include "components/ntp_snippets/features.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/test_utils.h"
-#include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/fake_signin_manager.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/signin/core/common/signin_pref_names.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/variations/variations_params_manager.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -48,7 +49,11 @@ TEST_F(RemoteSuggestionsStatusServiceTest, NoSigninNeeded) {
             service->GetStatusFromDeps());
 
   // One can still sign in.
+#if defined(OS_CHROMEOS)
   utils_.fake_signin_manager()->SignIn("foo@bar.com");
+#else
+  utils_.fake_signin_manager()->SignIn("foo@bar.com", "user", "pass");
+#endif
   EXPECT_EQ(RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN,
             service->GetStatusFromDeps());
 }
@@ -66,7 +71,11 @@ TEST_F(RemoteSuggestionsStatusServiceTest, DisabledViaPref) {
             service->GetStatusFromDeps());
 
   // The other dependencies shouldn't matter anymore.
+#if defined(OS_CHROMEOS)
   utils_.fake_signin_manager()->SignIn("foo@bar.com");
+#else
+  utils_.fake_signin_manager()->SignIn("foo@bar.com", "user", "pass");
+#endif
   EXPECT_EQ(RemoteSuggestionsStatus::EXPLICITLY_DISABLED,
             service->GetStatusFromDeps());
 }

components/ntp_snippets/remote/test_utils.cc

@@ -7,10 +7,9 @@
 #include <memory>
 
 #include "base/memory/ptr_util.h"
-#include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/fake_signin_manager.h"
+#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "components/sync/driver/fake_sync_service.h"
@@ -48,24 +47,37 @@ syncer::ModelTypeSet FakeSyncService::GetActiveDataTypes() const {
 }
 
 RemoteSuggestionsTestUtils::RemoteSuggestionsTestUtils()
-    : pref_service_(base::MakeUnique<TestingPrefServiceSimple>()) {
-  pref_service_->registry()->RegisterStringPref(prefs::kGoogleServicesAccountId,
-                                                std::string());
-  pref_service_->registry()->RegisterStringPref(
-      prefs::kGoogleServicesLastAccountId, std::string());
-  pref_service_->registry()->RegisterStringPref(
-      prefs::kGoogleServicesLastUsername, std::string());
+    : pref_service_(base::MakeUnique<TestingPrefServiceSyncable>()) {
+  AccountTrackerService::RegisterPrefs(pref_service_->registry());
+
+#if defined(OS_CHROMEOS)
+  SigninManagerBase::RegisterProfilePrefs(pref_service_->registry());
+  SigninManagerBase::RegisterPrefs(pref_service_->registry());
+#else
+  SigninManager::RegisterProfilePrefs(pref_service_->registry());
+  SigninManager::RegisterPrefs(pref_service_->registry());
+#endif  // OS_CHROMEOS
+
+  token_service_ = base::MakeUnique<FakeProfileOAuth2TokenService>();
   signin_client_ = base::MakeUnique<TestSigninClient>(pref_service_.get());
   account_tracker_ = base::MakeUnique<AccountTrackerService>();
+  account_tracker_->Initialize(signin_client_.get());
   fake_sync_service_ = base::MakeUnique<FakeSyncService>();
+
   ResetSigninManager();
 }
 
 RemoteSuggestionsTestUtils::~RemoteSuggestionsTestUtils() = default;
 
 void RemoteSuggestionsTestUtils::ResetSigninManager() {
+#if defined(OS_CHROMEOS)
   fake_signin_manager_ = base::MakeUnique<FakeSigninManagerBase>(
       signin_client_.get(), account_tracker_.get());
+#else
+  fake_signin_manager_ = base::MakeUnique<FakeSigninManager>(
+      signin_client_.get(), token_service_.get(), account_tracker_.get(),
+      /*cookie_manager_service=*/nullptr);
+#endif
 }
 
 }  // namespace test

components/ntp_snippets/remote/test_utils.h

@@ -7,14 +7,25 @@
 
 #include <memory>
 
+#include "build/build_config.h"
+#include "components/signin/core/browser/fake_signin_manager.h"
 #include "components/sync/driver/fake_sync_service.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class AccountTrackerService;
-class FakeSigninManagerBase;
-class TestingPrefServiceSimple;
+class FakeProfileOAuth2TokenService;
 class TestSigninClient;
 
+using sync_preferences::TestingPrefServiceSyncable;
+
+#if defined(OS_CHROMEOS)
+// ChromeOS doesn't have SigninManager.
+using SigninManagerForTest = FakeSigninManagerBase;
+#else
+using SigninManagerForTest = FakeSigninManager;
+#endif  // OS_CHROMEOS
+
 namespace ntp_snippets {
 namespace test {
 
@@ -46,15 +57,19 @@ class RemoteSuggestionsTestUtils {
   void ResetSigninManager();
 
   FakeSyncService* fake_sync_service() { return fake_sync_service_.get(); }
-  FakeSigninManagerBase* fake_signin_manager() {
+  SigninManagerForTest* fake_signin_manager() {
     return fake_signin_manager_.get();
   }
-  TestingPrefServiceSimple* pref_service() { return pref_service_.get(); }
+  TestingPrefServiceSyncable* pref_service() { return pref_service_.get(); }
+  FakeProfileOAuth2TokenService* token_service() {
+    return token_service_.get();
+  }
 
  private:
-  std::unique_ptr<FakeSigninManagerBase> fake_signin_manager_;
+  std::unique_ptr<SigninManagerForTest> fake_signin_manager_;
   std::unique_ptr<FakeSyncService> fake_sync_service_;
-  std::unique_ptr<TestingPrefServiceSimple> pref_service_;
+  std::unique_ptr<TestingPrefServiceSyncable> pref_service_;
+  std::unique_ptr<FakeProfileOAuth2TokenService> token_service_;
   std::unique_ptr<TestSigninClient> signin_client_;
   std::unique_ptr<AccountTrackerService> account_tracker_;
 };