Revert 132455 - 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 92 matching lines @@
   SyncerStep current_step = first_step;
 
   SyncerStep next_step = current_step;
   while (!ExitRequested()) {
     TRACE_EVENT1("sync", "SyncerStateMachine",
                  "state", SyncerStepToString(current_step));
     DVLOG(1) << "Syncer step:" << SyncerStepToString(current_step);
 
     switch (current_step) {
       case SYNCER_BEGIN:
+        // This isn't perfect, as we can end up bundling extensions activity
+        // intended for the next session into the current one.  We could do a
+        // test-and-reset as with the source, but note that also falls short if
+        // the commit request fails (e.g. due to lost connection), as we will
+        // fall all the way back to the syncer thread main loop in that case,
+        // creating a new session when a connection is established, losing the
+        // records set here on the original attempt.  This should provide us
+        // with the right data "most of the time", and we're only using this
+        // for analysis purposes, so Law of Large Numbers FTW.
+        session->context()->extensions_monitor()->GetAndClearRecords(
+            session->mutable_extensions_activity());
         session->context()->PruneUnthrottledTypes(base::TimeTicks::Now());
         session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_BEGIN);
 
         next_step = CLEANUP_DISABLED_TYPES;
         break;
       case CLEANUP_DISABLED_TYPES: {
         CleanupDisabledTypesCommand cleanup;
         cleanup.Execute(session);
         next_step = DOWNLOAD_UPDATES;
         break;
@@ skipping 25 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 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.
-          next_step = SYNCER_END;
-          DVLOG(1) << "Aborting sync cycle due to download updates failure";
-        } else if (!session->status_controller()
-                       .ServerSaysNothingMoreToDownload()) {
+        // We should download all of the updates before attempting to process
+        // them.
+        if (session->status_controller().ServerSaysNothingMoreToDownload() ||
+            !session->status_controller().download_updates_succeeded()) {
+          next_step = APPLY_UPDATES;
+        } else {
           next_step = DOWNLOAD_UPDATES;
-        } else {
-          next_step = APPLY_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 10 matching lines @@
         WriteTransaction trans(FROM_HERE, SYNCER, dir);
         sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans);
 
         DVLOG(1) << "Getting the Commit IDs";
         GetCommitIdsCommand get_commit_ids_command(
             session->context()->max_commit_batch_size());
         get_commit_ids_command.Execute(session);
 
         if (!session->status_controller().commit_ids().empty()) {
           DVLOG(1) << "Building a commit message";
-
-          // This isn't perfect, since the set of extensions activity may not
-          // correlate exactly with the items being committed.  That's OK as
-          // long as we're looking for a rough estimate of extensions activity,
-          // not an precise mapping of which commits were triggered by which
-          // extension.
-          //
-          // We will push this list of extensions activity back into the
-          // ExtensionsActivityMonitor if this commit turns out to not contain
-          // any bookmarks, or if the commit fails.
-          session->context()->extensions_monitor()->GetAndClearRecords(
-              session->mutable_extensions_activity());
-
           BuildCommitCommand build_commit_command;
           build_commit_command.Execute(session);
 
           next_step = POST_COMMIT_MESSAGE;
         } else {
           next_step = RESOLVE_CONFLICTS;
         }
 
         break;
       }
@@ skipping 120 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