[Sync] Handle errors during first sync gracefully.

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 14 matching lines @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/sync/api/sync_error.h"
 #include "chrome/browser/sync/backend_migrator.h"
 #include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/data_type_controller.h"
-#include "chrome/browser/sync/glue/data_type_manager.h"
 #include "chrome/browser/sync/glue/session_data_type_controller.h"
 #include "chrome/browser/sync/glue/session_model_associator.h"
 #include "chrome/browser/sync/glue/typed_url_data_type_controller.h"
 #include "chrome/browser/sync/internal_api/configure_reason.h"
 #include "chrome/browser/sync/internal_api/sync_manager.h"
 #include "chrome/browser/sync/js/js_arg_list.h"
 #include "chrome/browser/sync/js/js_event_details.h"
 #include "chrome/browser/sync/profile_sync_components_factory_impl.h"
 #include "chrome/browser/sync/sync_global_error.h"
 #include "chrome/browser/sync/util/cryptographer.h"
@@ skipping 74 matching lines @@
       wizard_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       signin_(signin_manager),
       unrecoverable_error_detected_(false),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       expect_sync_configuration_aborted_(false),
       clear_server_data_state_(CLEAR_NOT_STARTED),
       encrypted_types_(browser_sync::Cryptographer::SensitiveTypes()),
       encrypt_everything_(false),
       encryption_pending_(false),
       auto_start_enabled_(start_behavior == AUTO_START),
-      failed_datatypes_handler_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+      failed_datatypes_handler_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      configure_status_(DataTypeManager::UNKNOWN) {
   // By default, dev, canary, and unbranded Chromium users will go to the
   // development servers. Development servers have more features than standard
   // sync servers. Users with officially-branded Chrome stable and beta builds
   // will go to the standard sync servers.
   //
   // GetChannel hits the registry on Windows. See http://crbug.com/70380.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
   if (channel == chrome::VersionInfo::CHANNEL_STABLE ||
       channel == chrome::VersionInfo::CHANNEL_BETA) {
@@ skipping 206 matching lines @@
   return !Intersection(preferred_types, encrypted_types).Empty();
 }
 
 void ProfileSyncService::OnSyncConfigureDone(
     DataTypeManager::ConfigureResult result) {
   if (failed_datatypes_handler_.UpdateFailedDatatypes(result)) {
     ReconfigureDatatypeManager();
   }
 }
 
+void ProfileSyncService::OnSyncConfigureRetry() {
+  // In platforms with auto start we would just wait for the
+  // configure to finish. In other platforms we would throw
+  // an unrecoverable error.

[Andrew T Wilson (Slow), 2012/01/27 18:41:11]

Add a note that the reason we do this is to force the login process to fail and
display a login error to the user.

[lipalani1, 2012/01/27 19:35:26]

Done.

+  // Also if backend has been initialized(the user is authenticated
+  // and nigori is downloaded) we would simply wait rather than going into
+  // unrecoverable error, even if the platform has auto start disabled.
+  // Note: In those scenarios the UI does not wait for the configuration
+  // to finish

[Andrew T Wilson (Slow), 2012/01/27 18:41:11]

nit: period at end of sentence.

[lipalani1, 2012/01/27 19:35:26]

Done.

+  if (!auto_start_enabled_ && !backend_initialized_) {
+    OnUnrecoverableError(FROM_HERE,
+                         "Configure failed to download.");
+  }
+
+  NotifyObservers();
+}
+
+
 void ProfileSyncService::StartUp() {
   // Don't start up multiple times.
   if (backend_.get()) {
     DVLOG(1) << "Skipping bringing up backend host.";
     return;
   }
 
   DCHECK(AreCredentialsAvailable());
 
   last_synced_time_ = sync_prefs_.GetLastSyncedTime();
@@ skipping 959 matching lines @@
   switch (type) {
     case chrome::NOTIFICATION_SYNC_CONFIGURE_START: {
       NotifyObservers();
       // TODO(sync): Maybe toast?
       break;
     }
     case chrome::NOTIFICATION_SYNC_CONFIGURE_DONE: {
       DataTypeManager::ConfigureResult* result =
           content::Details<DataTypeManager::ConfigureResult>(details).ptr();
 
-      DataTypeManager::ConfigureStatus status = result->status;
-      DVLOG(1) << "PSS SYNC_CONFIGURE_DONE called with status: " << status;
-      if (status == DataTypeManager::ABORTED &&
+      configure_status_ = result->status;
+      DVLOG(1) << "PSS SYNC_CONFIGURE_DONE called with status: "
+               << configure_status_;
+
+      // The possible status values:
+      //    ABORT - Configuration was aborted. This is not an error, if
+      //            initiated by user..

[Andrew T Wilson (Slow), 2012/01/27 18:41:11]

nit: double '..'

[lipalani1, 2012/01/27 19:35:26]

Done.

+      //    RETRY - Configure failed but we are retrying.
+      //    OK - Everything succeeded.
+      //    PARTIAL_SUCCESS - Some datatypes failed to start.
+      //    Everything else is an UnrecoverableError. So treat it as such.
+
+      // First handle the abort case.
+      if (configure_status_ == DataTypeManager::ABORTED &&
           expect_sync_configuration_aborted_) {
         DVLOG(0) << "ProfileSyncService::Observe Sync Configure aborted";
         expect_sync_configuration_aborted_ = false;
         return;
       }
-      if (status != DataTypeManager::OK &&
-          status != DataTypeManager::PARTIAL_SUCCESS) {
+
+      // Handle retry case.
+      if (configure_status_ == DataTypeManager::RETRY) {
+        OnSyncConfigureRetry();
+        return;
+      }
+
+      // Handle unrecoverable error.
+      if (configure_status_ != DataTypeManager::OK &&
+          configure_status_ != DataTypeManager::PARTIAL_SUCCESS) {
         // Something catastrophic had happened. We should only have one
         // error representing it.
         DCHECK(result->errors.size() == 1);
         SyncError error = result->errors.front();
         DCHECK(error.IsSet());
         std::string message =
           "Sync configuration failed with status " +
-          DataTypeManager::ConfigureStatusToString(status) +
+          DataTypeManager::ConfigureStatusToString(configure_status_) +
           " during " + syncable::ModelTypeToString(error.type()) +
           ": " + error.message();
         LOG(ERROR) << "ProfileSyncService error: "
                    << message;
         // TODO: Don't
         OnUnrecoverableError(error.location(), message);
         return;
       }
 
+      // Now handle partial success and full success.
       MessageLoop::current()->PostTask(FROM_HERE,
           base::Bind(&ProfileSyncService::OnSyncConfigureDone,
                      weak_factory_.GetWeakPtr(), *result));
 
       // We should never get in a state where we have no encrypted datatypes
       // enabled, and yet we still think we require a passphrase for decryption.
       DCHECK(!(IsPassphraseRequiredForDecryption() &&
                !IsEncryptedDatatypeEnabled()));
 
       // This must be done before we start syncing with the server to avoid
@@ skipping 187 matching lines @@
              << "Unrecoverable error.";
   } else {
     DVLOG(0) << "ConfigureDataTypeManager not invoked because backend is not "
              << "initialized";
   }
 }
 
 const FailedDatatypesHandler& ProfileSyncService::failed_datatypes_handler() {
   return failed_datatypes_handler_;
 }