[Sync] Handle errors during first sync gracefully.

chrome/browser/sync/profile_sync_service.cc

Index: chrome/browser/sync/profile_sync_service.cc
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 21673d880f045cdd5da3494950f4182b6c8daf41..bd395407d90d47867b794fc168b5dfb4b0b437ec 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -354,6 +354,16 @@ void ProfileSyncService::OnSyncConfigureDone(
   }
 }
 
+void ProfileSyncService::OnSyncConfigureRetry() {
+  if (wizard_.IsVisible()) {

[Andrew T Wilson (Slow), 2012/01/27 00:19:03]

Instead of forwarding this to the SyncSetupWizard (see my comment in
sync_setup_flow.cc) I think we should do this:

// If we're not doing autostart, then abort sync setup if our configuration
fails.
if (!auto_start_enabled_)
  OnUnrecoverableError(....);

[lipalani1, 2012/01/27 02:54:29]

Done.

+    wizard_.Step(SyncSetupWizard::CONFIGURE_RETRY);

[Andrew T Wilson (Slow), 2012/01/27 00:19:03]

Soon, SyncSetupWizard will be going away. Instead, the UI will register itself
as an Observer, and will need to query for things like "retry status" via APIs,
rather than relying on the ProfileSyncService directly modifying state in
SyncSetupWizard. This will also allow things like Clank to provide feedback to
the user without actually having to use SyncSetupWizard.

So, can we track and expose this retry state through a member variable and also
notify observers when the status of that variable changes?

[lipalani1, 2012/01/27 02:54:29]

Done.

+  }
+
+  // If the wizard is not visible nothing more to do. We just wait until
+  // configure succeeds.
+}
+
+
 void ProfileSyncService::StartUp() {
   // Don't start up multiple times.
   if (backend_.get()) {
@@ -1335,12 +1345,30 @@ void ProfileSyncService::Observe(int type,
 
       DataTypeManager::ConfigureStatus status = result->status;
       DVLOG(1) << "PSS SYNC_CONFIGURE_DONE called with status: " << status;
+
+      // The possible status values:
+      //    ABORT - Configuration was aborted. This is not an error, if
+      //            initiated by user..
+      //    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 (status == DataTypeManager::ABORTED &&
           expect_sync_configuration_aborted_) {
         DVLOG(0) << "ProfileSyncService::Observe Sync Configure aborted";
         expect_sync_configuration_aborted_ = false;
         return;
       }
+
+      // Handle retry case.
+      if (status == DataTypeManager::RETRY) {
+        OnSyncConfigureRetry();
+        return;
+      }
+
+      // Hanlde unrecoverable error.

[Andrew T Wilson (Slow), 2012/01/27 00:19:03]

nit: Hanlde->Handle

[lipalani1, 2012/01/27 02:54:29]

Done.

       if (status != DataTypeManager::OK &&
           status != DataTypeManager::PARTIAL_SUCCESS) {
         // Something catastrophic had happened. We should only have one
@@ -1360,6 +1388,7 @@ void ProfileSyncService::Observe(int type,
         return;
       }
 
+      // Now handle partial success and full success.
       MessageLoop::current()->PostTask(FROM_HERE,
           base::Bind(&ProfileSyncService::OnSyncConfigureDone,
                      weak_factory_.GetWeakPtr(), *result));