Make the SigninErrorController a PKS.

components/signin/core/browser/profile_oauth2_token_service.h

Index: components/signin/core/browser/profile_oauth2_token_service.h
diff --git a/components/signin/core/browser/profile_oauth2_token_service.h b/components/signin/core/browser/profile_oauth2_token_service.h
index 13210c1fcfe5ba4f087972a943d54bd36ab79aba..c5a3c201ed72c036f8f19013df68ef2bd97feb70 100644
--- a/components/signin/core/browser/profile_oauth2_token_service.h
+++ b/components/signin/core/browser/profile_oauth2_token_service.h
@@ -10,7 +10,6 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/linked_ptr.h"
 #include "components/keyed_service/core/keyed_service.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 
 namespace net {
@@ -19,6 +18,7 @@ class URLRequestContextGetter;
 
 class GoogleServiceAuthError;
 class SigninClient;
+class SigninErrorController;
 
 // ProfileOAuth2TokenService is a KeyedService that retrieves
 // OAuth2 access tokens for a given set of scopes using the OAuth2 login
@@ -40,7 +40,8 @@ class ProfileOAuth2TokenService : public OAuth2TokenService,
   ~ProfileOAuth2TokenService() override;
 
   // Initializes this token service with the SigninClient.
-  virtual void Initialize(SigninClient* client);
+  virtual void Initialize(SigninClient* client,
+                          SigninErrorController* signin_error_controller);

[Roger Tawa OOO till Jul 10th, 2015/01/08 22:30:54]

Instead of adding a signin_error_controller argument here, I think it would be
better to add it to ChromeSigninClient.  The base class SigninClient can expose
a signin_error_controller() method, and the ChromeSigninClient factory can set
the SigninErrorController.  I think this would simplify PO2TS and its derived
classes and minimize the coupling.

[Mike Lerman, 2015/01/09 14:27:07]

I disagree, for a few reasons. 

The SigninClient is supposed to expose things outside the component (from
chrome/), but the SigninErrorController's in the component. 

In the CL to introduce the Auth Error Histogram, I'll be adding a dependency
from the SigninErrorController onto the SigninClient, so that the
SigninErrorController can set things on the ProfileInfoCache. If I here add an
unnecessary dependency from the SigninClient to the SigninErrorController, I'll
now have circular dependencies :S

I don't like hiding the dependency from PO2TS->SigninErrorController. There's a
real dependency (all the subclasses rely on it), which I think is best made
obvious in the factory, which is how the PKS dependencies should work. All we'd
be doing is passing the link from PO2TS to SigninClient (which doesn't currently
depend on SigninErrorController), which then makes everything that depends on
SigninClient now also dependent on SigninErrorController needlessly.

 
   // KeyedService implementation.
   void Shutdown() override;
@@ -67,14 +68,6 @@ class ProfileOAuth2TokenService : public OAuth2TokenService,
   // Revokes all credentials handled by the object.
   virtual void RevokeAllCredentials();
 
-  SigninErrorController* signin_error_controller() {
-    return signin_error_controller_.get();
-  }
-
-  const SigninErrorController* signin_error_controller() const {
-    return signin_error_controller_.get();
-  }
-
   SigninClient* client() const { return client_; }
 
  protected:
@@ -96,12 +89,16 @@ class ProfileOAuth2TokenService : public OAuth2TokenService,
   // when invalid.
   void ValidateAccountId(const std::string& account_id) const;
 
+  SigninErrorController* signin_error_controller() {
+    return signin_error_controller_;
+  }
+
  private:
   // The client with which this instance was initialized, or NULL.
   SigninClient* client_;
 
-  // Used to expose auth errors to the UI.
-  scoped_ptr<SigninErrorController> signin_error_controller_;
+  // The error controller with which this instance was initialized, or NULL.
+  SigninErrorController* signin_error_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileOAuth2TokenService);
 };