Convert UserPolicySigninService to use OAuth2TokenService

chrome/browser/policy/cloud/user_policy_signin_service.cc

Index: chrome/browser/policy/cloud/user_policy_signin_service.cc
diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.cc b/chrome/browser/policy/cloud/user_policy_signin_service.cc
index 56d712cb561fa63d7ba6c90e06b28b6f53865974..12e5fee7cc6f56e3cfb8de07854a1776c9e06cbb 100644
--- a/chrome/browser/policy/cloud/user_policy_signin_service.cc
+++ b/chrome/browser/policy/cloud/user_policy_signin_service.cc
@@ -14,10 +14,10 @@
 #include "chrome/browser/policy/cloud/user_cloud_policy_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/signin/profile_oauth2_token_service.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
-#include "chrome/browser/signin/token_service.h"
-#include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
@@ -35,28 +35,37 @@ UserPolicySigninService::UserPolicySigninService(
   if (profile->GetPrefs()->GetBoolean(prefs::kDisableCloudPolicyOnSignin))
     return;
 
+  ProfileOAuth2TokenService* oauth_token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile);

[Mattias Nissler (ping if slow), 2013/08/19 14:03:22]

Can we inject this pointer from the factory via a ctor parameter? This helps
prevent cycles in the dependency graph, because the PKSF can't grab pointers for
not-yet-created services.

[Andrew T Wilson (Slow), 2013/08/20 09:28:35]

Yep, we discussed this and I was planning to do that in a future CL, but I
suppose I can do it in this one.

+
+  // ProfileOAuth2TokenService should not yet have loaded its tokens since this
+  // happens in the background after PKS initialization - so this service
+  // should always be created before the oauth token is available.
+  DCHECK(!oauth_token_service->RefreshTokenIsAvailable());
+
   // Listen for an OAuth token to become available so we can register a client
   // if for some reason the client is not already registered (for example, if
   // the policy load failed during initial signin).
-  registrar()->Add(this,
-                   chrome::NOTIFICATION_TOKEN_AVAILABLE,
-                   content::Source<TokenService>(
-                       TokenServiceFactory::GetForProfile(profile)));
-
-  // TokenService should not yet have loaded its tokens since this happens in
-  // the background after PKS initialization - so this service should always be
-  // created before the oauth token is available.
-  DCHECK(!TokenServiceFactory::GetForProfile(profile)->HasOAuthLoginToken());
+  oauth_token_service->AddObserver(this);
 }
 
-UserPolicySigninService::~UserPolicySigninService() {}
+UserPolicySigninService::~UserPolicySigninService() {
+}
 
