Revert 132455 - 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

TBR=rlarocque@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10107004

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

8 files changed, 276 insertions, 382 deletions

diff --git a/sync/engine/sync_scheduler.cc b/sync/engine/sync_scheduler.cc
index 4d537ae..87304ea 100644
--- a/sync/engine/sync_scheduler.cc
+++ b/sync/engine/sync_scheduler.cc
@@ -683,37 +683,6 @@ const char* SyncScheduler::GetDecisionString(
   return "";
 }
 
-// static
-void SyncScheduler::SetSyncerStepsForPurpose(
-    SyncSessionJob::SyncSessionJobPurpose purpose,
-    SyncerStep* start,
-    SyncerStep* end) {
-  switch (purpose) {
-    case SyncSessionJob::CONFIGURATION:
-      *start = DOWNLOAD_UPDATES;
-      *end = APPLY_UPDATES;
-      return;
-    case SyncSessionJob::CLEAR_USER_DATA:
-      *start = CLEAR_PRIVATE_DATA;
-      *end = CLEAR_PRIVATE_DATA;
-       return;
-    case SyncSessionJob::NUDGE:
-    case SyncSessionJob::POLL:
-      *start = SYNCER_BEGIN;
-      *end = SYNCER_END;
-      return;
-    case SyncSessionJob::CLEANUP_DISABLED_TYPES:
-      *start = CLEANUP_DISABLED_TYPES;
-      *end = CLEANUP_DISABLED_TYPES;
-      return;
-    default:
-      NOTREACHED();
-      *start = SYNCER_END;
-      *end = SYNCER_END;
-      return;
-  }
-}
-
 void SyncScheduler::PostTask(
     const tracked_objects::Location& from_here,
     const char* name, const base::Closure& task) {
@@ -762,6 +731,36 @@ void SyncScheduler::ScheduleSyncSessionJob(const SyncSessionJob& job) {
                   delay);
 }
 
+void SyncScheduler::SetSyncerStepsForPurpose(
+    SyncSessionJob::SyncSessionJobPurpose purpose,
+    SyncerStep* start, SyncerStep* end) {
+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  switch (purpose) {
+    case SyncSessionJob::CONFIGURATION:
+      *start = DOWNLOAD_UPDATES;
+      *end = APPLY_UPDATES;
+      return;
+    case SyncSessionJob::CLEAR_USER_DATA:
+      *start = CLEAR_PRIVATE_DATA;
+      *end = CLEAR_PRIVATE_DATA;
+       return;
+    case SyncSessionJob::NUDGE:
+    case SyncSessionJob::POLL:
+      *start = SYNCER_BEGIN;
+      *end = SYNCER_END;
+      return;
+    case SyncSessionJob::CLEANUP_DISABLED_TYPES:
+      *start = CLEANUP_DISABLED_TYPES;
+      *end = CLEANUP_DISABLED_TYPES;
+      return;
+    default:
+      NOTREACHED();
+      *start = SYNCER_END;
+      *end = SYNCER_END;
+      return;
+  }
+}
+
 void SyncScheduler::DoSyncSessionJob(const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   if (!ShouldRunJob(job)) {
diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h
index 4ec621d..d5ff4c0 100644
--- a/sync/engine/sync_scheduler.h
+++ b/sync/engine/sync_scheduler.h
@@ -179,7 +179,6 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
   };
   friend class SyncSchedulerTest;
   friend class SyncSchedulerWhiteboxTest;
-  friend class SyncerTest;
 
   FRIEND_TEST_ALL_PREFIXES(SyncSchedulerWhiteboxTest,
       DropNudgeWhileExponentialBackOff);
@@ -248,12 +247,6 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
 
   static const char* GetDecisionString(JobProcessDecision decision);
 
-  // Assign |start| and |end| to appropriate SyncerStep values for the
-  // specified |purpose|.
-  static void SetSyncerStepsForPurpose(
-      SyncSessionJob::SyncSessionJobPurpose purpose,
-      SyncerStep* start, SyncerStep* end);
-
   // Helpers that log before posting to |sync_loop_|.  These will only post
   // the task in between calls to Start/Stop.
   void PostTask(const tracked_objects::Location& from_here,
@@ -347,6 +340,12 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
   // Creates a session for a poll and performs the sync.
   void PollTimerCallback();
 
+  // Assign |start| and |end| to appropriate SyncerStep values for the
+  // specified |purpose|.
+  void SetSyncerStepsForPurpose(SyncSessionJob::SyncSessionJobPurpose purpose,
+                                SyncerStep* start,
+                                SyncerStep* end);
+
   // Used to update |connection_code_|, see below.
   void UpdateServerConnectionManagerStatus(
       HttpResponse::ServerConnectionCode code);
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 5de4ae5..62c7af8 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -110,6 +110,17 @@ void Syncer::SyncShare(sessions::SyncSession* session,
 
     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);
 
@@ -155,19 +166,13 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       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()) {
-          next_step = DOWNLOAD_UPDATES;
-        } else {
+        // 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;
         }
         break;
       }
@@ -198,19 +203,6 @@ void Syncer::SyncShare(sessions::SyncSession* 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);
 
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index b2d694e..1d8cbb0 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -33,7 +33,6 @@
 #include "sync/engine/syncer_util.h"
 #include "sync/engine/syncproto.h"
 #include "sync/engine/traffic_recorder.h"
-#include "sync/engine/sync_scheduler.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/nigori_specifics.pb.h"
 #include "sync/protocol/preference_specifics.pb.h"
@@ -187,33 +186,18 @@ class SyncerTest : public testing::Test,
         info, workers);
   }
 
