Make the change in ProfileSyncService such that it declares success

for new users only when the oauth login token is successfully generated.
Make sure for existing logged in users, it continues to setup sync
properly on startup.
Review URL: http://codereview.chromium.org/8760019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112714 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/profile_sync_service.cc

@@ -84,6 +84,20 @@ const char* ProfileSyncService::kDevServerUrl =
 
 static const int kSyncClearDataTimeoutInSeconds = 60;  // 1 minute.
 
+static const char* kRelevantTokenServices[] = {
+    GaiaConstants::kSyncService,
+    GaiaConstants::kGaiaOAuth2LoginRefreshToken};
+static const int kRelevantTokenServicesCount =
+    arraysize(kRelevantTokenServices);
+
+// Helper to check if the given token service is relevant for sync.
+static bool IsTokenServiceRelevant(const std::string& service) {
+  for (int i = 0; i < kRelevantTokenServicesCount; ++i) {
+    if (service == kRelevantTokenServices[i])
+      return true;
+  }
+  return false;
+}
 
 bool ShouldShowActionOnUI(
     const browser_sync::SyncProtocolError& error) {
@@ -141,18 +155,27 @@ ProfileSyncService::~ProfileSyncService() {
 }
 
 bool ProfileSyncService::AreCredentialsAvailable() {
+  return AreCredentialsAvailable(false);
+}
+
+bool ProfileSyncService::AreCredentialsAvailable(
+    bool check_oauth_login_token) {
   if (IsManaged()) {
     return false;
   }
 
   // CrOS user is always logged in. Chrome uses signin_ to check logged in.
-  if (!cros_user_.empty() || !signin_->GetUsername().empty()) {
-    // TODO(chron): Verify CrOS unit test behavior.
-    return profile()->GetTokenService() &&
-        profile()->GetTokenService()->HasTokenForService(
-            browser_sync::SyncServiceName());
-  }
-  return false;
+  if (cros_user_.empty() && signin_->GetUsername().empty())
+    return false;
+
+  TokenService* token_service = profile()->GetTokenService();
+  if (!token_service)
+    return false;
+
+  // TODO(chron): Verify CrOS unit test behavior.
+  if (!token_service->HasTokenForService(browser_sync::SyncServiceName()))
+    return false;
+  return !check_oauth_login_token || token_service->HasOAuthLoginToken();
 }
 
 void ProfileSyncService::Initialize() {
@@ -1435,13 +1458,22 @@ void ProfileSyncService::Observe(int type,
       break;
     }
     case chrome::NOTIFICATION_TOKEN_REQUEST_FAILED: {
-      GoogleServiceAuthError error(
-          GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
-      UpdateAuthErrorState(error);
+      const TokenService::TokenRequestFailedDetails& token_details =
+          *(content::Details<const TokenService::TokenRequestFailedDetails>(
+              details).ptr());
+      if (IsTokenServiceRelevant(token_details.service())) {
+        GoogleServiceAuthError error(
+            GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
+        UpdateAuthErrorState(error);
+      }
       break;
     }
     case chrome::NOTIFICATION_TOKEN_AVAILABLE: {
-      if (AreCredentialsAvailable()) {
+      const TokenService::TokenAvailableDetails& token_details =
+          *(content::Details<const TokenService::TokenAvailableDetails>(
+              details).ptr());
+      if (IsTokenServiceRelevant(token_details.service()) &&
+          AreCredentialsAvailable(true)) {
         if (backend_initialized_) {
           backend_->UpdateCredentials(GetCredentials());
         }
@@ -1451,11 +1483,22 @@ void ProfileSyncService::Observe(int type,
       break;
     }
     case chrome::NOTIFICATION_TOKEN_LOADING_FINISHED: {
-      // If not in Chrome OS, and we have a username without tokens,
-      // the user will need to signin again, so sign out.
-      if (cros_user_.empty() &&
-          !signin_->GetUsername().empty() &&
-          !AreCredentialsAvailable()) {
+      // This notification gets fired when TokenService loads the tokens
+      // from storage. Here we only check if the chromiumsync token is
+      // available (versus both chromiumsync and oauth login tokens) to
+      // start up sync successfully for already logged in users who may
+      // only have chromiumsync token if they logged in before the code
+      // to generate oauth login token released.
+      if (AreCredentialsAvailable()) {
+        // Initialize the backend if sync token was loaded.
+        if (backend_initialized_) {
+          backend_->UpdateCredentials(GetCredentials());
+        }
+        if (!sync_prefs_.IsStartSuppressed())
+          StartUp();
+      } else if (cros_user_.empty() && !signin_->GetUsername().empty()) {
+        // If not in Chrome OS, and we have a username without tokens,
+        // the user will need to signin again, so sign out.
         DisableForUser();
       }
       break;

chrome/browser/sync/profile_sync_service.h

@@ -160,10 +160,14 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
 
   void RegisterAuthNotifications();
 
-  // Return whether all sync tokens are loaded and
-  // available for the backend to start up.
+  // Same as AreCredentialsAvailable(false).
   bool AreCredentialsAvailable();
 
+  // Return whether all sync tokens are loaded and available for the backend to
+  // start up. Also checks for OAuth login token if |check_oauth_login_token| is
+  // true.
+  bool AreCredentialsAvailable(bool check_oauth_login_token);
+
   // Registers a data type controller with the sync service.  This
   // makes the data type controller available for use, it does not
   // enable or activate the synchronization of the data type (see

chrome/browser/sync/profile_sync_service_startup_unittest.cc

@@ -124,6 +124,8 @@ TEST_F(ProfileSyncServiceStartupTest, StartFirstTime) {
   service_->OnUserSubmittedAuth("test_user", "", "", "");
   profile_->GetTokenService()->IssueAuthTokenForTest(
       GaiaConstants::kSyncService, "sync_token");
+  profile_->GetTokenService()->IssueAuthTokenForTest(
+      GaiaConstants::kGaiaOAuth2LoginRefreshToken, "oauth2_login_token");
 
   syncable::ModelTypeSet set;
   set.insert(syncable::BOOKMARKS);

chrome/browser/sync/profile_sync_service_unittest.cc

@@ -106,13 +106,19 @@ class ProfileSyncServiceTest : public testing::Test {
       }
 
       if (issue_auth_token) {
-        profile_->GetTokenService()->IssueAuthTokenForTest(
-            GaiaConstants::kSyncService, "token");
+        IssueTestTokens();
       }
       service_->Initialize();
     }
   }
 
+  void IssueTestTokens() {
+    profile_->GetTokenService()->IssueAuthTokenForTest(
+        GaiaConstants::kSyncService, "token1");
+    profile_->GetTokenService()->IssueAuthTokenForTest(
+        GaiaConstants::kGaiaOAuth2LoginRefreshToken, "token2");
+  }
+
   MessageLoop ui_loop_;
   // Needed by |service_|.
   content::TestBrowserThread ui_thread_;
@@ -165,8 +171,7 @@ TEST_F(ProfileSyncServiceTest, DisableAndEnableSyncTemporarily) {
   EXPECT_CALL(factory_, CreateDataTypeManager(_, _)).
       WillRepeatedly(ReturnNewDataTypeManager());
 
-  profile_->GetTokenService()->IssueAuthTokenForTest(
-      GaiaConstants::kSyncService, "token");
+  IssueTestTokens();
 
   service_->Initialize();
   EXPECT_TRUE(service_->sync_initialized());
@@ -207,8 +212,7 @@ TEST_F(ProfileSyncServiceTest,
   js_controller->AddJsEventHandler(&event_handler);
   // Since we're doing synchronous initialization, backend should be
   // initialized by this call.
-  profile_->GetTokenService()->IssueAuthTokenForTest(
-      GaiaConstants::kSyncService, "token");
+  IssueTestTokens();
   EXPECT_TRUE(service_->sync_initialized());
   js_controller->RemoveJsEventHandler(&event_handler);
 }
@@ -253,8 +257,7 @@ TEST_F(ProfileSyncServiceTest,
                                     args1, reply_handler.AsWeakHandle());
   }
 
-  profile_->GetTokenService()->IssueAuthTokenForTest(
-      GaiaConstants::kSyncService, "token");
+  IssueTestTokens();
 
   // This forces the sync thread to process the message and reply.
   service_.reset();
@@ -286,8 +289,7 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) {
 
   // Since we're doing synchronous initialization, backend should be
   // initialized by this call.
-  profile_->GetTokenService()->IssueAuthTokenForTest(
-      GaiaConstants::kSyncService, "token");
+  IssueTestTokens();
 
   // Stop the service so we can read the new Sync Data files that were
   // created.

chrome/browser/sync/test/integration/sync_test.cc

@@ -381,6 +381,10 @@ void SyncTest::SetupMockGaiaResponses() {
   fake_factory_->SetFakeResponse(kSearchDomainCheckUrl, ".google.com", true);
   fake_factory_->SetFakeResponse(
       GaiaUrls::GetInstance()->client_login_to_oauth2_url(),
+      "some_response",
+      true);
+  fake_factory_->SetFakeResponse(
+      GaiaUrls::GetInstance()->oauth2_token_url(),
       kOAuth2LoginTokenValidResponse,
       true);
 }