Eliminate SigninClient::AddCookieChangeCallback()

While working on GaiaCookieManagerService, I noticed that
SigninClient::AddCookieChangeCallback() is unnecessary to call out to
the embedder for: the only embedder-specific parameterization of the
//components-level SigninCookieChangeSubscription is for the
URLRequestContextGetter, which is already available to the component
via SigninClient::GetURLRequestContext().

This CL eliminates that client method in favor of having
GaiaCookieManagerService construct SigninCookieChangeSubscription
directly. I verified that all production clients were supplying the same
URLRequestContextGetter that they return in their implementation of
SigninClient::GetURLRequestContext().

TBR=jzw@chromium.org

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ic8a9d0c6e3e63cf531408eb6ab46db65bcf3e668
Reviewed-on: https://chromium-review.googlesource.com/1145312
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577467}

chrome/browser/signin/chrome_signin_client.cc

@@ -40,7 +40,6 @@
 #include "components/signin/core/browser/profile_management_switches.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_buildflags.h"
-#include "components/signin/core/browser/signin_cookie_change_subscription.h"
 #include "components/signin/core/browser/signin_header_helper.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 #include "components/signin/core/browser/signin_switches.h"
@@ -249,18 +248,6 @@ void ChromeSigninClient::RemoveContentSettingsObserver(
       ->RemoveObserver(observer);
 }
 