-  bool SyncShareAsDelegate(
-      SyncScheduler::SyncSessionJob::SyncSessionJobPurpose purpose) {
-    SyncerStep start;
-    SyncerStep end;
-    SyncScheduler::SetSyncerStepsForPurpose(purpose, &start, &end);
-
+  bool SyncShareAsDelegate() {
     session_.reset(MakeSession());
-    syncer_->SyncShare(session_.get(), start, end);
+    syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
     return session_->HasMoreToSync();
   }
 
-  bool SyncShareNudge() {
-    session_.reset(MakeSession());
-    return SyncShareAsDelegate(SyncScheduler::SyncSessionJob::NUDGE);
-  }
-
-  bool SyncShareConfigure() {
-    session_.reset(MakeSession());
-    return SyncShareAsDelegate(SyncScheduler::SyncSessionJob::CONFIGURATION);
-  }
-
   void LoopSyncShare() {
     bool should_loop = false;
     int loop_iterations = 0;
     do {
       ASSERT_LT(++loop_iterations, 100) << "infinite loop detected. please fix";
-      should_loop = SyncShareNudge();
+      should_loop = SyncShareAsDelegate();
     } while (should_loop);
   }
 
@@ -558,7 +542,7 @@ TEST_F(SyncerTest, GetCommitIdsCommandTruncates) {
   // Create two server entries.
   mock_server_->AddUpdateDirectory(ids_.MakeServer("x"), root, "X", 10, 10);
   mock_server_->AddUpdateDirectory(ids_.MakeServer("w"), root, "W", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // Create some new client entries.
   CreateUnsyncedDirectory("C", ids_.MakeLocal("c"));
@@ -612,7 +596,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
   AddDefaultFieldValue(syncable::BOOKMARKS, &bookmark_data);
 
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -627,7 +611,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
   context_->SetUnthrottleTime(
       throttled_types,
       base::TimeTicks::Now() + base::TimeDelta::FromSeconds(1200));
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     // Nothing should have been committed as bookmarks is throttled.
@@ -641,7 +625,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
   context_->SetUnthrottleTime(
       throttled_types,
       base::TimeTicks::Now() - base::TimeDelta::FromSeconds(1200));
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // It should have been committed.
     ReadTransaction rtrans(FROM_HERE, directory());
@@ -681,7 +665,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10);
   mock_server_->AddUpdateDirectory(3, 0, "C", 10, 10);
   mock_server_->AddUpdateDirectory(4, 0, "D", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Server side change will put A in conflict.
   mock_server_->AddUpdateDirectory(1, 0, "A", 20, 20);
   {
@@ -724,7 +708,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     D.Put(SPECIFICS, encrypted_bookmark);
     D.Put(NON_UNIQUE_NAME, "not encrypted");
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // We remove any unready entries from the status controller's unsynced
     // handles, so this should remain 0 even though the entries didn't commit.
@@ -741,7 +725,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     // Resolve the pending keys.
     cryptographer(&rtrans)->DecryptPendingKeys(other_params);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // 2 unsynced handles to reflect the items that committed succesfully.
     EXPECT_EQ(2U, session_->status_controller().unsynced_handles().size());
@@ -770,7 +754,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     D.Put(SPECIFICS, encrypted_bookmark);
     D.Put(NON_UNIQUE_NAME, kEncryptedString);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // We attempted to commit two items.
     EXPECT_EQ(2U, session_->status_controller().unsynced_handles().size());
@@ -821,7 +805,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   mock_server_->AddUpdateSpecifics(2, 1, "B", 10, 10, false, 2, bookmark);
   mock_server_->AddUpdateSpecifics(3, 1, "C", 10, 10, false, 1, bookmark);
   mock_server_->AddUpdateSpecifics(4, 0, "D", 10, 10, false, 0, pref);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
     // Initial state. Everything is normal.
@@ -842,7 +826,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
                                    encrypted_bookmark);
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 20, 20, false, 0,
                                    encrypted_pref);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
     // All should be unapplied due to being undecryptable and have a valid
@@ -863,7 +847,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   // Item 3 doesn't change.
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 30, 30, false, 0,
                                    encrypted_pref);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
     // Items 1, 2, and 4 should have newer server versions, 3 remains the same.
@@ -882,7 +866,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   // Reordered to after item 2.
   mock_server_->AddUpdateSpecifics(3, 1, kEncryptedString, 30, 30, false, 3,
                                    encrypted_bookmark);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
     // Items 2 and 4 should be the only ones with BASE_SERVER_SPECIFICS set.
@@ -918,7 +902,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     D.Put(NON_UNIQUE_NAME, kEncryptedString);
     D.Put(IS_UNSYNCED, true);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
     // Item 1 remains unsynced due to there being pending keys.
@@ -936,7 +920,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     cryptographer(&rtrans)->DecryptPendingKeys(key_params);
   }
   // First cycle resolves conflicts, second cycle commits changes.
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(2, session_->status_controller().syncer_status().
       num_server_overwrites);
   EXPECT_EQ(1, session_->status_controller().syncer_status().
@@ -944,7 +928,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   // We attempted to commit item 1.
   EXPECT_EQ(1U, session_->status_controller().unsynced_handles().size());
   EXPECT_TRUE(session_->status_controller().did_commit_items());
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // Everything should be resolved now. The local changes should have
     // overwritten the server changes for 2 and 4, while the server changes
@@ -989,7 +973,7 @@ TEST_F(SyncerTest, ReceiveOldNigori) {
   sync_pb::EntitySpecifics initial_nigori_specifics;
   initial_nigori_specifics.mutable_nigori();
   mock_server_->SetNigori(1, 10, 10, initial_nigori_specifics);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     // Set up the current nigori node (containing both keys and encrypt
@@ -1016,7 +1000,7 @@ TEST_F(SyncerTest, ReceiveOldNigori) {
         cryptographer(&wtrans)->GetEncryptedTypes()));
   }
 
-  SyncShareNudge();  // Commit it.
+  SyncShareAsDelegate();  // Commit it.
 
   // Now set up the old nigori node and add it as a server update.
   sync_pb::EntitySpecifics old_nigori_specifics;
