[NTP Snippets] Have SubscriptionManager use IdentityManager

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

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

I tested the relevant behavior manually as follows, based on
instructions from mamir@chromium.org:

1. Enabled Breaking News Push by turning on "enable-breaking-newspush
on chrome://flags:
https://cs.chromium.org/chromium/src/chrome/browser/about_flags.cc?type=cs&q=kEnableBreakingNewsPushName&sq=package:chromium&l=2497

2. Checked the about:histograms for the feature's metrics by doing
"find in page" for "subscription". Saw no histogram entries after
visiting the NTP when signed out but *did* see histogram entries after
visiting the NTP when signed in. Metrics listed here:
https://cs.chromium.org/chromium/src/components/ntp_snippets/breaking_news/breaking_news_metrics.cc?type=cs&l=17

A followup change will convert the test to use IdentityTestEnvironment.

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

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

@@ -276,11 +276,8 @@ MakeBreakingNewsGCMAppHandlerIfEnabled(
   gcm::GCMDriver* gcm_driver =
       gcm::GCMProfileServiceFactory::GetForProfile(profile)->driver();
 
-  OAuth2TokenService* token_service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
-
-  SigninManagerBase* signin_manager =
-      SigninManagerFactory::GetForProfile(profile);
+  identity::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
 
   scoped_refptr<net::URLRequestContextGetter> request_context =
       content::BrowserContext::GetDefaultStoragePartition(profile)
@@ -296,9 +293,8 @@ MakeBreakingNewsGCMAppHandlerIfEnabled(
   }
 
   auto subscription_manager = base::MakeUnique<SubscriptionManagerImpl>(
-      request_context, pref_service, variations_service, signin_manager,
-      token_service, api_key, locale,
-      GetPushUpdatesSubscriptionEndpoint(chrome::GetChannel()),
+      request_context, pref_service, variations_service, identity_manager,
+      api_key, locale, GetPushUpdatesSubscriptionEndpoint(chrome::GetChannel()),
       GetPushUpdatesUnsubscriptionEndpoint(chrome::GetChannel()));
 
   instance_id::InstanceIDProfileService* instance_id_profile_service =

components/ntp_snippets/breaking_news/subscription_manager_impl.cc

@@ -14,7 +14,6 @@
 #include "components/ntp_snippets/pref_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
-#include "components/signin/core/browser/signin_manager_base.h"
 #include "components/variations/service/variations_service.h"
 #include "net/base/url_util.h"
 #include "services/identity/public/cpp/primary_account_access_token_fetcher.h"
@@ -34,8 +33,7 @@ SubscriptionManagerImpl::SubscriptionManagerImpl(
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
     PrefService* pref_service,
     variations::VariationsService* variations_service,
-    SigninManagerBase* signin_manager,
-    OAuth2TokenService* access_token_service,
+    identity::IdentityManager* identity_manager,
     const std::string& locale,
     const std::string& api_key,
     const GURL& subscribe_url,
@@ -43,17 +41,16 @@ SubscriptionManagerImpl::SubscriptionManagerImpl(
     : url_request_context_getter_(std::move(url_request_context_getter)),
       pref_service_(pref_service),
       variations_service_(variations_service),
-      signin_manager_(signin_manager),
-      access_token_service_(access_token_service),
+      identity_manager_(identity_manager),
       locale_(locale),
       api_key_(api_key),
       subscribe_url_(subscribe_url),
       unsubscribe_url_(unsubscribe_url) {
-  signin_manager_->AddObserver(this);
+  identity_manager_->AddObserver(this);
 }
 
 SubscriptionManagerImpl::~SubscriptionManagerImpl() {
-  signin_manager_->RemoveObserver(this);
+  identity_manager_->RemoveObserver(this);
 }
 
 void SubscriptionManagerImpl::Subscribe(const std::string& subscription_token) {
@@ -61,7 +58,7 @@ void SubscriptionManagerImpl::Subscribe(const std::string& subscription_token) {
   if (request_) {
     request_ = nullptr;
   }
-  if (signin_manager_->IsAuthenticated()) {
+  if (identity_manager_->HasPrimaryAccount()) {
     StartAccessTokenRequest(subscription_token);
   } else {
     SubscribeInternal(subscription_token, /*access_token=*/std::string());
@@ -104,12 +101,13 @@ void SubscriptionManagerImpl::StartAccessTokenRequest(
   }
 
   OAuth2TokenService::ScopeSet scopes = {kContentSuggestionsApiScope};
-  access_token_fetcher_ = std::make_unique<
-      identity::PrimaryAccountAccessTokenFetcher>(
-      "ntp_snippets", signin_manager_, access_token_service_, scopes,
-      base::BindOnce(&SubscriptionManagerImpl::AccessTokenFetchFinished,
-                     base::Unretained(this), subscription_token),
-      identity::PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable);
+  access_token_fetcher_ =
+      identity_manager_->CreateAccessTokenFetcherForPrimaryAccount(
+          "ntp_snippets", scopes,
+          base::BindOnce(&SubscriptionManagerImpl::AccessTokenFetchFinished,
+                         base::Unretained(this), subscription_token),
+          identity::PrimaryAccountAccessTokenFetcher::Mode::
+              kWaitUntilAvailable);
 }
 
 void SubscriptionManagerImpl::AccessTokenFetchFinished(
@@ -193,7 +191,7 @@ bool SubscriptionManagerImpl::NeedsToResubscribe() {
   // Check if authentication state changed after subscription.
   bool is_auth_subscribe = pref_service_->GetBoolean(
       prefs::kBreakingNewsSubscriptionDataIsAuthenticated);
-  bool is_authenticated = signin_manager_->IsAuthenticated();
+  bool is_authenticated = identity_manager_->HasPrimaryAccount();
   return is_auth_subscribe != is_authenticated;
 }
 
@@ -235,14 +233,13 @@ void SubscriptionManagerImpl::DidUnsubscribe(const std::string& new_token,
   }
 }
 
-void SubscriptionManagerImpl::GoogleSigninSucceeded(
-    const std::string& account_id,
-    const std::string& username) {
+void SubscriptionManagerImpl::OnPrimaryAccountSet(
+    const AccountInfo& account_info) {
   SigninStatusChanged();
 }
 
-void SubscriptionManagerImpl::GoogleSignedOut(const std::string& account_id,
-                                              const std::string& username) {
+void SubscriptionManagerImpl::OnPrimaryAccountCleared(
+    const AccountInfo& account_info) {
   SigninStatusChanged();
 }
 

components/ntp_snippets/breaking_news/subscription_manager_impl.h

@@ -11,11 +11,10 @@
 #include "base/memory/ref_counted.h"
 #include "components/ntp_snippets/breaking_news/subscription_json_request.h"
 #include "components/ntp_snippets/breaking_news/subscription_manager.h"
-#include "components/signin/core/browser/signin_manager_base.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/identity/public/cpp/identity_manager.h"
 #include "url/gurl.h"
 
-class OAuth2TokenService;
 class PrefRegistrySimple;
 class PrefService;
 
@@ -35,14 +34,13 @@ namespace ntp_snippets {
 // for subscription. Bookkeeping is required to detect any change (e.g. the
 // token render invalid), and resubscribe accordingly.
 class SubscriptionManagerImpl : public SubscriptionManager,
-                                public SigninManagerBase::Observer {
+                                public identity::IdentityManager::Observer {
  public:
   SubscriptionManagerImpl(
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
       PrefService* pref_service,
       variations::VariationsService* variations_service,
-      SigninManagerBase* signin_manager,
-      OAuth2TokenService* access_token_service,
+      identity::IdentityManager* identity_manager,
       const std::string& locale,
       const std::string& api_key,
       const GURL& subscribe_url,
@@ -65,11 +63,9 @@ class SubscriptionManagerImpl : public SubscriptionManager,
   static void ClearProfilePrefs(PrefService* pref_service);
 
  private:
-  // SigninManagerBase::Observer implementation.
-  void GoogleSigninSucceeded(const std::string& account_id,
-                             const std::string& username) override;
-  void GoogleSignedOut(const std::string& account_id,
-                       const std::string& username) override;
+  // identity:IdentityManager::Observer implementation.
+  void OnPrimaryAccountSet(const AccountInfo& account_info) override;
+  void OnPrimaryAccountCleared(const AccountInfo& account_info) override;
 
   void SigninStatusChanged();
 
@@ -104,8 +100,7 @@ class SubscriptionManagerImpl : public SubscriptionManager,
   variations::VariationsService* const variations_service_;
 
   // Authentication for signed-in users.
-  SigninManagerBase* signin_manager_;
-  OAuth2TokenService* access_token_service_;
+  identity::IdentityManager* identity_manager_;
 
   const std::string locale_;
 

components/ntp_snippets/breaking_news/subscription_manager_impl_unittest.cc

@@ -18,6 +18,8 @@
 #include "net/base/net_errors.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_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -25,7 +27,10 @@ using testing::ElementsAre;
 
 namespace ntp_snippets {
 
+#if !defined(OS_CHROMEOS)
 const char kTestEmail[] = "test@email.com";
+#endif
+
 const char kAPIKey[] = "fakeAPIkey";
 const char kSubscriptionUrl[] = "http://valid-url.test/subscribe";
 const char kSubscriptionUrlSignedIn[] = "http://valid-url.test/subscribe";
@@ -42,7 +47,8 @@ class SubscriptionManagerImplTest
       public OAuth2TokenService::DiagnosticsObserver {
  public:
   SubscriptionManagerImplTest()
-      : request_context_getter_(
+      : identity_manager_(utils_.fake_signin_manager(), utils_.token_service()),
+        request_context_getter_(
             new net::TestURLRequestContextGetter(message_loop_.task_runner())) {
   }
 
@@ -66,13 +72,14 @@ class SubscriptionManagerImplTest
     return utils_.token_service();
   }
 
+  identity::IdentityManager* GetIdentityManager() { return &identity_manager_; }
+
   SigninManagerBase* GetSigninManager() { return utils_.fake_signin_manager(); }
 
   std::unique_ptr<SubscriptionManagerImpl> BuildSubscriptionManager() {
     return std::make_unique<SubscriptionManagerImpl>(
         GetRequestContext(), GetPrefService(),
-        /*variations_service=*/nullptr, GetSigninManager(),
-        GetOAuth2TokenService(),
+        /*variations_service=*/nullptr, GetIdentityManager(),
         /*locale=*/"", kAPIKey, GURL(kSubscriptionUrl),
         GURL(kUnsubscriptionUrl));
   }
@@ -130,19 +137,26 @@ class SubscriptionManagerImplTest
 
 #if !defined(OS_CHROMEOS)
   void SignIn() {
-    utils_.fake_signin_manager()->SignIn(kTestEmail, "user", "pass");
+    identity::MakePrimaryAccountAvailable(utils_.fake_signin_manager(),
+                                          GetOAuth2TokenService(),
+                                          GetIdentityManager(), kTestEmail);
   }
 
-  void SignOut() { utils_.fake_signin_manager()->ForceSignOut(); }
+  void SignOut() {
+    identity::ClearPrimaryAccount(utils_.fake_signin_manager(),
+                                  GetIdentityManager());
+  }
 #endif  // !defined(OS_CHROMEOS)
 
   void IssueRefreshToken(FakeProfileOAuth2TokenService* auth_token_service) {
-    auth_token_service->GetDelegate()->UpdateCredentials(kTestEmail, "token");
+    auth_token_service->GetDelegate()->UpdateCredentials(
+        GetIdentityManager()->GetPrimaryAccountInfo().account_id, "token");
   }
 
   void IssueAccessToken(FakeProfileOAuth2TokenService* auth_token_service) {
-    auth_token_service->IssueAllTokensForAccount(kTestEmail, "access_token",
-                                                 base::Time::Max());
+    auth_token_service->IssueAllTokensForAccount(
+        GetIdentityManager()->GetPrimaryAccountInfo().account_id,
+        "access_token", base::Time::Max());
   }
 
   void set_on_access_token_request_callback(base::OnceClosure callback) {
@@ -178,6 +192,7 @@ class SubscriptionManagerImplTest
 
   base::MessageLoop message_loop_;
   test::RemoteSuggestionsTestUtils utils_;
+  identity::IdentityManager identity_manager_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
   net::TestURLFetcherFactory url_fetcher_factory_;
   base::OnceClosure on_access_token_request_callback_;