Abort sync cycles when download step fails

This change causes the syncer to exit from its download then commit
function if the download fails.  This helps prevent the creation of
server-side conflicts, which would be more likely to happen if a
download failed but the following commit succeeded.

The main changes are as follows:
- Rather than proceed to the next step when a download updates failure
  is detected, the syncer will exit.  This should cause the
SyncScheduler to schedule a retry at a later time.  
- The definition of a download updates failure is now based on the
  return code of the download attempt, rather than checking the contents
of the (possible non-existent) returned protobuf.  This makes the error
detection logic used here more closely match the logic used to decide
whether or not to schedule retries.

This implementation has a side-effect on configure sync cycles.  The old
behaviour was to handle failures by applying whatever updates we had
downloaded at that point.  The new behaviour will leave updates
unapplied if any error occurs.  This better matches a nearby comment's
assertion which states that we should not attempt to apply updates until
we have downloaded all of them.  I believe the author of that comment
would approve of this change.

This change moves around some of the ExtensionActivityMonitor logic.
It's important that we not take the data from the extensions acitivity
monitor at the start of the cycle.  Restoring that data is handled in
the commit building and response code, which might not be executed if we
need to break out early.  This also fixes issue 123270.

Most of the diffs for this change are concentrated in the unit tests:
- Expose more of the SyncScheduler to the SyncerTest test harness.
- Add functions to SyncerTest for testing specific types of sync jobs.
- Add some functions that allow us to better control failures in
MockConnectionManager.
- Added tests for configure job success and failure.
- Added tests for update then commit job success and failure.
- Removed some redundant tests.

BUG=122033,123270
TEST=sync_unit_tests, specifically:
SyncerTest.UpdateThenCommit,
SyncerTest.UpdateFailsThenDontCommit,
SyncerTest.ConfigureDownloadsTwoBatchesSuccess,
SyncerTest.ConfigureFailsDontApplyUpdates

Review URL: http://codereview.chromium.org/10006046

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

2 files changed, 14 insertions, 6 deletions

diff --git a/sync/test/engine/mock_connection_manager.cc b/sync/test/engine/mock_connection_manager.cc
index ab95bce..127e858 100644
--- a/sync/test/engine/mock_connection_manager.cc
+++ b/sync/test/engine/mock_connection_manager.cc
@@ -48,7 +48,7 @@ MockConnectionManager::MockConnectionManager(syncable::Directory* directory)
       store_birthday_sent_(false),
       client_stuck_(false),
       commit_time_rename_prepended_string_(""),
-      fail_next_postbuffer_(false),
+      countdown_to_postbuffer_fail_(0),
       directory_(directory),
       mid_commit_observer_(NULL),
       throttling_(false),
@@ -111,8 +111,9 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
     InvalidateAndClearAuthToken();
   }
 
-  if (fail_next_postbuffer_) {
-    fail_next_postbuffer_ = false;
+  if (--countdown_to_postbuffer_fail_ == 0) {
+    // Fail as countdown hits zero.
+    params->response.server_status = HttpResponse::SYNC_SERVER_ERROR;
     return false;
   }
 
diff --git a/sync/test/engine/mock_connection_manager.h b/sync/test/engine/mock_connection_manager.h
index a2b5060..5cc1b87 100644
--- a/sync/test/engine/mock_connection_manager.h
+++ b/sync/test/engine/mock_connection_manager.h
@@ -117,7 +117,8 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   // issue multiple requests during a sync cycle.
   void NextUpdateBatch();
 
-  void FailNextPostBufferToPathCall() { fail_next_postbuffer_ = true; }
+  void FailNextPostBufferToPathCall() { countdown_to_postbuffer_fail_ = 1; }
+  void FailNthPostBufferToPathCall(int n) { countdown_to_postbuffer_fail_ = n; }
 
   void SetClearUserDataResponseStatus(sync_pb::SyncEnums::ErrorType errortype);
 
@@ -222,6 +223,11 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
     return store_birthday_;
   }
 
+  // Explicitly indicate that we will not be fetching some updates.
+  void ClearUpdatesQueue() {
+    update_queue_.clear();
+  }
+
   // Locate the most recent update message for purpose of alteration.
   sync_pb::SyncEntity* GetMutableLastUpdate();
 
@@ -299,8 +305,9 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   bool client_stuck_;
   std::string commit_time_rename_prepended_string_;
 
-  // Fail on the next call to PostBufferToPath().
-  bool fail_next_postbuffer_;
+  // On each PostBufferToPath() call, we decrement this counter.  The call fails
+  // iff we hit zero at that call.
+  int countdown_to_postbuffer_fail_;
 
   // Our directory.  Used only to ensure that we are not holding the transaction
   // lock when performing network I/O.  Can be NULL if the test author is