Abort sync cycles when download step fails

sync/engine/syncer.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 "sync/engine/syncer.h"
 
 #include "base/debug/trace_event.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ skipping 148 matching lines @@
       }
       case PROCESS_UPDATES: {
         ProcessUpdatesCommand process_updates;
         process_updates.Execute(session);
         next_step = STORE_TIMESTAMPS;
         break;
       }
       case STORE_TIMESTAMPS: {
         StoreTimestampsCommand store_timestamps;
         store_timestamps.Execute(session);
-        // We should download all of the updates before attempting to process
-        // them.
-        if (session->status_controller().ServerSaysNothingMoreToDownload() ||
-            !session->status_controller().download_updates_succeeded()) {
+        // We download all of the updates before attempting to apply them.
+        if (!session->status_controller().download_updates_succeeded()) {
+          // We may have downloaded some updates, but if the latest download
+          // attempt failed then we don't have all the updates.  We'll leave
+          // it to a retry job to pick up where we left off.
+          last_step = SYNCER_END; // Necessary for CONFIGURATION mode.

[tim (not reviewing), 2012/04/06 20:38:57]

If we're going straight to END, why not call RequestEarlyExit()? That function
is meant to provide support for bailing out of a sync session at any time.

[tim (not reviewing), 2012/04/06 20:48:21]

Oh, I guess you *want* to run END. 

I think the comment about "CONFIGURATION" mode confused me a bit. Is the more
general comment "Necessary because some components may rely on knowing when a
sync cycle that started has ended"?

I know we've went back and forth with just sending the notification
unconditionally after the cycle loop, I forget why at the moment...

+          next_step = SYNCER_END;
+          DVLOG(1) << "Aborting sync cycle due to download updates failure";
+        } else if (!session->status_controller()
+                   .ServerSaysNothingMoreToDownload()) {

[Nicolas Zea, 2012/04/09 19:09:14]

nit: indent by four more spaces

+          next_step = DOWNLOAD_UPDATES;
+        } else {
           next_step = APPLY_UPDATES;
-        } else {
-          next_step = DOWNLOAD_UPDATES;
         }
         break;
       }
       case APPLY_UPDATES: {
         ApplyUpdatesCommand apply_updates;
         apply_updates.Execute(session);
         if (last_step == APPLY_UPDATES) {
           // We're in configuration mode, but we still need to run the
           // SYNCER_END step.
           last_step = SYNCER_END;
@@ skipping 150 matching lines @@
   entry->Put(SERVER_CTIME, Time());
   entry->Put(SERVER_VERSION, 0);
   entry->Put(SERVER_IS_DIR, false);
   entry->Put(SERVER_IS_DEL, false);
   entry->Put(IS_UNAPPLIED_UPDATE, false);
   entry->Put(SERVER_SPECIFICS, sync_pb::EntitySpecifics::default_instance());
   entry->Put(SERVER_POSITION_IN_PARENT, 0);
 }
 
 }  // namespace browser_sync