@@ -1025,7 +1009,7 @@ TEST_F(SyncerTest, ReceiveOldNigori) {
   other_cryptographer.UpdateNigoriFromEncryptedTypes(old_nigori);
   mock_server_->SetNigori(1, 30, 30, old_nigori_specifics);
 
-  SyncShareNudge();  // Download the old nigori and apply it.
+  SyncShareAsDelegate();  // Download the old nigori and apply it.
 
   {
     // Ensure everything is committed and stable now. The cryptographer
@@ -1074,7 +1058,7 @@ TEST_F(SyncerTest, NigoriConflicts) {
   our_encrypted_specifics.mutable_bookmark()->set_title("title2");
 
   // Receive the initial nigori node.
-  SyncShareNudge();
+  SyncShareAsDelegate();
   encrypted_types = syncable::ModelTypeSet::All();
   {
     // Local changes with different passphrase, different types, and sync_tabs.
@@ -1115,8 +1099,8 @@ TEST_F(SyncerTest, NigoriConflicts) {
   // data (with priority given to the server's encryption keys if they are
   // undecryptable), which we then commit. The cryptographer should have pending
   // keys and merge the set of encrypted types.
-  SyncShareNudge();  // Resolve conflict in this cycle.
-  SyncShareNudge();  // Commit local change in this cycle.
+  SyncShareAsDelegate();  // Resolve conflict in this cycle.
+  SyncShareAsDelegate();  // Commit local change in this cycle.
   {
     // Ensure the nigori data merged (encrypted types, sync_tabs).
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -1146,7 +1130,7 @@ TEST_F(SyncerTest, NigoriConflicts) {
     nigori_entry.Put(IS_UNSYNCED, true);
   }
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // Ensure everything is committed and stable now. The cryptographer
     // should be able to decrypt both sets of keys, sync_tabs should be true,
@@ -1735,11 +1719,11 @@ TEST_F(SyncerTest, UpdateWithZeroLengthName) {
   mock_server_->AddUpdateDirectory(1, 0, "", 1, 10);
   // And one legal one that we're going to delete.
   mock_server_->AddUpdateDirectory(2, 0, "FOO", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Delete the legal one. The new update has a null name.
   mock_server_->AddUpdateDirectory(2, 0, "", 2, 20);
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 }
 
 TEST_F(SyncerTest, TestBasicUpdate) {
@@ -1750,7 +1734,7 @@ TEST_F(SyncerTest, TestBasicUpdate) {
   int64 timestamp = 10;
   mock_server_->AddUpdateDirectory(id, parent_id, name, version, timestamp);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     Entry entry(&trans, GET_BY_ID,
@@ -1928,7 +1912,7 @@ TEST_F(SyncerTest, CommitTimeRename) {
   // Mix in a directory creation too for later.
   mock_server_->AddUpdateDirectory(2, 0, "dir_in_root", 10, 10);
   mock_server_->SetCommitTimeRename("renamed_");
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // Verify it was correctly renamed.
   {
@@ -1969,7 +1953,7 @@ TEST_F(SyncerTest, CommitTimeRenameI18N) {
   }
 
   mock_server_->SetCommitTimeRename(i18nString);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // Verify it was correctly renamed.
   {
@@ -2049,7 +2033,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
   mock_server_->set_conflict_all_commits(true);
 
   // Alright! Apply that update!
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // The folder's ID should have been updated.
     ReadTransaction trans(FROM_HERE, directory());
@@ -2115,7 +2099,7 @@ TEST_F(SyncerTest, CommitReuniteUpdate) {
   mock_server_->set_conflict_all_commits(true);
 
   // Alright! Apply that update!
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_HANDLE, entry_metahandle);
@@ -2178,7 +2162,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateDoesNotChokeOnDeletedLocalEntry) {
   }
 
   // Just don't CHECK fail in sync, have the update split.
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Id new_entry_id = GetOnlyEntryWithName(
@@ -2198,7 +2182,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesUnsanitizedNames) {
   mock_server_->AddUpdateDirectory(1, 0, "A/A", 10, 10);
   mock_server_->AddUpdateDirectory(2, 0, "B/B", 10, 10);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
 
@@ -2238,7 +2222,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesNormalNames) {
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
 
@@ -2342,7 +2326,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
 
 TEST_F(SyncerTest, NegativeIDInUpdate) {
   mock_server_->AddUpdateBookmark(-10, 0, "bad", 40, 40);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // The negative id would make us CHECK!
 }
 
@@ -2358,7 +2342,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
     WriteTestDataToEntry(&trans, &fred_match);
   }
   // Commit it.
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1u, mock_server_->committed_ids().size());
   mock_server_->set_conflict_all_commits(true);
   syncable::Id fred_match_id;
@@ -2374,7 +2358,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
   }
   // Run the syncer.
   for (int i = 0 ; i < 30 ; ++i) {
-    SyncShareNudge();
+    SyncShareAsDelegate();
   }
 }
 
@@ -2437,7 +2421,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
     entry.Put(syncable::MTIME, test_time);
     entry_metahandle = entry.Get(META_HANDLE);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   syncable::Id id;
   int64 version;
   int64 server_position_in_parent;
@@ -2456,7 +2440,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
   EXPECT_EQ(root_id_.GetServerId(), update->parent_id_string());
   EXPECT_EQ(version, update->version());
   EXPECT_EQ(server_position_in_parent, update->position_in_parent());
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, syncable::GET_BY_ID, id);
@@ -2491,9 +2475,9 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
   mock_server_->AddUpdateDirectory(parent_id, root_id_, "Folder", 10, 10);
   mock_server_->AddUpdateBookmark(child_id, parent_id, "test.htm", 10, 10);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
-  SyncShareNudge();
-  SyncShareNudge();
+  SyncShareAsDelegate();
+  SyncShareAsDelegate();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Directory::ChildHandles children;
@@ -2518,7 +2502,7 @@ TEST_F(SyncerTest, CommittingNewDeleted) {
     entry.Put(IS_UNSYNCED, true);
     entry.Put(IS_DEL, true);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0u, mock_server_->committed_ids().size());
 }
 
@@ -2601,7 +2585,7 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
   int64 newfolder_metahandle;
 
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry newfolder(&trans, CREATE, ids_.FromNumber(1), "local");
@@ -2624,10 +2608,10 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
 TEST_F(SyncerTest, FolderSwapUpdate) {
   mock_server_->AddUpdateDirectory(7801, 0, "bob", 1, 10);
   mock_server_->AddUpdateDirectory(1024, 0, "fred", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   mock_server_->AddUpdateDirectory(1024, 0, "bob", 2, 20);
   mock_server_->AddUpdateDirectory(7801, 0, "fred", 2, 20);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2646,7 +2630,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) {
   mock_server_->AddUpdateDirectory(7801, 0, "bob", 1, 10);
   mock_server_->AddUpdateDirectory(1024, 0, "fred", 1, 10);
   mock_server_->AddUpdateDirectory(4096, 0, "alice", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2665,7 +2649,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) {
   mock_server_->AddUpdateDirectory(1024, 0, "bob", 2, 20);
   mock_server_->AddUpdateDirectory(7801, 0, "fred", 2, 20);
   mock_server_->AddUpdateDirectory(4096, 0, "bob", 2, 20);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2699,7 +2683,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo) {
     }
   }
   uint32 num_loops = 0;
-  while (SyncShareNudge()) {
+  while (SyncShareAsDelegate()) {
     num_loops++;
     ASSERT_LT(num_loops, max_batches * 2);
   }
@@ -2724,7 +2708,7 @@ TEST_F(SyncerTest, HugeConflict) {
       last_id = next_id;
     }
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // Check they're in the expected conflict state.
   {
@@ -2741,7 +2725,7 @@ TEST_F(SyncerTest, HugeConflict) {
   // Add the missing parent directory.
   mock_server_->AddUpdateDirectory(parent_id, TestIdFactory::root(),
       "BOB", 2, 20);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // Now they should all be OK.
   {
@@ -2757,7 +2741,7 @@ TEST_F(SyncerTest, HugeConflict) {
 
 TEST_F(SyncerTest, DontCrashOnCaseChange) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry e(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -2766,22 +2750,22 @@ TEST_F(SyncerTest, DontCrashOnCaseChange) {
   }
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateDirectory(1, 0, "BOB", 2, 20);
-  SyncShareNudge();  // USED TO CAUSE AN ASSERT
+  SyncShareAsDelegate();  // USED TO CAUSE AN ASSERT
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, UnsyncedItemAndUpdate) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateDirectory(2, 0, "bob", 2, 20);
-  SyncShareNudge();  // USED TO CAUSE AN ASSERT
+  SyncShareAsDelegate();  // USED TO CAUSE AN ASSERT
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   int64 local_folder_handle;
   syncable::Id local_folder_id;
   {
@@ -2798,7 +2782,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   }
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
   {
     // Update #20 should have been dropped in favor of the local version.
@@ -2817,13 +2801,13 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   }
   // Allow local changes to commit.
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
 
   // Now add a server change to make the two names equal.  There should
   // be no conflict with that, since names are not unique.
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -2847,7 +2831,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
 TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   mock_server_->set_use_legacy_bookmarks_protocol(true);
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   int64 local_folder_handle;
   syncable::Id local_folder_id;
   {
@@ -2864,7 +2848,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   }
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
   {
     // Update #20 should have been dropped in favor of the local version.
@@ -2883,13 +2867,13 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   }
   // Allow local changes to commit.
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
 
   // Now add a server change to make the two names equal.  There should
   // be no conflict with that, since names are not unique.
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 30, 30);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -2914,7 +2898,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
   // we don't currently resolve this. This test ensures we don't.
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
@@ -2925,7 +2909,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
   }
   mock_server_->AddUpdateDirectory(2, 1, "A", 20, 20);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -2943,7 +2927,7 @@ TEST_F(SyncerTest, SwapEntryNames) {
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
@@ -2956,7 +2940,7 @@ TEST_F(SyncerTest, SwapEntryNames) {
     ASSERT_TRUE(B.Put(NON_UNIQUE_NAME, "A"));
     ASSERT_TRUE(A.Put(NON_UNIQUE_NAME, "B"));
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   saw_syncer_event_ = false;
 }
 
@@ -2964,7 +2948,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) {
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
   mock_server_->AddUpdateBookmark(2, 0, "B", 10, 10);
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry B(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -2974,7 +2958,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) {
   }
   mock_server_->AddUpdateBookmark(2, 0, "A", 11, 11);
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry B(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -2989,7 +2973,7 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
   int64 bob_metahandle;
 
   mock_server_->AddUpdateBookmark(1, 0, "bob", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3000,8 +2984,8 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
   mock_server_->AddUpdateBookmark(1, 0, "bob", 2, 10);
   mock_server_->SetLastUpdateDeleted();
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
-  SyncShareNudge();
+  SyncShareAsDelegate();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry bob(&trans, GET_BY_HANDLE, bob_metahandle);
@@ -3035,16 +3019,16 @@ TEST_F(SyncerTest, DuplicateIDReturn) {
   mock_server_->set_next_new_id(10000);
   EXPECT_EQ(1u, directory()->unsynced_entity_count());
   // we get back a bad id in here (should never happen).
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1u, directory()->unsynced_entity_count());
-  SyncShareNudge();  // another bad id in here.
+  SyncShareAsDelegate();  // another bad id in here.
   EXPECT_EQ(0u, directory()->unsynced_entity_count());
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3055,8 +3039,8 @@ TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) {
     bob.Put(IS_UNSYNCED, true);
   }
   mock_server_->AddUpdateDirectory(2, 1, "fred", 1, 10);
-  SyncShareNudge();
-  SyncShareNudge();
+  SyncShareAsDelegate();
+  SyncShareAsDelegate();
 }
 
 TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
@@ -3076,7 +3060,7 @@ TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
 
   // We don't care about actually committing, just the resolution.
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3112,7 +3096,7 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
   mock_server_->set_conflict_all_commits(true);
 
   // The syncer should not attempt to apply the invalid update.
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3131,7 +3115,7 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
 TEST_F(SyncerTest, MergingExistingItems) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, CREATE, trans.root_id(), "Copy of base");
@@ -3199,7 +3183,7 @@ TEST_F(SyncerTest, DontMergeTwoExistingItems) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10);
   mock_server_->AddUpdateBookmark(2, 0, "base2", 10, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3227,10 +3211,10 @@ TEST_F(SyncerTest, TestUndeleteUpdate) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateDirectory(1, 0, "foo", 1, 1);
   mock_server_->AddUpdateDirectory(2, 1, "bar", 1, 2);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   mock_server_->AddUpdateDirectory(2, 1, "bar", 2, 3);
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   int64 metahandle;
   {
@@ -3242,11 +3226,11 @@ TEST_F(SyncerTest, TestUndeleteUpdate) {
   }
   mock_server_->AddUpdateDirectory(1, 0, "foo", 2, 4);
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // This used to be rejected as it's an undeletion. Now, it results in moving
   // the delete path aside.
   mock_server_->AddUpdateDirectory(2, 1, "bar", 3, 5);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3261,7 +3245,7 @@ TEST_F(SyncerTest, TestUndeleteUpdate) {
 TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) {
   mock_server_->AddUpdateDirectory(1, 0, "foo", 1, 1);
   mock_server_->AddUpdateDirectory(2, 0, ":::", 1, 2);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3269,10 +3253,10 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) {
     EXPECT_TRUE(entry.Put(PARENT_ID, ids_.FromNumber(1)));
     EXPECT_TRUE(entry.Put(IS_UNSYNCED, true));
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // We use the same sync ts as before so our times match up.
   mock_server_->AddUpdateDirectory(2, 1, ":::", 2, 2);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 }
 
 // Don't crash when this occurs.
@@ -3280,7 +3264,7 @@ TEST_F(SyncerTest, UpdateWhereParentIsNotAFolder) {
   mock_server_->AddUpdateBookmark(1, 0, "B", 10, 10);
   mock_server_->AddUpdateDirectory(2, 1, "BookmarkParent", 10, 10);
   // Used to cause a CHECK
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction rtrans(FROM_HERE, directory());
     Entry good_entry(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1));
@@ -3302,7 +3286,7 @@ TEST_F(SyncerTest, DirectoryUpdateTest) {
                                    "in_root_name", 2, 2);
   mock_server_->AddUpdateDirectory(in_in_root_id, in_root_id,
                                    "in_in_root_name", 3, 3);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry in_root(&trans, GET_BY_ID, in_root_id);
@@ -3340,7 +3324,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
     bar_metahandle = child.Get(META_HANDLE);
     in_dir_id = parent.Get(syncable::ID);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     ReadTransaction trans(FROM_HERE, directory());
     Entry fail_by_old_id_entry(&trans, GET_BY_ID, in_root_id);
@@ -3367,7 +3351,7 @@ TEST_F(SyncerTest, TestClientCommand) {
   command->set_set_sync_long_poll_interval(800);
   command->set_sessions_commit_delay_seconds(3141);
   mock_server_->AddUpdateDirectory(1, 0, "in_root", 1, 1);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_TRUE(TimeDelta::FromSeconds(8) ==
               last_short_poll_interval_received_);
@@ -3381,7 +3365,7 @@ TEST_F(SyncerTest, TestClientCommand) {
   command->set_set_sync_long_poll_interval(190);
   command->set_sessions_commit_delay_seconds(2718);
   mock_server_->AddUpdateDirectory(1, 0, "in_root", 1, 1);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_TRUE(TimeDelta::FromSeconds(180) ==
               last_short_poll_interval_received_);
@@ -3399,7 +3383,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
       "folder_one", 1, 1);
   mock_server_->AddUpdateDirectory(folder_two_id, TestIdFactory::root(),
       "folder_two", 1, 1);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   {
     // A moved entry should send an "old parent."
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
@@ -3412,7 +3396,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
     create.Put(IS_UNSYNCED, true);
     create.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit();
   ASSERT_EQ(2, commit.entries_size());
   EXPECT_TRUE(commit.entries(0).parent_id_string() == "2");
@@ -3447,7 +3431,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
   mock_server_->set_conflict_all_commits(true);
   // Let there be an entry from the server.
   mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Check it out and delete it.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3459,7 +3443,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
     // Delete it locally.
     entry.Put(IS_DEL, true);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Confirm we see IS_DEL and not SERVER_IS_DEL.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3470,11 +3454,11 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
     EXPECT_TRUE(entry.Get(IS_DEL));
     EXPECT_FALSE(entry.Get(SERVER_IS_DEL));
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Update from server confirming deletion.
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 11);
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // IS_DEL AND SERVER_IS_DEL now both true.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3487,7 +3471,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
   }
   // Undelete from server.
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // IS_DEL and SERVER_IS_DEL now both false.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3505,7 +3489,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
   // Let there be a entry, from the server.
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Check it out and delete it.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3517,7 +3501,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
     // Delete it locally.
     entry.Put(IS_DEL, true);
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Confirm we see IS_DEL and not SERVER_IS_DEL.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3528,11 +3512,11 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
     EXPECT_TRUE(entry.Get(IS_DEL));
     EXPECT_FALSE(entry.Get(SERVER_IS_DEL));
   }
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // Say we do not get an update from server confirming deletion. Undelete
   // from server
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // IS_DEL and SERVER_IS_DEL now both false.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3552,16 +3536,16 @@ TEST_F(SyncerTest, TestUndeleteIgnoreCorrectlyUnappliedUpdate) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(id1, root, "foo", 1, 10);
   mock_server_->AddUpdateBookmark(id2, root, "foo", 1, 10);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   mock_server_->AddUpdateBookmark(id2, root, "foo2", 2, 20);
-  SyncShareNudge();  // Now just don't explode.
+  SyncShareAsDelegate();  // Now just don't explode.
 }
 
 TEST_F(SyncerTest, ClientTagServerCreatedUpdatesWork) {
   mock_server_->AddUpdateDirectory(1, 0, "permitem1", 1, 10);
   mock_server_->SetLastUpdateClientTag("permfolder");
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3576,7 +3560,7 @@ TEST_F(SyncerTest, ClientTagServerCreatedUpdatesWork) {
 
   mock_server_->AddUpdateDirectory(1, 0, "permitem_renamed", 10, 100);
   mock_server_->SetLastUpdateClientTag("permfolder");
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3595,7 +3579,7 @@ TEST_F(SyncerTest, ClientTagIllegalUpdateIgnored) {
   mock_server_->AddUpdateDirectory(1, 0, "permitem1", 1, 10);
   mock_server_->SetLastUpdateClientTag("permfolder");
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3610,7 +3594,7 @@ TEST_F(SyncerTest, ClientTagIllegalUpdateIgnored) {
 
   mock_server_->AddUpdateDirectory(1, 0, "permitem_renamed", 10, 100);
   mock_server_->SetLastUpdateClientTag("wrongtag");
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3655,7 +3639,7 @@ TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
       CopyFrom(server_bookmark);
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // This should cause client tag reunion, preserving the metahandle.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3677,7 +3661,7 @@ TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
   }
 
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The resolved entry ought to commit cleanly.
   {
@@ -3716,7 +3700,7 @@ TEST_F(SyncerTest, ClientTagConflictWithDeletedLocalEntry) {
   mock_server_->SetLastUpdateClientTag("clientperm");
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // This should cause client tag overwrite.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3745,7 +3729,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
 
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   int64 tag1_metahandle = syncable::kInvalidMetaHandle;
   int64 tag2_metahandle = syncable::kInvalidMetaHandle;
   // This should cause client tag overwrite.
@@ -3785,7 +3769,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
   mock_server_->SetLastUpdateClientTag("tag1");
   mock_server_->AddUpdateBookmark(3, 0, "Three", 13, 130);
   mock_server_->SetLastUpdateClientTag("tag2");
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3857,7 +3841,7 @@ TEST_F(SyncerTest, ClientTagClashWithinBatchOfUpdates) {
 
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   // This should cause client tag overwrite.
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3925,7 +3909,7 @@ TEST_F(SyncerTest, UniqueServerTagUpdates) {
   mock_server_->SetLastUpdateServerTag("alpha");
   mock_server_->AddUpdateDirectory(2, 0, "update2", 2, 20);
   mock_server_->SetLastUpdateServerTag("bob");
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   {
     ReadTransaction trans(FROM_HERE, directory());
@@ -3956,146 +3940,31 @@ TEST_F(SyncerTest, GetUpdatesSetsRequestedTypes) {
   // GetUpdates handler.  EnableDatatype sets the expectation value from our
   // set of enabled/disabled datatypes.
   EnableDatatype(syncable::BOOKMARKS);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   EnableDatatype(syncable::AUTOFILL);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   EnableDatatype(syncable::PREFERENCES);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(syncable::BOOKMARKS);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(syncable::AUTOFILL);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(syncable::PREFERENCES);
   EnableDatatype(syncable::AUTOFILL);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 }
 
-// A typical scenario: server and client each have one update for the other.
-// It is the "happy path" alternative to the test below.
-TEST_F(SyncerTest, UpdateThenCommit) {
-  syncable::Id to_receive = ids_.NewServerId();
-  syncable::Id to_commit = ids_.NewLocalId();
-
-  mock_server_->AddUpdateDirectory(to_receive, ids_.root(), "x", 1, 10);
-  int64 commit_handle = CreateUnsyncedDirectory("y", to_commit);
-  SyncShareNudge();
-
-  ReadTransaction trans(FROM_HERE, directory());
-
-  Entry received(&trans, GET_BY_ID, to_receive);
-  ASSERT_TRUE(received.good());
-  EXPECT_FALSE(received.Get(IS_UNSYNCED));
-  EXPECT_FALSE(received.Get(IS_UNAPPLIED_UPDATE));
-
-  Entry committed(&trans, GET_BY_HANDLE, commit_handle);
-  ASSERT_TRUE(committed.good());
-  EXPECT_FALSE(committed.Get(IS_UNSYNCED));
-  EXPECT_FALSE(committed.Get(IS_UNAPPLIED_UPDATE));
-}
-
-// Same as above, but this time we fail to download updates.
-// We should not attempt to commit anything unless we successfully downloaded
-// updates, otherwise we risk causing a server-side conflict.
-TEST_F(SyncerTest, UpdateFailsThenDontCommit) {
-  syncable::Id to_receive = ids_.NewServerId();
-  syncable::Id to_commit = ids_.NewLocalId();
-
-  mock_server_->AddUpdateDirectory(to_receive, ids_.root(), "x", 1, 10);
-  int64 commit_handle = CreateUnsyncedDirectory("y", to_commit);
-  mock_server_->FailNextPostBufferToPathCall();
-  SyncShareNudge();
-
-  ReadTransaction trans(FROM_HERE, directory());
-
-  // We did not receive this update.
-  Entry received(&trans, GET_BY_ID, to_receive);
-  ASSERT_FALSE(received.good());
-
-  // And our local update remains unapplied.
-  Entry committed(&trans, GET_BY_HANDLE, commit_handle);
-  ASSERT_TRUE(committed.good());
-  EXPECT_TRUE(committed.Get(IS_UNSYNCED));
-  EXPECT_FALSE(committed.Get(IS_UNAPPLIED_UPDATE));
-
-  // Inform the Mock we won't be fetching all updates.
-  mock_server_->ClearUpdatesQueue();
-}
-
-// Downloads two updates and applies them successfully.
-// This is the "happy path" alternative to the test below.
-TEST_F(SyncerTest, ConfigureDownloadsTwoBatchesSuccess) {
-  syncable::Id node1 = ids_.NewServerId();
-  syncable::Id node2 = ids_.NewServerId();
-
-  // Construct the first GetUpdates response.
-  mock_server_->AddUpdateDirectory(node1, ids_.root(), "one", 1, 10);
-  mock_server_->SetChangesRemaining(1);
-  mock_server_->NextUpdateBatch();
-
-  // Construct the second GetUpdates response.
-  mock_server_->AddUpdateDirectory(node2, ids_.root(), "two", 1, 20);
-
-  SyncShareConfigure();
-
-  ReadTransaction trans(FROM_HERE, directory());
-  // Both nodes should be downloaded and applied.
-
-  Entry n1(&trans, GET_BY_ID, node1);
-  ASSERT_TRUE(n1.good());
-  EXPECT_FALSE(n1.Get(IS_UNAPPLIED_UPDATE));
-
-  Entry n2(&trans, GET_BY_ID, node2);
-  ASSERT_TRUE(n2.good());
-  EXPECT_FALSE(n2.Get(IS_UNAPPLIED_UPDATE));
-}
-
-// Same as the above case, but this time the second batch fails to download.
-TEST_F(SyncerTest, ConfigureFailsDontApplyUpdates) {
-  syncable::Id node1 = ids_.NewServerId();
-  syncable::Id node2 = ids_.NewServerId();
-
-  // The scenario: we have two batches of updates with one update each.  A
-  // normal confgure step would download all the updates one batch at a time and
-  // apply them.  This configure will succeed in downloading the first batch
-  // then fail when downloading the second.
-  mock_server_->FailNthPostBufferToPathCall(2);
-
-  // Construct the first GetUpdates response.
-  mock_server_->AddUpdateDirectory(node1, ids_.root(), "one", 1, 10);
-  mock_server_->SetChangesRemaining(1);
-  mock_server_->NextUpdateBatch();
-
-  // Consutrct the second GetUpdates response.
-  mock_server_->AddUpdateDirectory(node2, ids_.root(), "two", 1, 20);
-
-  SyncShareConfigure();
-
-  ReadTransaction trans(FROM_HERE, directory());
-
-  // The first node was downloaded, but not applied.
-  Entry n1(&trans, GET_BY_ID, node1);
-  ASSERT_TRUE(n1.good());
-  EXPECT_TRUE(n1.Get(IS_UNAPPLIED_UPDATE));
-
-  // The second node was not downloaded.
-  Entry n2(&trans, GET_BY_ID, node2);
-  EXPECT_FALSE(n2.good());
-
-  // One update remains undownloaded.
-  mock_server_->ClearUpdatesQueue();
-}
-
 // Test what happens if a client deletes, then recreates, an object very
 // quickly.  It is possible that the deletion gets sent as a commit, and
 // the undelete happens during the commit request.  The principle here
@@ -4233,7 +4102,7 @@ class SyncerUndeletionTest : public SyncerTest {
 TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4244,7 +4113,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   ExpectUnsyncedDeletion();
   mock_server_->SetMidCommitCallback(
       base::Bind(&SyncerUndeletionTest::Undelete, base::Unretained(this)));
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The item ought to exist as an unsynced undeletion (meaning,
   // we think that the next commit ought to be a recreation commit).
@@ -4258,7 +4127,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   // normal to recieve updates from our own commits.
   mock_server_->SetMidCommitCallback(base::Closure());
   mock_server_->AddUpdateTombstone(Get(metahandle_, ID));
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4267,7 +4136,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
 TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4278,7 +4147,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   ExpectUnsyncedDeletion();
   Undelete();
   ExpectUnsyncedEdit();  // Edit, not undelete: server thinks it exists.
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4289,7 +4158,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // update.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4298,7 +4167,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4307,7 +4176,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   // Delete and commit.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4321,7 +4190,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4330,14 +4199,14 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
 
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -4345,7 +4214,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   // Delete and commit.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4355,7 +4224,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  Should be consistent.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndDeleted();
@@ -4366,7 +4235,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
 
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4376,7 +4245,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4384,14 +4253,14 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
 
   // Add a delete from the server.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
 
   // Some other client deletes the item.
   mock_server_->AddUpdateTombstone(Get(metahandle_, ID));
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4401,7 +4270,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   // Undelete it locally.
   Undelete();
   ExpectUnsyncedUndeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4409,7 +4278,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4418,7 +4287,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4427,7 +4296,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   // Some other client deletes the item before we get a chance
   // to GetUpdates our original request.
   mock_server_->AddUpdateTombstone(Get(metahandle_, ID));
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4437,7 +4306,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   // Undelete it locally.
   Undelete();
   ExpectUnsyncedUndeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4445,7 +4314,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4454,7 +4323,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
 TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4462,7 +4331,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
 
   // Get the updates of our just-committed entry.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -4470,7 +4339,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   // We delete the item.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4480,7 +4349,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndDeleted();
@@ -4490,7 +4359,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
                                   Get(metahandle_, PARENT_ID),
                                   "Thadeusz", 100, 1000);
   mock_server_->SetLastUpdateClientTag(client_tag_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4500,7 +4369,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
 TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4508,7 +4377,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
 
   // Get the updates of our just-committed entry.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -4516,7 +4385,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
   // We delete the item.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  SyncShareAsDelegate();
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -4529,7 +4398,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
                                   Get(metahandle_, PARENT_ID),
                                   "Thadeusz", 100, 1000);
   mock_server_->SetLastUpdateClientTag(client_tag_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -4594,7 +4463,7 @@ TEST_F(SyncerPositionUpdateTest, InOrderPositive) {
   AddRootItemWithPosition(201);
   AddRootItemWithPosition(400);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 }
 
@@ -4606,7 +4475,7 @@ TEST_F(SyncerPositionUpdateTest, InOrderNegative) {
   AddRootItemWithPosition(-150);
   AddRootItemWithPosition(100);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 }
 
@@ -4621,7 +4490,7 @@ TEST_F(SyncerPositionUpdateTest, ReverseOrder) {
   AddRootItemWithPosition(-200);
   AddRootItemWithPosition(-400);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 }
 
@@ -4633,7 +4502,7 @@ TEST_F(SyncerPositionUpdateTest, RandomOrderInBatches) {
   AddRootItemWithPosition(-400);
   AddRootItemWithPosition(100);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 
   AddRootItemWithPosition(-150);
@@ -4641,12 +4510,12 @@ TEST_F(SyncerPositionUpdateTest, RandomOrderInBatches) {
   AddRootItemWithPosition(200);
   AddRootItemWithPosition(-201);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 
   AddRootItemWithPosition(-144);
 
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalItemsInServerOrder();
 }
 
@@ -4706,7 +4575,7 @@ TEST_F(SyncerPositionTiebreakingTest, LowMidHigh) {
   Add(low_id_);
   Add(mid_id_);
   Add(high_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
@@ -4714,7 +4583,7 @@ TEST_F(SyncerPositionTiebreakingTest, LowHighMid) {
   Add(low_id_);
   Add(high_id_);
   Add(mid_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
@@ -4722,7 +4591,7 @@ TEST_F(SyncerPositionTiebreakingTest, HighMidLow) {
   Add(high_id_);
   Add(mid_id_);
   Add(low_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
@@ -4730,7 +4599,7 @@ TEST_F(SyncerPositionTiebreakingTest, HighLowMid) {
   Add(high_id_);
   Add(low_id_);
   Add(mid_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
@@ -4738,7 +4607,7 @@ TEST_F(SyncerPositionTiebreakingTest, MidHighLow) {
   Add(mid_id_);
   Add(high_id_);
   Add(low_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
@@ -4746,7 +4615,7 @@ TEST_F(SyncerPositionTiebreakingTest, MidLowHigh) {
   Add(mid_id_);
   Add(low_id_);
   Add(high_id_);
-  SyncShareNudge();
+  SyncShareAsDelegate();
   ExpectLocalOrderIsByServerId();
 }
 
diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h
index 7e2f258..c2a3cfc 100644
--- a/sync/sessions/status_controller.h
+++ b/sync/sessions/status_controller.h
@@ -175,8 +175,7 @@ class StatusController {
   // Returns true if the last download_updates_command received a valid
   // server response.
   bool download_updates_succeeded() const {
-    return shared_.error.value().last_download_updates_result
-        == SYNCER_OK;
+    return updates_response().has_get_updates();
   }
 
   // Returns true if the last updates response indicated that we were fully
diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc
index d5ce46a..48524a4 100644
--- a/sync/sessions/sync_session_unittest.cc
+++ b/sync/sessions/sync_session_unittest.cc
@@ -215,8 +215,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfUnsyncedGreaterThanCommitted) {
 TEST_F(SyncSessionTest, MoreToDownloadIfDownloadFailed) {
   status()->set_updates_request_types(ParamsMeaningAllEnabledTypes());
 
-  status()->set_last_download_updates_result(NETWORK_IO_ERROR);
-
   // When DownloadUpdatesCommand fails, these should be false.
   EXPECT_FALSE(status()->ServerSaysNothingMoreToDownload());
   EXPECT_FALSE(status()->download_updates_succeeded());
@@ -231,7 +229,6 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotChangesRemaining) {
 
   // When the server returns changes_remaining, that means there's
   // more to download.
-  status()->set_last_download_updates_result(SYNCER_OK);
   status()->mutable_updates_response()->mutable_get_updates()
      ->set_changes_remaining(1000L);
   EXPECT_FALSE(status()->ServerSaysNothingMoreToDownload());
@@ -245,7 +242,54 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotChangesRemaining) {
 TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemaining) {
   status()->set_updates_request_types(ParamsMeaningAllEnabledTypes());
 
-  status()->set_last_download_updates_result(SYNCER_OK);
+  // When the server returns a timestamp, that means we're up to date.
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->set_changes_remaining(0);
+  EXPECT_TRUE(status()->ServerSaysNothingMoreToDownload());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemainingForSubset) {
+  status()->set_updates_request_types(ParamsMeaningJustOneEnabledType());
+
+  // When the server returns a timestamp, that means we're up to date for that
+  // type.  But there may still be more to download if there are other
+  // datatypes that we didn't request on this go-round.
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->set_changes_remaining(0);
+
+  EXPECT_TRUE(status()->ServerSaysNothingMoreToDownload());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+TEST_F(SyncSessionTest, MoreToDownloadIfGotChangesRemainingAndEntries) {
+  status()->set_updates_request_types(ParamsMeaningAllEnabledTypes());
+  // The actual entry count should not factor into the HasMoreToSync
+  // determination.
+  status()->mutable_updates_response()->mutable_get_updates()->add_entries();
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->set_changes_remaining(1000000L);;
+  EXPECT_FALSE(status()->ServerSaysNothingMoreToDownload());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemainingAndEntries) {
+  status()->set_updates_request_types(ParamsMeaningAllEnabledTypes());
+  // The actual entry count should not factor into the HasMoreToSync
+  // determination.
+  status()->mutable_updates_response()->mutable_get_updates()->add_entries();
   status()->mutable_updates_response()->mutable_get_updates()
       ->set_changes_remaining(0);
   EXPECT_TRUE(status()->ServerSaysNothingMoreToDownload());
diff --git a/sync/test/engine/mock_connection_manager.cc b/sync/test/engine/mock_connection_manager.cc
index 127e858..ab95bce 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_(""),
-      countdown_to_postbuffer_fail_(0),
+      fail_next_postbuffer_(false),
       directory_(directory),
       mid_commit_observer_(NULL),
       throttling_(false),
@@ -111,9 +111,8 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
     InvalidateAndClearAuthToken();
   }
 
-  if (--countdown_to_postbuffer_fail_ == 0) {
-    // Fail as countdown hits zero.
-    params->response.server_status = HttpResponse::SYNC_SERVER_ERROR;
+  if (fail_next_postbuffer_) {
+    fail_next_postbuffer_ = false;
     return false;
   }
 
diff --git a/sync/test/engine/mock_connection_manager.h b/sync/test/engine/mock_connection_manager.h
index 5cc1b87..a2b5060 100644
--- a/sync/test/engine/mock_connection_manager.h
+++ b/sync/test/engine/mock_connection_manager.h
@@ -117,8 +117,7 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   // issue multiple requests during a sync cycle.
   void NextUpdateBatch();
 
-  void FailNextPostBufferToPathCall() { countdown_to_postbuffer_fail_ = 1; }
-  void FailNthPostBufferToPathCall(int n) { countdown_to_postbuffer_fail_ = n; }
+  void FailNextPostBufferToPathCall() { fail_next_postbuffer_ = true; }
 
   void SetClearUserDataResponseStatus(sync_pb::SyncEnums::ErrorType errortype);
 
@@ -223,11 +222,6 @@ 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();
 
@@ -305,9 +299,8 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   bool client_stuck_;
   std::string commit_time_rename_prepended_string_;
 
-  // On each PostBufferToPath() call, we decrement this counter.  The call fails
-  // iff we hit zero at that call.
-  int countdown_to_postbuffer_fail_;
+  // Fail on the next call to PostBufferToPath().
+  bool fail_next_postbuffer_;
 
   // 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