[Dice] UserPolicySigninService should start when the user signs in.

If the token service has already a refresh token for the authenticated
account before this account is signed in to Chrome, then |OnRefreshToken|
notification will not be called after |GoogleSigninSucceeded|.

This CL changes the UserPolicySigninService to handle the case when the
refresh token is already present when |GoogleSigninSucceeded| is fired.

See bug for the link to the design document.

Bug: 733226
Change-Id: I17e3ef8285b4012e125951c5a38e1851a09c9c9a
Reviewed-on: https://chromium-review.googlesource.com/559538
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486008}

chrome/browser/policy/cloud/user_policy_signin_service.cc

@@ -137,6 +137,17 @@ void UserPolicySigninService::CallPolicyRegistrationCallback(
   callback.Run(client->dm_token(), client->client_id());
 }
 
+void UserPolicySigninService::GoogleSigninSucceeded(
+    const std::string& account_id,
+    const std::string& username) {
+  if (!oauth2_token_service_->RefreshTokenIsAvailable(account_id))
+    return;
+
+  // ProfileOAuth2TokenService now has a refresh token for the primary account
+  // so initialize the UserCloudPolicyManager.
+  TryInitializeForSignedInUser();
+}
+
 void UserPolicySigninService::OnRefreshTokenAvailable(
     const std::string& account_id) {
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
@@ -145,6 +156,20 @@ void UserPolicySigninService::OnRefreshTokenAvailable(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422460 UserPolicySigninService::OnRefreshTokenAvailable"));
 
+  // Ignore OAuth tokens for any account but the primary one.
+  if (account_id != signin_manager()->GetAuthenticatedAccountId())
+    return;
+
+  // ProfileOAuth2TokenService now has a refresh token for the primary account
+  // so initialize the UserCloudPolicyManager.
+  TryInitializeForSignedInUser();
+}
+
+void UserPolicySigninService::TryInitializeForSignedInUser() {
+  DCHECK(signin_manager()->IsAuthenticated());
+  DCHECK(oauth2_token_service_->RefreshTokenIsAvailable(
+      signin_manager()->GetAuthenticatedAccountId()));
+
   // If using a TestingProfile with no UserCloudPolicyManager, skip
   // initialization.
   if (!policy_manager()) {
@@ -152,12 +177,6 @@ void UserPolicySigninService::OnRefreshTokenAvailable(
     return;
   }
 
-  // Ignore OAuth tokens for any account but the primary one.
-  if (account_id != signin_manager()->GetAuthenticatedAccountId())
-    return;
-
-  // ProfileOAuth2TokenService now has a refresh token so initialize the
-  // UserCloudPolicyManager.
   InitializeForSignedInUser(
       signin_manager()->GetAuthenticatedAccountInfo().email,
       profile_->GetRequestContext());

chrome/browser/policy/cloud/user_policy_signin_service.h

@@ -60,6 +60,11 @@ class UserPolicySigninService : public UserPolicySigninServiceBase,
       const std::string& account_id,
       const PolicyRegistrationCallback& callback);
 
+  // SigninManagerBase::Observer implementation:
+  // UserPolicySigninService is already an observer of SigninManagerBase.
+  void GoogleSigninSucceeded(const std::string& account_id,
+                             const std::string& username) override;
+
   // OAuth2TokenService::Observer implementation:
   void OnRefreshTokenAvailable(const std::string& account_id) override;
 
@@ -92,6 +97,11 @@ class UserPolicySigninService : public UserPolicySigninServiceBase,
   // cloud policy.
   void ProhibitSignoutIfNeeded();
 
+  // Helper method that attempts calls |InitializeForSignedInUser| only if
+  // |policy_manager| is not-nul. Expects that there is a refresh token for
+  // the primary account.
+  void TryInitializeForSignedInUser();
+
   // Invoked when a policy registration request is complete.
   void CallPolicyRegistrationCallback(std::unique_ptr<CloudPolicyClient> client,
                                       PolicyRegistrationCallback callback);

chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc

@@ -400,9 +400,39 @@ TEST_F(UserPolicySigninServiceTest, InitWhileSignedOut) {
   ASSERT_FALSE(manager_->core()->service());
 }
 
-  // TODO(joaodasilva): these tests rely on issuing the OAuth2 login refresh
-  // token after signin. Revisit this after figuring how to handle that on
-  // Android.
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+TEST_F(UserPolicySigninServiceTest, InitRefreshTokenAvailableBeforeSignin) {
+  // Make sure user is not signed in.
+  ASSERT_FALSE(
+      SigninManagerFactory::GetForProfile(profile_.get())->IsAuthenticated());
+
+  // No oauth access token yet, so client registration should be deferred.
+  ASSERT_FALSE(IsRequestActive());
+
+  // Make oauth token available.
+  std::string account_id = AccountTrackerService::PickAccountIdForAccount(
+      profile_.get()->GetPrefs(), kTestGaiaId, kTestUser);
+  GetTokenService()->UpdateCredentials(account_id, "oauth_login_refresh_token");
+
+  // Not ssigned in yet, so client registration should be deferred.
+  ASSERT_FALSE(IsRequestActive());
+
+  // Sign in to Chrome.
+  signin_manager_->SignIn(kTestGaiaId, kTestUser, "");
+
+  // Complete initialization of the store.
+  mock_store_->NotifyStoreLoaded();
+
+  // Client registration should be in progress since we now have an oauth token
+  // for the authenticated account id.
+  EXPECT_EQ(mock_store_->signin_username(), kTestUser);
+  ASSERT_TRUE(IsRequestActive());
+}
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+
+// TODO(joaodasilva): these tests rely on issuing the OAuth2 login refresh
+// token after signin. Revisit this after figuring how to handle that on
+// Android.
 #if !defined(OS_ANDROID)
 
 TEST_F(UserPolicySigninServiceSignedInTest, InitWhileSignedIn) {