-void UserPolicySigninService::Shutdown() {
+void UserPolicySigninService::PrepareForUserCloudPolicyManagerShutdown() {
   // Stop any pending registration helper activity. We do this here instead of
   // in the destructor because we want to shutdown the registration helper
   // before UserCloudPolicyManager shuts down the CloudPolicyClient.
   registration_helper_.reset();
+
+  UserPolicySigninServiceBase::PrepareForUserCloudPolicyManagerShutdown();
+}
+
+void UserPolicySigninService::Shutdown() {
   UserPolicySigninServiceBase::Shutdown();
+  ProfileOAuth2TokenService* oauth_token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile());

[Mattias Nissler (ping if slow), 2013/08/19 14:03:22]

pass pointer via ctor (also below)?

[Andrew T Wilson (Slow), 2013/08/20 09:28:35]

Done.

+  oauth_token_service->RemoveObserver(this);
 }
 
 void UserPolicySigninService::RegisterPolicyClient(
@@ -98,44 +107,23 @@ void UserPolicySigninService::CallPolicyRegistrationCallback(
   callback.Run(client.Pass());
 }
 
-void UserPolicySigninService::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-
-  if (profile()->IsManaged()) {
-    registrar()->RemoveAll();
-    return;
-  }
-
-  // If using a TestingProfile with no SigninManager or UserCloudPolicyManager,
-  // skip initialization.
-  if (!GetManager() || !SigninManagerFactory::GetForProfile(profile())) {
+void UserPolicySigninService::OnRefreshTokenAvailable(
+    const std::string& account_id) {
+  // If using a TestingProfile with no UserCloudPolicyManager, skip
+  // initialization.
+  if (!GetManager()) {
     DVLOG(1) << "Skipping initialization for tests due to missing components.";
     return;
   }
 
-  switch (type) {
-    case chrome::NOTIFICATION_TOKEN_AVAILABLE: {
-      const TokenService::TokenAvailableDetails& token_details =
-          *(content::Details<const TokenService::TokenAvailableDetails>(
-              details).ptr());
-      if (token_details.service() ==
-              GaiaConstants::kGaiaOAuth2LoginRefreshToken) {
-        SigninManager* signin_manager =
-            SigninManagerFactory::GetForProfile(profile());
-        std::string username = signin_manager->GetAuthenticatedUsername();
-        // Should not have GAIA tokens if the user isn't signed in.
-        DCHECK(!username.empty());
-        // TokenService now has a refresh token (implying that the user is
-        // signed in) so initialize the UserCloudPolicyManager.
-        InitializeForSignedInUser(username);
-      }
-      break;
-    }
-    default:
-      UserPolicySigninServiceBase::Observe(type, source, details);
-  }
+  SigninManager* signin_manager =
+      SigninManagerFactory::GetForProfile(profile());

[Mattias Nissler (ping if slow), 2013/08/19 14:03:22]

ditto, can we pass the pointer via ctor (also below)?

[Andrew T Wilson (Slow), 2013/08/20 09:28:35]

Done.

[Andrew T Wilson (Slow), 2013/08/20 12:24:22]

Actually, I take it back - we can't inject this at construction time because
this pulls in a bunch of other dependencies (like TokenService, which relies on
the existence of a UI thread) and this breaks tests that just want to create a
TestingProfile without creating threads.

I've logged crbug.com/276270 to track this issue - we're getting rid of
TokenService and once that's complete we can inject SigninManager also.

+  std::string username = signin_manager->GetAuthenticatedUsername();
+  // Should not have OAuth tokens if the user isn't signed in.
+  DCHECK(!username.empty());
+  // ProfileOAuth2TokenService now has a refresh token so initialize the
+  // UserCloudPolicyManager.
+  InitializeForSignedInUser(username);
 }
 
 void UserPolicySigninService::InitializeUserCloudPolicyManager(
@@ -164,22 +152,21 @@ void UserPolicySigninService::OnInitializationCompleted(
   DVLOG_IF(1, manager->IsClientRegistered())
       << "Client already registered - not fetching DMToken";
   if (!manager->IsClientRegistered()) {
-    std::string token = TokenServiceFactory::GetForProfile(profile())->
-        GetOAuth2LoginRefreshToken();
-    if (token.empty()) {
-      // No token yet - this class listens for NOTIFICATION_TOKEN_AVAILABLE
+    ProfileOAuth2TokenService* oauth_token_service =
+        ProfileOAuth2TokenServiceFactory::GetForProfile(profile());
+    if (!oauth_token_service->RefreshTokenIsAvailable()) {
+      // No token yet - this class listens for OnRefreshTokenAvailable()
       // and will re-attempt registration once the token is available.
       DLOG(WARNING) << "No OAuth Refresh Token - delaying policy download";
       return;
     }
-    RegisterCloudPolicyService(token);
+    RegisterCloudPolicyService();
   }
   // If client is registered now, prohibit signout.
   ProhibitSignoutIfNeeded();
 }
 
-void UserPolicySigninService::RegisterCloudPolicyService(
-    const std::string& login_token) {
+void UserPolicySigninService::RegisterCloudPolicyService() {
   DCHECK(!GetManager()->IsClientRegistered());
   DVLOG(1) << "Fetching new DM Token";
   // Do nothing if already starting the registration process.
@@ -193,8 +180,11 @@ void UserPolicySigninService::RegisterCloudPolicyService(
       GetManager()->core()->client(),
       ShouldForceLoadPolicy(),
       enterprise_management::DeviceRegisterRequest::BROWSER));
-  registration_helper_->StartRegistrationWithLoginToken(
-      login_token,
+  SigninManager* signin_manager =
+      SigninManagerFactory::GetForProfile(profile());
+  registration_helper_->StartRegistration(
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile()),
+      signin_manager->GetAuthenticatedUsername(),
       base::Bind(&UserPolicySigninService::OnRegistrationComplete,
                  base::Unretained(this)));
 }