-std::unique_ptr<SigninClient::CookieChangeSubscription>
-ChromeSigninClient::AddCookieChangeCallback(
-    const GURL& url,
-    const std::string& name,
-    net::CookieChangeCallback callback) {
-  scoped_refptr<net::URLRequestContextGetter> context_getter =
-      profile_->GetRequestContext();
-  DCHECK(context_getter.get());
-  return std::make_unique<SigninCookieChangeSubscription>(
-      context_getter, url, name, std::move(callback));
-}
-
 void ChromeSigninClient::OnSignedIn(const std::string& account_id,
                                     const std::string& gaia_id,
                                     const std::string& username,

chrome/browser/signin/chrome_signin_client.h

@@ -71,10 +71,6 @@ class ChromeSigninClient
   // <Build Info> <OS> <Version number> (<Last change>)<channel or "-devel">
   // If version information is unavailable, returns "invalid."
   std::string GetProductVersion() override;
-  std::unique_ptr<CookieChangeSubscription> AddCookieChangeCallback(
-      const GURL& url,
-      const std::string& name,
-      net::CookieChangeCallback callback) override;
   void OnSignedIn(const std::string& account_id,
                   const std::string& gaia_id,
                   const std::string& username,

components/signin/core/browser/gaia_cookie_manager_service.cc

@@ -18,6 +18,7 @@
 #include "base/values.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/signin/core/browser/account_tracker_service.h"
+#include "components/signin/core/browser/signin_cookie_change_subscription.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_constants.h"
@@ -350,10 +351,19 @@ GaiaCookieManagerService::~GaiaCookieManagerService() {
 }
 
 void GaiaCookieManagerService::Init() {
-  cookie_change_subscription_ = signin_client_->AddCookieChangeCallback(
-      GaiaUrls::GetInstance()->google_url(), kGaiaCookieName,
-      base::BindRepeating(&GaiaCookieManagerService::OnCookieChange,
-                          base::Unretained(this)));
+  scoped_refptr<net::URLRequestContextGetter> context_getter =
+      signin_client_->GetURLRequestContext();
+
+  // NOTE: |context_getter| can be nullptr when TestSigninClient is used in
+  // testing contexts.
+  if (context_getter) {
+    cookie_change_subscription_ =
+        std::make_unique<SigninCookieChangeSubscription>(
+            context_getter, GaiaUrls::GetInstance()->google_url(),
+            kGaiaCookieName,
+            base::BindRepeating(&GaiaCookieManagerService::OnCookieChange,
+                                base::Unretained(this)));
+  }
 }
 
 void GaiaCookieManagerService::Shutdown() {

components/signin/core/browser/gaia_cookie_manager_service.h

@@ -25,6 +25,7 @@ class GaiaAuthFetcher;
 class GaiaCookieRequest;
 class GoogleServiceAuthError;
 class OAuth2TokenService;
+class SigninCookieChangeSubscription;
 
 namespace network {
 class SharedURLLoaderFactory;
@@ -309,8 +310,7 @@ class GaiaCookieManagerService : public KeyedService,
   std::string access_token_;
 
   // Subscription to be called whenever the GAIA cookies change.
-  std::unique_ptr<SigninClient::CookieChangeSubscription>
-      cookie_change_subscription_;
+  std::unique_ptr<SigninCookieChangeSubscription> cookie_change_subscription_;
 
   // A worklist for this class. Stores any pending requests that couldn't be
   // executed right away, since this class only permits one request to be

components/signin/core/browser/signin_client.h

@@ -42,12 +42,6 @@ class SharedURLLoaderFactory;
 // embedder.
 class SigninClient : public KeyedService {
  public:
-  // The subcription for cookie changed notifications.
-  class CookieChangeSubscription {
-   public:
-    virtual ~CookieChangeSubscription() = default;
-  };
-
   ~SigninClient() override = default;
 
   // If |for_ephemeral| is true, special kind of device ID for ephemeral users
@@ -90,15 +84,6 @@ class SigninClient : public KeyedService {
   // Signin component is being used.
   virtual std::string GetProductVersion() = 0;
 
-  // Adds a callback to be called each time a cookie for |url| with name |name|
-  // changes.
-  // Note that |callback| will always be called on the thread that
-  // |AddCookieChangeCallback| was called on.
-  virtual std::unique_ptr<CookieChangeSubscription> AddCookieChangeCallback(
-      const GURL& url,
-      const std::string& name,
-      net::CookieChangeCallback callback) = 0;
-
   // Called after Google signin has succeeded.
   virtual void OnSignedIn(const std::string& account_id,
                           const std::string& gaia_id,

components/signin/core/browser/signin_cookie_change_subscription.h

@@ -11,15 +11,14 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/task_runner.h"
 #include "base/threading/thread_checker.h"
-#include "components/signin/core/browser/signin_client.h"
 #include "net/cookies/cookie_change_dispatcher.h"
 #include "net/url_request/url_request_context_getter.h"
 
 // The subscription for cookie changes. This class lives on the main thread.
 class SigninCookieChangeSubscription
-    : public SigninClient::CookieChangeSubscription,
-      public base::SupportsWeakPtr<SigninCookieChangeSubscription> {
+    : public base::SupportsWeakPtr<SigninCookieChangeSubscription> {
  public:
   // Creates a cookie change subscription and registers for cookie changed
   // events.
@@ -28,7 +27,7 @@ class SigninCookieChangeSubscription
       const GURL& url,
       const std::string& name,
       net::CookieChangeCallback callback);
-  ~SigninCookieChangeSubscription() override;
+  ~SigninCookieChangeSubscription();
 
  private:
   // Holder of a cookie store cookie changed subscription.

components/signin/core/browser/test_signin_client.cc

@@ -79,13 +79,6 @@ void TestSigninClient::LoadTokenDatabase() {
   database_->Init();
 }
 
-std::unique_ptr<SigninClient::CookieChangeSubscription>
-TestSigninClient::AddCookieChangeCallback(const GURL& url,
-                                          const std::string& name,
-                                          net::CookieChangeCallback callback) {
-  return std::make_unique<SigninClient::CookieChangeSubscription>();
-}
-
 void TestSigninClient::SetNetworkCallsDelayed(bool value) {
   network_calls_delayed_ = value;
 

components/signin/core/browser/test_signin_client.h

@@ -78,13 +78,6 @@ class TestSigninClient : public SigninClient {
     return &test_url_loader_factory_;
   }
 
-  // Registers |callback| and returns the subscription.
-  // Note that |callback| will never be called.
-  std::unique_ptr<SigninClient::CookieChangeSubscription>
-  AddCookieChangeCallback(const GURL& url,
-                          const std::string& name,
-                          net::CookieChangeCallback callback) override;
-
   void set_are_signin_cookies_allowed(bool value) {
     are_signin_cookies_allowed_ = value;
   }

ios/chrome/browser/signin/ios_chrome_signin_client.h

@@ -61,10 +61,6 @@ class IOSChromeSigninClient : public SigninClient,
       content_settings::Observer* observer) override;
   void RemoveContentSettingsObserver(
       content_settings::Observer* observer) override;
-  std::unique_ptr<CookieChangeSubscription> AddCookieChangeCallback(
-      const GURL& url,
-      const std::string& name,
-      net::CookieChangeCallback callback) override;
   void DelayNetworkCall(const base::Closure& callback) override;
 
   // SigninErrorController::Observer implementation.

ios/chrome/browser/signin/ios_chrome_signin_client.mm

@@ -7,7 +7,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/metrics/metrics_service.h"
 #include "components/signin/core/browser/cookie_settings_util.h"
-#include "components/signin/core/browser/signin_cookie_change_subscription.h"
 #include "components/signin/ios/browser/account_consistency_service.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/browser_state_info_cache.h"
@@ -136,18 +135,6 @@ void IOSChromeSigninClient::RemoveContentSettingsObserver(
   host_content_settings_map_->RemoveObserver(observer);
 }
 
-std::unique_ptr<SigninClient::CookieChangeSubscription>
-IOSChromeSigninClient::AddCookieChangeCallback(
-    const GURL& url,
-    const std::string& name,
-    net::CookieChangeCallback callback) {
-  scoped_refptr<net::URLRequestContextGetter> context_getter =
-      GetURLRequestContext();
-  DCHECK(context_getter.get());
-  return std::make_unique<SigninCookieChangeSubscription>(
-      context_getter, url, name, std::move(callback));
-}
-
 void IOSChromeSigninClient::DelayNetworkCall(const base::Closure& callback) {
   network_callback_helper_->HandleCallback(callback);
 }

ios/web_view/internal/signin/ios_web_view_signin_client.h

@@ -51,10 +51,6 @@ class IOSWebViewSigninClient : public SigninClient,
       content_settings::Observer* observer) override;
   void RemoveContentSettingsObserver(
       content_settings::Observer* observer) override;
-  std::unique_ptr<CookieChangeSubscription> AddCookieChangeCallback(
-      const GURL& url,
-      const std::string& name,
-      net::CookieChangeCallback callback) override;
   void DelayNetworkCall(const base::Closure& callback) override;
   std::unique_ptr<GaiaAuthFetcher> CreateGaiaAuthFetcher(
       GaiaAuthConsumer* consumer,

ios/web_view/internal/signin/ios_web_view_signin_client.mm

@@ -5,7 +5,6 @@
 #include "ios/web_view/internal/signin/ios_web_view_signin_client.h"
 
 #include "components/signin/core/browser/cookie_settings_util.h"
-#include "components/signin/core/browser/signin_cookie_change_subscription.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
@@ -98,18 +97,6 @@ void IOSWebViewSigninClient::RemoveContentSettingsObserver(
   host_content_settings_map_->RemoveObserver(observer);
 }
 
-std::unique_ptr<SigninClient::CookieChangeSubscription>
-IOSWebViewSigninClient::AddCookieChangeCallback(
-    const GURL& url,
-    const std::string& name,
-    net::CookieChangeCallback callback) {
-  scoped_refptr<net::URLRequestContextGetter> context_getter =
-      GetURLRequestContext();
-  DCHECK(context_getter.get());
-  return std::make_unique<SigninCookieChangeSubscription>(
-      context_getter, url, name, std::move(callback));
-}
-
 void IOSWebViewSigninClient::DelayNetworkCall(const base::Closure& callback) {
   network_callback_helper_->HandleCallback(callback);
 }