Reland [Sync] Refactoring polling to be reliable.

This is a reland of https://codereview.chromium.org/1132013004/

Polling is now an important component of sync health, as it can sometimes be
the only time we'll query for certain datatypes. As such, the following
has changed:
- Polls that fail will be retried (with backoff).
- As such, the logic to force a poll after an auth error isn't needed anymore
- The last successful poll time will be persisted in the sync prefs.
- On startup, schedule the first poll for last_poll_time + poll_interval
  (or Now(), whichever is latest).
- Receiving a new poll interval from the server will update the poll timer
- The poll timer is now a one shot timer, and only restarts on success
- Some code cleanup to make the above more straightforward

BUG=482154

Review URL: https://codereview.chromium.org/1144543004

Cr-Commit-Position: refs/heads/master@{#330024}

35 files changed, 640 insertions, 475 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host_core.cc b/chrome/browser/sync/glue/sync_backend_host_core.cc
index cade550..ffce750 100644
--- a/chrome/browser/sync/glue/sync_backend_host_core.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_core.cc
@@ -457,9 +457,10 @@ void SyncBackendHostCore::DoUpdateCredentials(
 }
 
 void SyncBackendHostCore::DoStartSyncing(
-    const syncer::ModelSafeRoutingInfo& routing_info) {
+    const syncer::ModelSafeRoutingInfo& routing_info,
+    base::Time last_poll_time) {
   DCHECK_EQ(base::MessageLoop::current(), sync_loop_);
-  sync_manager_->StartSyncingNormally(routing_info);
+  sync_manager_->StartSyncingNormally(routing_info, last_poll_time);
 }
 
 void SyncBackendHostCore::DoSetEncryptionPassphrase(
diff --git a/chrome/browser/sync/glue/sync_backend_host_core.h b/chrome/browser/sync/glue/sync_backend_host_core.h
index 88a7e6f0..6bcb767 100644
--- a/chrome/browser/sync/glue/sync_backend_host_core.h
+++ b/chrome/browser/sync/glue/sync_backend_host_core.h
@@ -150,7 +150,8 @@ class SyncBackendHostCore
 
   // Called to tell the syncapi to start syncing (generally after
   // initialization and authentication).
-  void DoStartSyncing(const syncer::ModelSafeRoutingInfo& routing_info);
+  void DoStartSyncing(const syncer::ModelSafeRoutingInfo& routing_info,
+                      base::Time last_poll_time);
 
   // Called to set the passphrase for encryption.
   void DoSetEncryptionPassphrase(const std::string& passphrase,
diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.cc b/chrome/browser/sync/glue/sync_backend_host_impl.cc
index fa0ba02..b84389c 100644
--- a/chrome/browser/sync/glue/sync_backend_host_impl.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_impl.cc
@@ -187,7 +187,7 @@ void SyncBackendHostImpl::StartSyncingWithServer() {
 
   registrar_->sync_thread()->message_loop()->PostTask(FROM_HERE,
       base::Bind(&SyncBackendHostCore::DoStartSyncing,
-                 core_.get(), routing_info));
+                 core_.get(), routing_info, sync_prefs_->GetLastPollTime()));
 }
 
 void SyncBackendHostImpl::SetEncryptionPassphrase(const std::string& passphrase,
@@ -719,6 +719,9 @@ void SyncBackendHostImpl::HandleSyncCycleCompletedOnFrontendLoop(
 
   SDVLOG(1) << "Got snapshot " << snapshot.ToString();
 
+  if (!snapshot.poll_finish_time().is_null())
+    sync_prefs_->SetLastPollTime(snapshot.poll_finish_time());
+
   // Process any changes to the datatypes we're syncing.
   // TODO(sync): add support for removing types.
   if (initialized())
diff --git a/components/sync_driver/pref_names.cc b/components/sync_driver/pref_names.cc
index b167a0b..1a5727a 100644
--- a/components/sync_driver/pref_names.cc
+++ b/components/sync_driver/pref_names.cc
@@ -11,6 +11,9 @@ namespace prefs {
 // 64-bit integer serialization of the base::Time when the last sync occurred.
 const char kSyncLastSyncedTime[] = "sync.last_synced_time";
 
+// 64-bit integer serialization of the base::Time of the last sync poll.
+const char kSyncLastPollTime[] = "sync.last_poll_time";
+
 // Boolean specifying whether the user finished setting up sync.
 const char kSyncHasSetupCompleted[] = "sync.has_setup_completed";
 
diff --git a/components/sync_driver/pref_names.h b/components/sync_driver/pref_names.h
index 5760deb..3cbce22 100644
--- a/components/sync_driver/pref_names.h
+++ b/components/sync_driver/pref_names.h
@@ -11,6 +11,7 @@ namespace sync_driver {
 namespace prefs {
 
 extern const char kSyncLastSyncedTime[];
+extern const char kSyncLastPollTime[];
 extern const char kSyncHasAuthError[];
 extern const char kSyncHasSetupCompleted[];
 extern const char kSyncKeepEverythingSynced[];
diff --git a/components/sync_driver/sync_prefs.cc b/components/sync_driver/sync_prefs.cc
index 89f5320..82106ee 100644
--- a/components/sync_driver/sync_prefs.cc
+++ b/components/sync_driver/sync_prefs.cc
@@ -38,6 +38,7 @@ void SyncPrefs::RegisterProfilePrefs(
   registry->RegisterBooleanPref(prefs::kSyncHasSetupCompleted, false);
   registry->RegisterBooleanPref(prefs::kSyncSuppressStart, false);
   registry->RegisterInt64Pref(prefs::kSyncLastSyncedTime, 0);
+  registry->RegisterInt64Pref(prefs::kSyncLastPollTime, 0);
   registry->RegisterInt64Pref(prefs::kSyncFirstSyncTime, 0);
 
   // All datatypes are on by default, but this gets set explicitly
@@ -97,6 +98,7 @@ void SyncPrefs::RemoveSyncPrefObserver(SyncPrefObserver* sync_pref_observer) {
 void SyncPrefs::ClearPreferences() {
   DCHECK(CalledOnValidThread());
   pref_service_->ClearPref(prefs::kSyncLastSyncedTime);
+  pref_service_->ClearPref(prefs::kSyncLastPollTime);
   pref_service_->ClearPref(prefs::kSyncHasSetupCompleted);
   pref_service_->ClearPref(prefs::kSyncEncryptionBootstrapToken);
   pref_service_->ClearPref(prefs::kSyncKeystoreEncryptionBootstrapToken);
@@ -148,6 +150,17 @@ void SyncPrefs::SetLastSyncedTime(base::Time time) {
   pref_service_->SetInt64(prefs::kSyncLastSyncedTime, time.ToInternalValue());
 }
 
+base::Time SyncPrefs::GetLastPollTime() const {
+  DCHECK(CalledOnValidThread());
+  return base::Time::FromInternalValue(
+      pref_service_->GetInt64(prefs::kSyncLastSyncedTime));
+}
+
+void SyncPrefs::SetLastPollTime(base::Time time) {
+  DCHECK(CalledOnValidThread());
+  pref_service_->SetInt64(prefs::kSyncLastPollTime, time.ToInternalValue());
+}
+
 bool SyncPrefs::HasKeepEverythingSynced() const {
   DCHECK(CalledOnValidThread());
   return pref_service_->GetBoolean(prefs::kSyncKeepEverythingSynced);
diff --git a/components/sync_driver/sync_prefs.h b/components/sync_driver/sync_prefs.h
index 2310946..5a3abe5 100644
--- a/components/sync_driver/sync_prefs.h
+++ b/components/sync_driver/sync_prefs.h
@@ -80,6 +80,9 @@ class SyncPrefs : NON_EXPORTED_BASE(public base::NonThreadSafe),
   base::Time GetLastSyncedTime() const;
   void SetLastSyncedTime(base::Time time);
 
+  base::Time GetLastPollTime() const;
+  void SetLastPollTime(base::Time time);
+
   bool HasKeepEverythingSynced() const;
   void SetKeepEverythingSynced(bool keep_everything_synced);
 
diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h
index 5dd1fd0..37ca968 100644
--- a/sync/engine/sync_scheduler.h
+++ b/sync/engine/sync_scheduler.h
@@ -68,8 +68,9 @@ class SYNC_EXPORT_PRIVATE SyncScheduler
 
   // Start the scheduler with the given mode.  If the scheduler is
   // already started, switch to the given mode, although some
-  // scheduled tasks from the old mode may still run.
-  virtual void Start(Mode mode) = 0;
+  // scheduled tasks from the old mode may still run. |last_poll_time| will
+  // be used to decide what the poll timer should be initialized with.
+  virtual void Start(Mode mode, base::Time last_poll_time) = 0;
 
   // Schedules the configuration task specified by |params|. Returns true if
   // the configuration task executed immediately, false if it had to be
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index 6f76be0..a1394fb 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -7,7 +7,6 @@
 #include <algorithm>
 #include <cstring>
 
-#include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
@@ -156,12 +155,10 @@ SyncSchedulerImpl::SyncSchedulerImpl(const std::string& name,
           TimeDelta::FromSeconds(kDefaultShortPollIntervalSeconds)),
       syncer_long_poll_interval_seconds_(
           TimeDelta::FromSeconds(kDefaultLongPollIntervalSeconds)),
-      mode_(NORMAL_MODE),
+      mode_(CONFIGURATION_MODE),
       delay_provider_(delay_provider),
       syncer_(syncer),
       session_context_(context),
-      no_scheduling_allowed_(false),
-      do_poll_after_credentials_updated_(false),
       next_sync_session_job_priority_(NORMAL_PRIORITY),
       weak_ptr_factory_(this),
       weak_ptr_factory_for_weak_handle_(this) {
@@ -207,7 +204,7 @@ void SyncSchedulerImpl::OnServerConnectionErrorFixed() {
   TryCanaryJob();
 }
 
-void SyncSchedulerImpl::Start(Mode mode) {
+void SyncSchedulerImpl::Start(Mode mode, base::Time last_poll_time) {
   DCHECK(CalledOnValidThread());
   std::string thread_name = base::MessageLoop::current()->thread_name();
   if (thread_name.empty())
@@ -223,12 +220,24 @@ void SyncSchedulerImpl::Start(Mode mode) {
   DCHECK(syncer_.get());
   Mode old_mode = mode_;
   mode_ = mode;
-  AdjustPolling(UPDATE_INTERVAL);  // Will kick start poll timer if needed.
+  // Only adjust the poll reset time if it was valid and in the past.
+  if (!last_poll_time.is_null() && last_poll_time < base::Time::Now()) {
+    // Convert from base::Time to base::TimeTicks. The reason we use Time
+    // for persisting is that TimeTicks can stop making forward progress when
+    // the machine is suspended. This implies that on resume the client might
+    // actually have miss the real poll, unless the client is restarted. Fixing
+    // that would require using an AlarmTimer though, which is only supported
+    // on certain platforms.
+    last_poll_reset_ =
+        base::TimeTicks::Now() - (base::Time::Now() - last_poll_time);
+  }
 
   if (old_mode != mode_ && mode_ == NORMAL_MODE) {
     // We just got back to normal mode.  Let's try to run the work that was
     // queued up while we were configuring.
 
+    AdjustPolling(UPDATE_INTERVAL);  // Will kick start poll timer if needed.
+
     // Update our current time before checking IsRetryRequired().
     nudge_tracker_.SetSyncCycleStartTime(base::TimeTicks::Now());
     if (nudge_tracker_.IsSyncRequired() && CanRunNudgeJobNow(NORMAL_PRIORITY)) {
@@ -306,14 +315,12 @@ void SyncSchedulerImpl::ScheduleConfiguration(
 
 bool SyncSchedulerImpl::CanRunJobNow(JobPriority priority) {
   DCHECK(CalledOnValidThread());
-  if (wait_interval_ && wait_interval_->mode == WaitInterval::THROTTLED) {
+  if (IsCurrentlyThrottled()) {
     SDVLOG(1) << "Unable to run a job because we're throttled.";
     return false;
   }
 
-  if (wait_interval_
-      && wait_interval_->mode == WaitInterval::EXPONENTIAL_BACKOFF
-      && priority != CANARY_PRIORITY) {
+  if (IsBackingOff() && priority != CANARY_PRIORITY) {
     SDVLOG(1) << "Unable to run a job because we're backing off.";
     return false;
   }
@@ -404,11 +411,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl(
     const TimeDelta& delay,
     const tracked_objects::Location& nudge_location) {
   DCHECK(CalledOnValidThread());
-
-  if (no_scheduling_allowed_) {
-    NOTREACHED() << "Illegal to schedule job while session in progress.";
-    return;
-  }
+  CHECK(!syncer_->IsSyncing());
 
   if (!started_) {
     SDVLOG_LOC(nudge_location, 2)
@@ -464,25 +467,23 @@ void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) {
   DVLOG(2) << "Will run normal mode sync cycle with types "
            << ModelTypeSetToString(session_context_->GetEnabledTypes());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
-  bool premature_exit = !syncer_->NormalSyncShare(
+  bool success = syncer_->NormalSyncShare(
       GetEnabledAndUnthrottledTypes(), &nudge_tracker_, session.get());
-  AdjustPolling(FORCE_RESET);
-  // Don't run poll job till the next time poll timer fires.
-  do_poll_after_credentials_updated_ = false;
-
-  bool success = !premature_exit
-      && !sessions::HasSyncerError(
-          session->status_controller().model_neutral_state());
 
   if (success) {
     // That cycle took care of any outstanding work we had.
     SDVLOG(2) << "Nudge succeeded.";
     nudge_tracker_.RecordSuccessfulSyncCycle();
     scheduled_nudge_time_ = base::TimeTicks();
-
-    // If we're here, then we successfully reached the server.  End all backoff.
-    wait_interval_.reset();
-    NotifyRetryTime(base::Time());
+    HandleSuccess();
+
+    // If this was a canary, we may need to restart the poll timer (the poll
+    // timer may have fired while the scheduler was in an error state, ignoring
+    // the poll).
+    if (!poll_timer_.IsRunning()) {
+      SDVLOG(1) << "Canary succeeded, restarting polling.";
+      AdjustPolling(UPDATE_INTERVAL);
+    }
   } else {
     HandleFailure(session->status_controller().model_neutral_state());
   }
@@ -505,26 +506,16 @@ void SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) {
   SDVLOG(2) << "Will run configure SyncShare with types "
             << ModelTypeSetToString(session_context_->GetEnabledTypes());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
-  bool premature_exit = !syncer_->ConfigureSyncShare(
+  bool success = syncer_->ConfigureSyncShare(
       pending_configure_params_->types_to_download,
       pending_configure_params_->source,
       session.get());
-  AdjustPolling(FORCE_RESET);
-  // Don't run poll job till the next time poll timer fires.
-  do_poll_after_credentials_updated_ = false;
-
-  bool success = !premature_exit
-      && !sessions::HasSyncerError(
-          session->status_controller().model_neutral_state());
 
   if (success) {
     SDVLOG(2) << "Configure succeeded.";
     pending_configure_params_->ready_task.Run();
     pending_configure_params_.reset();
-
-    // If we're here, then we successfully reached the server.  End all backoff.
-    wait_interval_.reset();
-    NotifyRetryTime(base::Time());
+    HandleSuccess();
   } else {
     HandleFailure(session->status_controller().model_neutral_state());
     // Sync cycle might receive response from server that causes scheduler to
@@ -536,11 +527,16 @@ void SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) {
   }
 }
 
+void SyncSchedulerImpl::HandleSuccess() {
+  // If we're here, then we successfully reached the server.  End all backoff.
+  wait_interval_.reset();
+  NotifyRetryTime(base::Time());
+}
+
 void SyncSchedulerImpl::HandleFailure(
     const sessions::ModelNeutralState& model_neutral_state) {
   if (IsCurrentlyThrottled()) {
     SDVLOG(2) << "Was throttled during previous sync cycle.";
-    RestartWaiting();
   } else if (!IsBackingOff()) {
     // Setup our backoff if this is our first such failure.
     TimeDelta length = delay_provider_->GetDelay(
@@ -549,27 +545,32 @@ void SyncSchedulerImpl::HandleFailure(
         new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF, length));
     SDVLOG(2) << "Sync cycle failed.  Will back off for "
         << wait_interval_->length.InMilliseconds() << "ms.";
-    RestartWaiting();
+  } else {
+    // Increase our backoff interval and schedule another retry.
+    TimeDelta length = delay_provider_->GetDelay(wait_interval_->length);
+    wait_interval_.reset(
+        new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF, length));
+    SDVLOG(2) << "Sync cycle failed.  Will back off for "
+        << wait_interval_->length.InMilliseconds() << "ms.";
   }
+  RestartWaiting();
 }
 
 void SyncSchedulerImpl::DoPollSyncSessionJob() {
-  base::AutoReset<bool> protector(&no_scheduling_allowed_, true);
-
   SDVLOG(2) << "Polling with types "
             << ModelTypeSetToString(GetEnabledAndUnthrottledTypes());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
-  syncer_->PollSyncShare(
+  bool success = syncer_->PollSyncShare(
       GetEnabledAndUnthrottledTypes(),
       session.get());
 
-  AdjustPolling(FORCE_RESET);
-
-  if (IsCurrentlyThrottled()) {
-    SDVLOG(2) << "Poll request got us throttled.";
-    // The OnSilencedUntil() call set up the WaitInterval for us.  All we need
-    // to do is start the timer.
-    RestartWaiting();
+  // Only restart the timer if the poll succeeded. Otherwise rely on normal
+  // failure handling to retry with backoff.
+  if (success) {
+    AdjustPolling(FORCE_RESET);
+    HandleSuccess();
+  } else {
+    HandleFailure(session->status_controller().model_neutral_state());
   }
 }
 
@@ -599,23 +600,44 @@ TimeDelta SyncSchedulerImpl::GetPollInterval() {
 
 void SyncSchedulerImpl::AdjustPolling(PollAdjustType type) {
   DCHECK(CalledOnValidThread());
+  if (!started_)
+    return;
 
-  TimeDelta poll = GetPollInterval();
-  bool rate_changed = !poll_timer_.IsRunning() ||
-                       poll != poll_timer_.GetCurrentDelay();
-
-  if (type == FORCE_RESET) {
-    last_poll_reset_ = base::TimeTicks::Now();
-    if (!rate_changed)
-      poll_timer_.Reset();
+  TimeDelta poll_interval = GetPollInterval();
+  TimeDelta poll_delay = poll_interval;
+  const TimeTicks now = TimeTicks::Now();
+
+  if (type == UPDATE_INTERVAL) {
+    if (!last_poll_reset_.is_null()) {
+      // Override the delay based on the last successful poll time (if it was
+      // set).
+      TimeTicks new_poll_time = poll_interval + last_poll_reset_;
+      poll_delay = new_poll_time - TimeTicks::Now();
+
+      if (poll_delay < TimeDelta()) {
+        // The desired poll time was in the past, so trigger a poll now (the
+        // timer will post the task asynchronously, so re-entrancy isn't an
+        // issue).
+        poll_delay = TimeDelta();
+      }
+    } else {
+      // There was no previous poll. Keep the delay set to the normal interval,
+      // as if we had just completed a poll.
+      DCHECK_EQ(GetPollInterval(), poll_delay);
+      last_poll_reset_ = now;
+    }
+  } else {
+    // Otherwise just restart the timer.
+    DCHECK_EQ(FORCE_RESET, type);
+    DCHECK_EQ(GetPollInterval(), poll_delay);
+    last_poll_reset_ = now;
   }
 
-  if (!rate_changed)
-    return;
+  SDVLOG(1) << "Updating polling delay to " << poll_delay.InMinutes()
+            << " minutes.";
 
-  // Adjust poll rate.
-  poll_timer_.Stop();
-  poll_timer_.Start(FROM_HERE, poll, this,
+  // Adjust poll rate. Start will reset the timer if it was already running.
+  poll_timer_.Start(FROM_HERE, poll_delay, this,
                     &SyncSchedulerImpl::PollTimerCallback);
 }
 
@@ -659,6 +681,7 @@ void SyncSchedulerImpl::Stop() {
 // privileges.  Everyone else should use NORMAL_PRIORITY.
 void SyncSchedulerImpl::TryCanaryJob() {
   next_sync_session_job_priority_ = CANARY_PRIORITY;
+  SDVLOG(2) << "Attempting canary job";
   TrySyncSessionJob();
 }
 
@@ -686,24 +709,16 @@ void SyncSchedulerImpl::TrySyncSessionJobImpl() {
     if (nudge_tracker_.IsSyncRequired()) {
       SDVLOG(2) << "Found pending nudge job";
       DoNudgeSyncSessionJob(priority);
-    } else if (do_poll_after_credentials_updated_ ||
-        ((base::TimeTicks::Now() - last_poll_reset_) >= GetPollInterval())) {
+    } else if (((base::TimeTicks::Now() - last_poll_reset_) >=
+                GetPollInterval())) {
+      SDVLOG(2) << "Found pending poll";
       DoPollSyncSessionJob();
-      // Poll timer fires infrequently. Usually by this time access token is
-      // already expired and poll job will fail with auth error. Set flag to
-      // retry poll once ProfileSyncService gets new access token, TryCanaryJob
-      // will be called after access token is retrieved.
-      if (HttpResponse::SYNC_AUTH_ERROR ==
-          session_context_->connection_manager()->server_status()) {
-        do_poll_after_credentials_updated_ = true;
-      }
     }
-  }
-
-  if (priority == CANARY_PRIORITY) {
-    // If this is canary job then whatever result was don't run poll job till
-    // the next time poll timer fires.
-    do_poll_after_credentials_updated_ = false;
+  } else {
+    // We must be in an error state. Transitioning out of each of these
+    // error states should trigger a canary job.
+    DCHECK(IsCurrentlyThrottled() || IsBackingOff() ||
+           session_context_->connection_manager()->HasInvalidAuthToken());
   }
 
   if (IsBackingOff() && !pending_wakeup_timer_.IsRunning()) {
@@ -712,7 +727,7 @@ void SyncSchedulerImpl::TrySyncSessionJobImpl() {
     // another retry.
     TimeDelta length = delay_provider_->GetDelay(wait_interval_->length);
     wait_interval_.reset(
-      new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF, length));
+        new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF, length));
     SDVLOG(2) << "Sync cycle failed.  Will back off for "
         << wait_interval_->length.InMilliseconds() << "ms.";
     RestartWaiting();
@@ -721,16 +736,7 @@ void SyncSchedulerImpl::TrySyncSessionJobImpl() {
 
 void SyncSchedulerImpl::PollTimerCallback() {
   DCHECK(CalledOnValidThread());
-  if (no_scheduling_allowed_) {
-    // The no_scheduling_allowed_ flag is set by a function-scoped AutoReset in
-    // functions that are called only on the sync thread.  This function is also
-    // called only on the sync thread, and only when it is posted by an expiring
-    // timer.  If we find that no_scheduling_allowed_ is set here, then
-    // something is very wrong.  Maybe someone mistakenly called us directly, or
-    // mishandled the book-keeping for no_scheduling_allowed_.
-    NOTREACHED() << "Illegal to schedule job while session in progress.";
-    return;
-  }
+  CHECK(!syncer_->IsSyncing());
 
   TrySyncSessionJob();
 }
@@ -826,6 +832,9 @@ void SyncSchedulerImpl::OnTypesThrottled(
     const base::TimeDelta& throttle_duration) {
   base::TimeTicks now = base::TimeTicks::Now();
 
+  SDVLOG(1) << "Throttling " << ModelTypeSetToString(types) << " for "
+            << throttle_duration.InMinutes() << " minutes.";
+
   nudge_tracker_.SetTypesThrottledUntil(types, throttle_duration, now);
   base::TimeDelta time_until_next_unthrottle =
       nudge_tracker_.GetTimeUntilNextUnthrottle(now);
@@ -847,13 +856,23 @@ bool SyncSchedulerImpl::IsCurrentlyThrottled() {
 void SyncSchedulerImpl::OnReceivedShortPollIntervalUpdate(
     const base::TimeDelta& new_interval) {
   DCHECK(CalledOnValidThread());
+  if (new_interval == syncer_short_poll_interval_seconds_)
+    return;
+  SDVLOG(1) << "Updating short poll interval to " << new_interval.InMinutes()
+            << " minutes.";
   syncer_short_poll_interval_seconds_ = new_interval;
+  AdjustPolling(UPDATE_INTERVAL);
 }
 
 void SyncSchedulerImpl::OnReceivedLongPollIntervalUpdate(
     const base::TimeDelta& new_interval) {
   DCHECK(CalledOnValidThread());
+  if (new_interval == syncer_long_poll_interval_seconds_)
+    return;
+  SDVLOG(1) << "Updating long poll interval to " << new_interval.InMinutes()
+            << " minutes.";
   syncer_long_poll_interval_seconds_ = new_interval;
+  AdjustPolling(UPDATE_INTERVAL);
 }
 
 void SyncSchedulerImpl::OnReceivedCustomNudgeDelays(
diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h
index e3c505b..b7d8d0b 100644
--- a/sync/engine/sync_scheduler_impl.h
+++ b/sync/engine/sync_scheduler_impl.h
@@ -51,7 +51,7 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // Calls Stop().
   ~SyncSchedulerImpl() override;
 
-  void Start(Mode mode) override;
+  void Start(Mode mode, base::Time last_poll_time) override;
   void ScheduleConfiguration(const ConfigurationParams& params) override;
   void Stop() override;
   void ScheduleLocalNudge(
@@ -151,7 +151,10 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // Invoke the syncer to perform a configuration job.
   void DoConfigurationSyncSessionJob(JobPriority priority);
 
-  // Helper function for Do{Nudge,Configuration}SyncSessionJob.
+  // Helper function for Do{Nudge,Configuration,Poll}SyncSessionJob.
+  void HandleSuccess();
+
+  // Helper function for Do{Nudge,Configuration,Poll}SyncSessionJob.
   void HandleFailure(
       const sessions::ModelNeutralState& model_neutral_state);
 
@@ -246,8 +249,10 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   base::TimeDelta syncer_short_poll_interval_seconds_;
   base::TimeDelta syncer_long_poll_interval_seconds_;
 
-  // Periodic timer for polling.  See AdjustPolling.
-  base::RepeatingTimer<SyncSchedulerImpl> poll_timer_;
+  // Timer for polling. Restarted on each successful poll, and when entering
+  // normal sync mode or exiting an error state. Not active in configuration
+  // mode.
+  base::OneShotTimer<SyncSchedulerImpl> poll_timer_;
 
   // The mode of operation.
   Mode mode_;
@@ -291,15 +296,6 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
   // could result in tight sync loops hitting sync servers.
   bool no_scheduling_allowed_;
 
-  // crbug/251307. This is a workaround for M29. crbug/259913 tracks proper fix
-  // for M30.
-  // The issue is that poll job runs after few hours of inactivity and therefore
-  // will always fail with auth error because of expired access token. Once
-  // fresh access token is requested poll job is not retried.
-  // The change is to remember that poll timer just fired and retry poll job
-  // after credentials are updated.
-  bool do_poll_after_credentials_updated_;
-
   // TryJob might get called for multiple reasons. It should only call
   // DoPollSyncSessionJob after some time since the last attempt.
   // last_poll_reset_ keeps track of when was last attempt.
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 1d43844..b6cdf6a 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -194,8 +194,12 @@ class SyncSchedulerTest : public testing::Test {
     QuitLoopNow();
   }
 
-  void StartSyncScheduler(SyncScheduler::Mode mode) {
-    scheduler()->Start(mode);
+  void StartSyncConfiguration() {
+    scheduler()->Start(SyncScheduler::CONFIGURATION_MODE, base::Time());
+  }
+
+  void StartSyncScheduler(base::Time last_poll_time) {
+    scheduler()->Start(SyncScheduler::NORMAL_MODE, last_poll_time);
   }
 
   // This stops the scheduler synchronously.
@@ -209,7 +213,7 @@ class SyncSchedulerTest : public testing::Test {
 
   bool RunAndGetBackoff() {
     ModelTypeSet nudge_types(THEMES);
-    StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+    StartSyncScheduler(base::Time());
 
     scheduler()->ScheduleLocalNudge(nudge_types, FROM_HERE);
     RunLoop();
@@ -264,21 +268,21 @@ void RecordSyncShareImpl(SyncShareTimes* times) {
   times->push_back(TimeTicks::Now());
 }
 
-ACTION_P(RecordSyncShare, times) {
+ACTION_P2(RecordSyncShare, times, success) {
   RecordSyncShareImpl(times);
   if (base::MessageLoop::current()->is_running())
     QuitLoopNow();
-  return true;
+  return success;
 }
 
-ACTION_P2(RecordSyncShareMultiple, times, quit_after) {
+ACTION_P3(RecordSyncShareMultiple, times, quit_after, success) {
   RecordSyncShareImpl(times);
   EXPECT_LE(times->size(), quit_after);
   if (times->size() >= quit_after &&
       base::MessageLoop::current()->is_running()) {
     QuitLoopNow();
   }
-  return true;
+  return success;
 }
 
 ACTION_P(StopScheduler, scheduler) {
@@ -291,9 +295,9 @@ ACTION(AddFailureAndQuitLoopNow) {
   return true;
 }
 
-ACTION(QuitLoopNowAction) {
+ACTION_P(QuitLoopNowAction, success) {
   QuitLoopNow();
-  return true;
+  return success;
 }
 
 // Test nudge scheduling.
@@ -303,10 +307,10 @@ TEST_F(SyncSchedulerTest, Nudge) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times)))
+                      RecordSyncShare(&times, true)))
       .RetiresOnSaturation();
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   scheduler()->ScheduleLocalNudge(model_types, FROM_HERE);
   RunLoop();
@@ -319,7 +323,7 @@ TEST_F(SyncSchedulerTest, Nudge) {
   model_types.Put(TYPED_URLS);
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times2)));
+                      RecordSyncShare(&times2, true)));
   scheduler()->ScheduleLocalNudge(model_types, FROM_HERE);
   RunLoop();
 }
@@ -332,9 +336,9 @@ TEST_F(SyncSchedulerTest, Config) {
 
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -358,13 +362,13 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
   SyncShareTimes times;
   const ModelTypeSet model_types(THEMES);
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      RecordSyncShare(&times)))
+                      RecordSyncShare(&times, false)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, false)));
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -389,7 +393,7 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
 
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
   RunLoop();
 
   ASSERT_EQ(1, ready_counter.times_called());
@@ -404,14 +408,14 @@ TEST_F(SyncSchedulerTest, ConfigWithStop) {
   SyncShareTimes times;
   const ModelTypeSet model_types(THEMES);
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   // Make ConfigureSyncShare call scheduler->Stop(). It is not supposed to call
   // retry_task or dereference configuration params.
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
                       StopScheduler(scheduler()),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, false)));
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -436,12 +440,12 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
       .WillRepeatedly(Return(TimeDelta::FromMilliseconds(50)));
   SyncShareTimes times;
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   // Request a configure and make sure it fails.
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, false)));
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
   ConfigurationParams params(
@@ -459,7 +463,7 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
   // Ask for a nudge while dealing with repeated configure failure.
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, false)));
   scheduler()->ScheduleLocalNudge(model_types, FROM_HERE);
   RunLoop();
   // Note that we're not RunLoop()ing for the NUDGE we just scheduled, but
@@ -471,25 +475,25 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
   // Let the next configure retry succeed.
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
   RunLoop();
 
   // Now change the mode so nudge can execute.
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times)));
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+                      RecordSyncShare(&times, true)));
+  StartSyncScheduler(base::Time());
   PumpLoop();
 }
 
 // Test that nudges are coalesced.
 TEST_F(SyncSchedulerTest, NudgeCoalescing) {
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   SyncShareTimes times;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
   const ModelTypeSet types1(THEMES), types2(TYPED_URLS), types3(THEMES);
   TimeTicks optimal_time = TimeTicks::Now() + default_delay();
   scheduler()->ScheduleLocalNudge(types1, FROM_HERE);
@@ -504,19 +508,19 @@ TEST_F(SyncSchedulerTest, NudgeCoalescing) {
   SyncShareTimes times2;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times2)));
+                      RecordSyncShare(&times2, true)));
   scheduler()->ScheduleLocalNudge(types3, FROM_HERE);
   RunLoop();
 }
 
 // Test that nudges are coalesced.
 TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   SyncShareTimes times;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
   ModelTypeSet types1(THEMES), types2(TYPED_URLS), types3;
 
   // Create a huge time delay.
@@ -542,12 +546,12 @@ TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) {
 
 // Test nudge scheduling.
 TEST_F(SyncSchedulerTest, NudgeWithStates) {
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   SyncShareTimes times1;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times1)))
+                      RecordSyncShare(&times1, true)))
       .RetiresOnSaturation();
   scheduler()->ScheduleInvalidationNudge(
       THEMES, BuildInvalidation(10, "test"), FROM_HERE);
@@ -559,7 +563,7 @@ TEST_F(SyncSchedulerTest, NudgeWithStates) {
   SyncShareTimes times2;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times2)));
+                      RecordSyncShare(&times2, true)));
   scheduler()->ScheduleInvalidationNudge(
       TYPED_URLS, BuildInvalidation(10, "test2"), FROM_HERE);
   RunLoop();
@@ -572,12 +576,61 @@ TEST_F(SyncSchedulerTest, Polling) {
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                RecordSyncShareMultiple(&times, kMinNumSamples)));
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
+
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
+
+  TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
+  StartSyncScheduler(base::Time());
+
+  // Run again to wait for polling.
+  RunLoop();
+
+  StopSyncScheduler();
+  AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval);
+}
+
+// Test that we reuse the previous poll time on startup, triggering the first
+// poll based on when the last one happened. Subsequent polls should have the
+// normal delay.
+TEST_F(SyncSchedulerTest, PollingPersistence) {
+  SyncShareTimes times;
+  // Use a large poll interval that wouldn't normally get hit on its own for
+  // some time yet.
+  TimeDelta poll_interval(TimeDelta::FromMilliseconds(500));
+  EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
+
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
+
+  // Set the start time to now, as the poll was overdue.
+  TimeTicks optimal_start = TimeTicks::Now();
+  StartSyncScheduler(base::Time::Now() - poll_interval);
+
+  // Run again to wait for polling.
+  RunLoop();
+
+  StopSyncScheduler();
+  AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval);
+}
+
+// Test that if the persisted poll is in the future, it's ignored (the case
+// where the local time has been modified).
+TEST_F(SyncSchedulerTest, PollingPersistenceBadClock) {
+  SyncShareTimes times;
+  TimeDelta poll_interval(TimeDelta::FromMilliseconds(30));
+  EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
+      .WillRepeatedly(
+          DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
 
   scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
 
+  // Set the start time to |poll_interval| in the future.
   TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time::Now() + base::TimeDelta::FromMinutes(10));
 
   // Run again to wait for polling.
   RunLoop();
@@ -593,13 +646,13 @@ TEST_F(SyncSchedulerTest, PollNotificationsDisabled) {
   EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples))
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                RecordSyncShareMultiple(&times, kMinNumSamples)));
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
 
   scheduler()->OnReceivedShortPollIntervalUpdate(poll_interval);
   scheduler()->SetNotificationsEnabled(false);
 
   TimeTicks optimal_start = TimeTicks::Now() + poll_interval;
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run again to wait for polling.
   RunLoop();
@@ -622,10 +675,10 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) {
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
                 WithArg<1>(
-                    RecordSyncShareMultiple(&times, kMinNumSamples))));
+                    RecordSyncShareMultiple(&times, kMinNumSamples, true))));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2;
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run again to wait for polling.
   RunLoop();
@@ -644,15 +697,15 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
           WithArg<2>(sessions::test_util::SimulateThrottled(throttle)),
-          Return(true)))
+          Return(false)))
       .WillRepeatedly(AddFailureAndQuitLoopNow());
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   scheduler()->ScheduleLocalNudge(types, FROM_HERE);
   PumpLoop();
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -679,15 +732,15 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromPoll) {
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(
           WithArg<1>(sessions::test_util::SimulateThrottled(throttle1)),
-          Return(true)))
+          Return(false)))
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                RecordSyncShareMultiple(&times, kMinNumSamples)));
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
 
   TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1;
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run again to wait for polling.
   RunLoop();
@@ -706,14 +759,14 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromNudge) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(
           WithArg<2>(sessions::test_util::SimulateThrottled(throttle1)),
-          Return(true)))
+          Return(false)))
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      QuitLoopNowAction()));
+                      QuitLoopNowAction(true)));
 
   const ModelTypeSet types(THEMES);
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
   scheduler()->ScheduleLocalNudge(types, FROM_HERE);
 
   PumpLoop(); // To get PerformDelayedNudge called.
@@ -735,14 +788,14 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromConfigure) {
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
           WithArg<2>(sessions::test_util::SimulateThrottled(throttle1)),
-          Return(true)))
+          Return(false)))
       .RetiresOnSaturation();
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      QuitLoopNowAction()));
+                      QuitLoopNowAction(true)));
 
   const ModelTypeSet types(THEMES);
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -780,10 +833,10 @@ TEST_F(SyncSchedulerTest, TypeThrottlingBlocksNudge) {
       .WillOnce(DoAll(
           WithArg<2>(
               sessions::test_util::SimulateTypesThrottled(types, throttle1)),
-          Return(true)))
+          Return(false)))
       .RetiresOnSaturation();
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
   scheduler()->ScheduleLocalNudge(types, FROM_HERE);
   PumpLoop(); // To get PerformDelayedNudge called.
   PumpLoop(); // To get TrySyncSessionJob called
@@ -815,10 +868,10 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) {
           WithArg<2>(
               sessions::test_util::SimulateTypesThrottled(
                   throttled_types, throttle1)),
-          Return(true)))
+          Return(false)))
       .RetiresOnSaturation();
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
   scheduler()->ScheduleLocalNudge(throttled_types, FROM_HERE);
   PumpLoop(); // To get PerformDelayedNudge called.
   PumpLoop(); // To get TrySyncSessionJob called
@@ -838,7 +891,7 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) {
   // Local nudges for non-throttled types will trigger a sync.
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                            RecordSyncShare(&times)));
+                            RecordSyncShare(&times, true)));
   scheduler()->ScheduleLocalNudge(unthrottled_types, FROM_HERE);
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
@@ -852,7 +905,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareTimes times;
   scheduler()->OnReceivedLongPollIntervalUpdate(poll);
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   const ModelTypeSet nudge_types(TYPED_URLS);
   scheduler()->ScheduleLocalNudge(nudge_types, FROM_HERE);
@@ -862,7 +915,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
 
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess),
-                      RecordSyncShare(&times)))
+                      RecordSyncShare(&times, true)))
       .RetiresOnSaturation();
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -884,12 +937,12 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
   SyncShareTimes times2;
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times2)));
+                      RecordSyncShare(&times2, true)));
 
   // TODO(tim): Figure out how to remove this dangerous need to reset
   // routing info between mode switches.
   context()->SetRoutingInfo(routing_info());
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
@@ -909,12 +962,12 @@ class BackoffTriggersSyncSchedulerTest : public SyncSchedulerTest {
   }
 };
 
-// Have the sycner fail during commit.  Expect that the scheduler enters
+// Have the syncer fail during commit.  Expect that the scheduler enters
 // backoff.
 TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnce) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      QuitLoopNowAction()));
+                      QuitLoopNowAction(false)));
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
@@ -924,9 +977,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadOnceThenSucceed) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(
           Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
-          Return(true)))
+          Return(false)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      QuitLoopNowAction()));
+                      QuitLoopNowAction(true)));
   EXPECT_FALSE(RunAndGetBackoff());
 }
 
@@ -936,9 +989,9 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailCommitOnceThenSucceed) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(
           Invoke(sessions::test_util::SimulateCommitFailed),
-          Return(true)))
+          Return(false)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      QuitLoopNowAction()));
+                      QuitLoopNowAction(true)));
   EXPECT_FALSE(RunAndGetBackoff());
 }
 
@@ -948,10 +1001,10 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadTwice) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(
           Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
-          Return(true)))
+          Return(false)))
       .WillRepeatedly(DoAll(
-              Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
-              QuitLoopNowAction()));
+          Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
+          QuitLoopNowAction(false)));
   EXPECT_TRUE(RunAndGetBackoff());
 }
 
@@ -961,11 +1014,11 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailGetEncryptionKey) {
   EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_))
       .WillOnce(DoAll(
           Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
-          Return(true)))
+          Return(false)))
       .WillRepeatedly(DoAll(
-              Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
-              QuitLoopNowAction()));
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+          Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed),
+          QuitLoopNowAction(false)));
+  StartSyncConfiguration();
 
   ModelTypeSet types(THEMES);
   CallbackCounter ready_counter;
@@ -992,11 +1045,11 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      RecordSyncShareMultiple(&times, 1U)));
+                      RecordSyncShareMultiple(&times, 1U, false)));
   EXPECT_CALL(*delay(), GetDelay(_)).
       WillRepeatedly(Return(TimeDelta::FromDays(1)));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // This nudge should fail and put us into backoff.  Thanks to our mock
   // GetDelay() setup above, this will be a long backoff.
@@ -1017,7 +1070,7 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
 
   EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
 
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
 
   CallbackCounter ready_counter;
   CallbackCounter retry_counter;
@@ -1041,7 +1094,7 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)).Times(kMinNumSamples)
       .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-          RecordSyncShareMultiple(&times, kMinNumSamples)));
+          RecordSyncShareMultiple(&times, kMinNumSamples, false)));
 
   const TimeDelta first = TimeDelta::FromSeconds(kInitialBackoffRetrySeconds);
   const TimeDelta second = TimeDelta::FromMilliseconds(20);
@@ -1060,7 +1113,7 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
           .RetiresOnSaturation();
   EXPECT_CALL(*delay(), GetDelay(fifth)).WillOnce(Return(sixth));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run again with a nudge.
   scheduler()->ScheduleLocalNudge(ModelTypeSet(THEMES), FROM_HERE);
@@ -1076,8 +1129,6 @@ TEST_F(SyncSchedulerTest, BackoffElevation) {
 // Test that things go back to normal once a retry makes forward progress.
 TEST_F(SyncSchedulerTest, BackoffRelief) {
   SyncShareTimes times;
-  const TimeDelta poll(TimeDelta::FromMilliseconds(10));
-  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
   UseMockDelayProvider();
 
   const TimeDelta backoff = TimeDelta::FromMilliseconds(10);
@@ -1085,12 +1136,12 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
 
   // Optimal start for the post-backoff poll party.
   TimeTicks optimal_start = TimeTicks::Now();
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Kick off the test with a failed nudge.
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, false)));
   scheduler()->ScheduleLocalNudge(ModelTypeSet(THEMES), FROM_HERE);
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
@@ -1102,7 +1153,7 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(
               Invoke(sessions::test_util::SimulateNormalSuccess),
-              RecordSyncShare(&times)));
+              RecordSyncShare(&times, true)));
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
   optimal_job_time = optimal_job_time + backoff;
@@ -1113,38 +1164,46 @@ TEST_F(SyncSchedulerTest, BackoffRelief) {
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulatePollSuccess),
-              RecordSyncShareMultiple(&times, kMinNumSamples)));
+              RecordSyncShareMultiple(&times, kMinNumSamples, true)));
+  const TimeDelta poll(TimeDelta::FromMilliseconds(10));
+  scheduler()->OnReceivedLongPollIntervalUpdate(poll);
+
+  // The new optimal time is now, since the desired poll should have happened
+  // in the past.
+  optimal_job_time = base::TimeTicks::Now();
   RunLoop();
   Mock::VerifyAndClearExpectations(syncer());
   ASSERT_EQ(kMinNumSamples, times.size());
   for (size_t i = 2; i < times.size(); i++) {
-    optimal_job_time = optimal_job_time + poll;
     SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")");
     EXPECT_GE(times[i], optimal_job_time);
+    optimal_job_time = optimal_job_time + poll;
   }
 
   StopSyncScheduler();
 }
 
-// Test that poll failures are ignored.  They should have no effect on
-// subsequent poll attempts, nor should they trigger a backoff/retry.
+// Test that poll failures are treated like any other failure. They should
+// result in retry with backoff.
 TEST_F(SyncSchedulerTest, TransientPollFailure) {
   SyncShareTimes times;
   const TimeDelta poll_interval(TimeDelta::FromMilliseconds(10));
   scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval);
   UseMockDelayProvider(); // Will cause test failure if backoff is initiated.
+  EXPECT_CALL(*delay(), GetDelay(_))
+      .WillRepeatedly(Return(TimeDelta::FromMilliseconds(0)));
 
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollFailed),
-                      RecordSyncShare(&times)))
+                      RecordSyncShare(&times, false)))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run the unsucessful poll. The failed poll should not trigger backoff.
   RunLoop();
-  EXPECT_FALSE(scheduler()->IsBackingOff());
+  EXPECT_TRUE(scheduler()->IsBackingOff());
 
   // Run the successful poll.
   RunLoop();
@@ -1158,10 +1217,10 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   connection()->UpdateConnectionStatus();
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure),
-                    Return(true)))
+                    Return(false)))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
                     Return(true)));
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   scheduler()->ScheduleLocalNudge(ModelTypeSet(THEMES), FROM_HERE);
   // Should save the nudge for until after the server is reachable.
@@ -1178,13 +1237,13 @@ TEST_F(SyncSchedulerTest, ServerConnectionChangeDuringBackoff) {
   EXPECT_CALL(*delay(), GetDelay(_))
       .WillRepeatedly(Return(TimeDelta::FromMilliseconds(0)));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
   connection()->SetServerNotReachable();
   connection()->UpdateConnectionStatus();
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure),
-                    Return(true)))
+                    Return(false)))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
                     Return(true)));
 
@@ -1208,17 +1267,17 @@ TEST_F(SyncSchedulerTest, ConnectionChangeCanaryPreemptedByNudge) {
   EXPECT_CALL(*delay(), GetDelay(_))
       .WillRepeatedly(Return(TimeDelta::FromMilliseconds(0)));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
   connection()->SetServerNotReachable();
   connection()->UpdateConnectionStatus();
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure),
-                    Return(true)))
+                    Return(false)))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
                     Return(true)))
     .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                    QuitLoopNowAction()));
+                    QuitLoopNowAction(true)));
 
   scheduler()->ScheduleLocalNudge(ModelTypeSet(THEMES), FROM_HERE);
 
@@ -1242,7 +1301,7 @@ TEST_F(SyncSchedulerTest, DoubleCanaryInConfigure) {
       .WillRepeatedly(DoAll(
               Invoke(sessions::test_util::SimulateConfigureConnectionFailure),
               Return(true)));
-  StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+  StartSyncConfiguration();
   connection()->SetServerNotReachable();
   connection()->UpdateConnectionStatus();
 
@@ -1272,10 +1331,10 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillRepeatedly(
           DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                RecordSyncShareMultiple(&times, kMinNumSamples)));
+                RecordSyncShareMultiple(&times, kMinNumSamples, true)));
 
   connection()->SetServerStatus(HttpResponse::SYNC_AUTH_ERROR);
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   // Run to wait for polling.
   RunLoop();
@@ -1285,7 +1344,7 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
   // poll once more
   EXPECT_CALL(*syncer(), PollSyncShare(_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
   scheduler()->OnCredentialsUpdated();
   connection()->SetServerStatus(HttpResponse::SERVER_CONNECTION_OK);
   RunLoop();
@@ -1293,7 +1352,7 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) {
 }
 
 TEST_F(SyncSchedulerTest, SuccessfulRetry) {
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   SyncShareTimes times;
   base::TimeDelta delay = base::TimeDelta::FromMilliseconds(10);
@@ -1303,7 +1362,7 @@ TEST_F(SyncSchedulerTest, SuccessfulRetry) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(
           DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                RecordSyncShare(&times)));
+                RecordSyncShare(&times, true)));
 
   // Run to wait for retrying.
   RunLoop();
@@ -1312,11 +1371,13 @@ TEST_F(SyncSchedulerTest, SuccessfulRetry) {
 }
 
 TEST_F(SyncSchedulerTest, FailedRetry) {
+  SyncShareTimes times;
+
   UseMockDelayProvider();
   EXPECT_CALL(*delay(), GetDelay(_))
       .WillRepeatedly(Return(TimeDelta::FromMilliseconds(10)));
 
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   base::TimeDelta delay = base::TimeDelta::FromMilliseconds(10);
   scheduler()->OnReceivedGuRetryDelay(delay);
@@ -1324,7 +1385,7 @@ TEST_F(SyncSchedulerTest, FailedRetry) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(
           DoAll(Invoke(sessions::test_util::SimulateDownloadUpdatesFailed),
-                QuitLoopNowAction()));
+                RecordSyncShare(&times, false)));
 
   // Run to wait for retrying.
   RunLoop();
@@ -1333,7 +1394,7 @@ TEST_F(SyncSchedulerTest, FailedRetry) {
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(
           DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                QuitLoopNowAction()));
+                RecordSyncShare(&times, true)));
 
   // Run to wait for second retrying.
   RunLoop();
@@ -1346,7 +1407,7 @@ ACTION_P2(VerifyRetryTimerDelay, scheduler_test, expected_delay) {
 }
 
 TEST_F(SyncSchedulerTest, ReceiveNewRetryDelay) {
-  StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+  StartSyncScheduler(base::Time());
 
   SyncShareTimes times;
   base::TimeDelta delay1 = base::TimeDelta::FromMilliseconds(100);
@@ -1360,7 +1421,7 @@ TEST_F(SyncSchedulerTest, ReceiveNewRetryDelay) {
       .WillOnce(DoAll(
           WithoutArgs(VerifyRetryTimerDelay(this, delay1)),
           WithArg<2>(sessions::test_util::SimulateGuRetryDelayCommand(delay2)),
-          RecordSyncShare(&times)));
+          RecordSyncShare(&times, true)));
 
   // Run nudge GU.
   RunLoop();
@@ -1368,7 +1429,7 @@ TEST_F(SyncSchedulerTest, ReceiveNewRetryDelay) {
 
   EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_))
       .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess),
-                      RecordSyncShare(&times)));
+                      RecordSyncShare(&times, true)));
 
   // Run to wait for retrying.
   RunLoop();
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index ac46881..0b6bed2 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -4,6 +4,7 @@
 
 #include "sync/engine/syncer.h"
 
+#include "base/auto_reset.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ -45,7 +46,8 @@ using sessions::SyncSession;
 using sessions::NudgeTracker;
 
 Syncer::Syncer(syncer::CancelationSignal* cancelation_signal)
-    : cancelation_signal_(cancelation_signal) {
+    : cancelation_signal_(cancelation_signal),
+      is_syncing_(false) {
 }
 
 Syncer::~Syncer() {}
@@ -54,9 +56,14 @@ bool Syncer::ExitRequested() {
   return cancelation_signal_->IsSignalled();
 }
 
+bool Syncer::IsSyncing() const {
+  return is_syncing_;
+}
+
 bool Syncer::NormalSyncShare(ModelTypeSet request_types,
                              NudgeTracker* nudge_tracker,
                              SyncSession* session) {
+  base::AutoReset<bool> is_syncing(&is_syncing_, true);
   HandleCycleBegin(session);
   if (nudge_tracker->IsGetUpdatesRequired() ||
       session->context()->ShouldFetchUpdatesBeforeCommit()) {
@@ -88,6 +95,7 @@ bool Syncer::ConfigureSyncShare(
     ModelTypeSet request_types,
     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source,
     SyncSession* session) {
+  base::AutoReset<bool> is_syncing(&is_syncing_, true);
   VLOG(1) << "Configuring types " << ModelTypeSetToString(request_types);
   HandleCycleBegin(session);
   ConfigureGetUpdatesDelegate configure_delegate(source);
@@ -104,6 +112,7 @@ bool Syncer::ConfigureSyncShare(
 
 bool Syncer::PollSyncShare(ModelTypeSet request_types,
                            SyncSession* session) {
+  base::AutoReset<bool> is_syncing(&is_syncing_, true);
   VLOG(1) << "Polling types " << ModelTypeSetToString(request_types);
   HandleCycleBegin(session);
   PollGetUpdatesDelegate poll_delegate;
@@ -202,7 +211,12 @@ bool Syncer::HandleCycleEnd(
     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source) {
   if (!ExitRequested()) {
     session->SendSyncCycleEndEventNotification(source);
-    return true;
+
+    bool success = !sessions::HasSyncerError(
+          session->status_controller().model_neutral_state());
+    if (success && source == sync_pb::GetUpdatesCallerInfo::PERIODIC)
+      session->mutable_status_controller()->UpdatePollTime();
+    return success;
   } else {
     return false;
   }
diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h
index ef74877..e473bae 100644
--- a/sync/engine/syncer.h
+++ b/sync/engine/syncer.h
@@ -41,12 +41,18 @@ class SYNC_EXPORT_PRIVATE Syncer {
   Syncer(CancelationSignal* cancelation_signal);
   virtual ~Syncer();
 
+  // Whether an early exist was requested due to a cancelation signal.
   bool ExitRequested();
 
+  // Whether the syncer is in the middle of a sync cycle.
+  bool IsSyncing() const;
+
   // Fetches and applies updates, resolves conflicts and commits local changes
   // for |request_types| as necessary until client and server states are in
   // sync.  The |nudge_tracker| contains state that describes why the client is
   // out of sync and what must be done to bring it back into sync.
+  // Returns: false if an error occurred and retries should backoff, true
+  // otherwise.
   virtual bool NormalSyncShare(ModelTypeSet request_types,
                                sessions::NudgeTracker* nudge_tracker,
                                sessions::SyncSession* session);
@@ -56,6 +62,8 @@ class SYNC_EXPORT_PRIVATE Syncer {
   // processors are in "passive" mode, so none of the downloaded updates will be
   // applied to the model.  The |source| is sent up to the server for debug
   // purposes.  It describes the reson for performing this initial download.
+  // Returns: false if an error occurred and retries should backoff, true
+  // otherwise.
   virtual bool ConfigureSyncShare(
       ModelTypeSet request_types,
       sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source,
@@ -65,10 +73,34 @@ class SYNC_EXPORT_PRIVATE Syncer {
   // client with a working connection to the invalidations server, this should
   // be unnecessary.  It may be invoked periodically to try to keep the client
   // in sync despite bugs or transient failures.
+  // Returns: false if an error occurred and retries should backoff, true
+  // otherwise.
   virtual bool PollSyncShare(ModelTypeSet request_types,
                              sessions::SyncSession* session);
 
  private:
+  friend class SyncerTest;
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, NameClashWithResolver);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, IllegalAndLegalUpdates);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingAndNewParent);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest,
+                           TestCommitListOrderingAndNewParentAndChild);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingCounterexample);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingWithNesting);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingWithNewItems);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestGetUnsyncedAndSimpleCommit);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestPurgeWhileUnsynced);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestPurgeWhileUnapplied);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, UnappliedUpdateDuringCommit);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, DeletingEntryInFolder);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest,
+                           LongChangelistCreatesFakeOrphanedEntries);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, QuicklyMergeDualCreatedHierarchy);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, LongChangelistWithApplicationConflict);
+  FRIEND_TEST_ALL_PREFIXES(SyncerTest, DeletingEntryWithLocalEdits);
+  FRIEND_TEST_ALL_PREFIXES(EntryCreatedInNewFolderTest,
+                           EntryCreatedInNewFolderMidSync);
+
   bool DownloadAndApplyUpdates(
       ModelTypeSet* request_types,
       sessions::SyncSession* session,
@@ -91,27 +123,8 @@ class SYNC_EXPORT_PRIVATE Syncer {
 
   syncer::CancelationSignal* const cancelation_signal_;
 
-  friend class SyncerTest;
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, NameClashWithResolver);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, IllegalAndLegalUpdates);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingAndNewParent);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest,
-                           TestCommitListOrderingAndNewParentAndChild);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingCounterexample);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingWithNesting);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestCommitListOrderingWithNewItems);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestGetUnsyncedAndSimpleCommit);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestPurgeWhileUnsynced);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, TestPurgeWhileUnapplied);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, UnappliedUpdateDuringCommit);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, DeletingEntryInFolder);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest,
-                           LongChangelistCreatesFakeOrphanedEntries);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, QuicklyMergeDualCreatedHierarchy);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, LongChangelistWithApplicationConflict);
-  FRIEND_TEST_ALL_PREFIXES(SyncerTest, DeletingEntryWithLocalEdits);
-  FRIEND_TEST_ALL_PREFIXES(EntryCreatedInNewFolderTest,
-                           EntryCreatedInNewFolderMidSync);
+  // Whether the syncer is in the middle of a sync attempt.
+  bool is_syncing_;
 
   DISALLOW_COPY_AND_ASSIGN(Syncer);
 };
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 3257c4b..a3375e3 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -256,22 +256,22 @@ class SyncerTest : public testing::Test,
     session_.reset(SyncSession::Build(context_.get(), this));
   }
 
-  void SyncShareNudge() {
+  bool SyncShareNudge() {
     ResetSession();
 
     // Pretend we've seen a local change, to make the nudge_tracker look normal.
     nudge_tracker_.RecordLocalChange(ModelTypeSet(BOOKMARKS));
 
-    EXPECT_TRUE(syncer_->NormalSyncShare(context_->GetEnabledTypes(),
-                                         &nudge_tracker_, session_.get()));
+    return syncer_->NormalSyncShare(context_->GetEnabledTypes(),
+                                         &nudge_tracker_, session_.get());
   }
 
-  void SyncShareConfigure() {
+  bool SyncShareConfigure() {
     ResetSession();
-    EXPECT_TRUE(syncer_->ConfigureSyncShare(
+    return syncer_->ConfigureSyncShare(
             context_->GetEnabledTypes(),
             sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
-            session_.get()));
+            session_.get());
   }
 
   void SetUp() override {
@@ -469,7 +469,7 @@ class SyncerTest : public testing::Test,
         test++;
       }
     }
-    SyncShareNudge();
+    EXPECT_TRUE(SyncShareNudge());
     ASSERT_TRUE(expected_positions.size() ==
                 mock_server_->committed_ids().size());
     // If this test starts failing, be aware other sort orders could be valid.
@@ -628,7 +628,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
 
   mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -657,7 +657,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
 
   // Sync again with bookmarks enabled.
   mock_server_->ExpectGetUpdatesRequestTypes(context_->GetEnabledTypes());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // It should have been committed.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
@@ -701,7 +701,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
                                    foreign_cache_guid(), "-3");
   mock_server_->AddUpdateDirectory(4, 0, "D", 10, 10,
                                    foreign_cache_guid(), "-4");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Server side change will put A in conflict.
   mock_server_->AddUpdateDirectory(1, 0, "A", 20, 20,
                                    foreign_cache_guid(), "-1");
@@ -745,7 +745,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     D.PutSpecifics(encrypted_bookmark);
     D.PutNonUniqueName("not encrypted");
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Nothing should have commited due to bookmarks being encrypted and
     // the cryptographer having pending keys. A would have been resolved
@@ -759,7 +759,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     // Resolve the pending keys.
     GetCryptographer(&rtrans)->DecryptPendingKeys(other_params);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // All properly encrypted and non-conflicting items should commit. "A" was
     // conflicting, but last sync cycle resolved it as simple conflict, so on
@@ -786,7 +786,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
     D.PutSpecifics(encrypted_bookmark);
     D.PutNonUniqueName(kEncryptedString);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     const StatusController& status_controller = session_->status_controller();
     // Expect success.
@@ -816,7 +816,7 @@ TEST_F(SyncerTest, GetUpdatesPartialThrottled) {
                                    foreign_cache_guid(), "-3");
   mock_server_->AddUpdateSpecifics(4, 0, "D", 10, 10, false, 0, pref);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Initial state. Everything is normal.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
@@ -850,7 +850,7 @@ TEST_F(SyncerTest, GetUpdatesPartialThrottled) {
     C.PutIsUnsynced(true);
     D.PutIsUnsynced(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // BOOKMARKS throttled.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
@@ -870,7 +870,7 @@ TEST_F(SyncerTest, GetUpdatesPartialThrottled) {
   mock_server_->AddUpdateSpecifics(3, 1, "G", 30, 30, false, 1, bookmark,
                                    foreign_cache_guid(), "-3");
   mock_server_->AddUpdateSpecifics(4, 0, "H", 30, 30, false, 0, pref);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // BOOKMARKS unthrottled.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
@@ -1025,7 +1025,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   mock_server_->AddUpdateSpecifics(3, 1, "C", 10, 10, false, 1, bookmark,
                                    foreign_cache_guid(), "-3");
   mock_server_->AddUpdateSpecifics(4, 0, "D", 10, 10, false, 0, pref);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Initial state. Everything is normal.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
@@ -1058,7 +1058,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 20, 20, false, 0,
                                    encrypted_pref,
                                    foreign_cache_guid(), "-4");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // All should be unapplied due to being undecryptable and have a valid
     // BASE_SERVER_SPECIFICS.
@@ -1081,7 +1081,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   mock_server_->AddUpdateSpecifics(4, 0, kEncryptedString, 30, 30, false, 0,
                                    encrypted_pref,
                                    foreign_cache_guid(), "-4");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Items 1, 2, and 4 should have newer server versions, 3 remains the same.
     // All should remain unapplied due to be undecryptable.
@@ -1101,7 +1101,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   mock_server_->AddUpdateSpecifics(3, 1, kEncryptedString, 30, 30, false, 3,
                                    encrypted_bookmark,
                                    foreign_cache_guid(), "-3");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Items 2 and 4 should be the only ones with BASE_SERVER_SPECIFICS set.
     // Items 1 is now unencrypted, so should have applied normally.
@@ -1136,7 +1136,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     D.PutNonUniqueName(kEncryptedString);
     D.PutIsUnsynced(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // Item 1 remains unsynced due to there being pending keys.
     // Items 2, 3, 4 should remain unsynced since they were not up to date.
@@ -1153,7 +1153,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
     GetCryptographer(&rtrans)->DecryptPendingKeys(key_params);
   }
   // First cycle resolves conflicts, second cycle commits changes.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_server_overwrites);
   EXPECT_EQ(1, GetUpdateCounters(PREFERENCES).num_server_overwrites);
   EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_local_overwrites);
@@ -1164,7 +1164,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   EXPECT_EQ(1, GetCommitCounters(PREFERENCES).num_commits_attempted);
   EXPECT_EQ(1, GetCommitCounters(PREFERENCES).num_commits_success);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Everything should be resolved now. The local changes should have
   // overwritten the server changes for 2 and 4, while the server changes
@@ -1203,7 +1203,7 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
     WriteTestDataToEntry(&wtrans, &child);
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(2u, mock_server_->committed_ids().size());
   // If this test starts failing, be aware other sort orders could be valid.
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1251,7 +1251,7 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
                                       ModelTypeSet(),
                                       ModelTypeSet());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(2U, mock_server_->committed_ids().size());
   // If this test starts failing, be aware other sort orders could be valid.
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1289,7 +1289,7 @@ TEST_F(SyncerTest, TestPurgeWhileUnapplied) {
                                       ModelTypeSet(),
                                       ModelTypeSet());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   directory()->SaveChanges();
   {
     syncable::ReadTransaction rt(FROM_HERE, directory());
@@ -1348,7 +1348,7 @@ TEST_F(SyncerTest, ResetVersions) {
   mock_server_->AddUpdatePref("id1", "", "tag1", 20, 20);
   mock_server_->AddUpdatePref("id2", "", "tag2", 30, 30);
   mock_server_->AddUpdatePref("id3", "", "tag3", 40, 40);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     // Modify one of the preferences locally, mark another one as unapplied,
@@ -1603,7 +1603,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
     }
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(6u, mock_server_->committed_ids().size());
   // This test will NOT unroll deletes because SERVER_PARENT_ID is not set.
   // It will treat these like moves.
@@ -1673,7 +1673,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
     child.PutBaseVersion(1);
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(6u, mock_server_->committed_ids().size());
 
   // This strange iteration and std::count() usage is to allow the order to
@@ -1727,7 +1727,7 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
     child2.PutBaseVersion(1);
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
   // There are two possible valid orderings.
@@ -1779,7 +1779,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
     child.PutBaseVersion(1);
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
   // If this test starts failing, be aware other sort orders could be valid.
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1850,7 +1850,7 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
     meta_handle_b = child.GetMetahandle();
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ASSERT_EQ(3u, mock_server_->committed_ids().size());
   // If this test starts failing, be aware other sort orders could be valid.
   EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1888,12 +1888,12 @@ TEST_F(SyncerTest, UpdateWithZeroLengthName) {
   // And one legal one that we're going to delete.
   mock_server_->AddUpdateDirectory(2, 0, "FOO", 1, 10,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Delete the legal one. The new update has a null name.
   mock_server_->AddUpdateDirectory(
       2, 0, std::string(), 2, 20, foreign_cache_guid(), "-2");
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 }
 
 TEST_F(SyncerTest, TestBasicUpdate) {
@@ -1905,7 +1905,7 @@ TEST_F(SyncerTest, TestBasicUpdate) {
   mock_server_->AddUpdateDirectory(id, parent_id, name, version, timestamp,
                                    foreign_cache_guid(), "-1");
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     Entry entry(&trans, GET_BY_ID,
@@ -1939,7 +1939,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   mock_server_->AddUpdateDirectory(3, -80, "bad_parent", 10, 10,
                                    foreign_cache_guid(), "-3");
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Id 3 should be in conflict now.
   EXPECT_EQ(
@@ -1965,7 +1965,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   mock_server_->AddUpdateDirectory(10, 0, "dir_to_bookmark", 10, 10,
                                    foreign_cache_guid(), "-10");
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // The three items with an unresolved parent should be unapplied (3, 9, 100).
   // The name clash should also still be in conflict.
   EXPECT_EQ(
@@ -2011,12 +2011,12 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   // Flip the is_dir bit: should fail verify & be dropped.
   mock_server_->AddUpdateBookmark(10, 0, "dir_to_bookmark", 20, 20,
                                   foreign_cache_guid(), "-10");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Version number older than last known: should fail verify & be dropped.
   mock_server_->AddUpdateDirectory(4, 0, "old_version", 10, 10,
                                    foreign_cache_guid(), "-4");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
 
@@ -2137,7 +2137,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateAdjustsChildren) {
   mock_server_->set_conflict_all_commits(true);
 
   // Alright! Apply that update!
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   {
     // The folder's ID should have been updated.
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -2202,7 +2202,7 @@ TEST_F(SyncerTest, CommitReuniteUpdate) {
   mock_server_->set_conflict_all_commits(true);
 
   // Alright! Apply that update!
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_HANDLE, entry_metahandle);
@@ -2263,7 +2263,7 @@ TEST_F(SyncerTest, CommitReuniteUpdateDoesNotChokeOnDeletedLocalEntry) {
   }
 
   // Just don't CHECK fail in sync, have the update split.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Id new_entry_id = GetOnlyEntryWithName(
@@ -2285,7 +2285,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesUnsanitizedNames) {
   mock_server_->AddUpdateDirectory(2, 0, "B/B", 10, 10,
                                    foreign_cache_guid(), "-2");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
 
@@ -2300,7 +2300,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesUnsanitizedNames) {
     B.PutIsUnappliedUpdate(true);
     B.PutServerVersion(20);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   saw_syncer_event_ = false;
   mock_server_->set_conflict_all_commits(false);
 
@@ -2327,7 +2327,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesNormalNames) {
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10,
                                    foreign_cache_guid(), "-2");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
 
@@ -2342,7 +2342,7 @@ TEST_F(SyncerTest, ConflictMatchingEntryHandlesNormalNames) {
     B.PutIsUnappliedUpdate(true);
     B.PutServerVersion(20);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   saw_syncer_event_ = false;
   mock_server_->set_conflict_all_commits(false);
 
@@ -2374,7 +2374,7 @@ TEST_F(SyncerTest, ReverseFolderOrderingTest) {
                                    foreign_cache_guid(), "-2");
   mock_server_->AddUpdateDirectory(1, 0, "parent", 10, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   syncable::ReadTransaction trans(FROM_HERE, directory());
 
   Id child_id = GetOnlyEntryWithName(
@@ -2416,7 +2416,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
   mock_server_->SetMidCommitCallback(
       base::Bind(&EntryCreatedInNewFolderTest::CreateFolderInBob,
                  base::Unretained(this)));
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // We loop until no unsynced handles remain, so we will commit both ids.
   EXPECT_EQ(2u, mock_server_->committed_ids().size());
   {
@@ -2436,7 +2436,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
 TEST_F(SyncerTest, NegativeIDInUpdate) {
   mock_server_->AddUpdateBookmark(-10, 0, "bad", 40, 40,
                                   foreign_cache_guid(), "-100");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // The negative id would make us CHECK!
 }
 
@@ -2454,7 +2454,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
     WriteTestDataToEntry(&trans, &fred_match);
   }
   // Commit it.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1u, mock_server_->committed_ids().size());
   mock_server_->set_conflict_all_commits(true);
   syncable::Id fred_match_id;
@@ -2470,7 +2470,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) {
   }
   // Run the syncer.
   for (int i = 0 ; i < 30 ; ++i) {
-    SyncShareNudge();
+    EXPECT_TRUE(SyncShareNudge());
   }
 }
 
@@ -2500,7 +2500,7 @@ TEST_F(SyncerTest, DoublyChangedWithResolver) {
   mock_server_->AddUpdateBookmark(child_id_, parent_id_, "Pete2.htm", 11, 10,
                                   local_cache_guid(), local_id.GetServerId());
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   syncable::Directory::Metahandles children;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -2538,7 +2538,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
     entry.PutMtime(test_time);
     entry_metahandle = entry.GetMetahandle();
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   syncable::Id id;
   int64 version;
   {
@@ -2556,7 +2556,7 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
   EXPECT_EQ(id.GetServerId(), update->id_string());
   EXPECT_EQ(root_id_.GetServerId(), update->parent_id_string());
   EXPECT_EQ(version, update->version());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, syncable::GET_BY_ID, id);
@@ -2604,9 +2604,9 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
                                   local_cache_guid(),
                                   child_local_id.GetServerId());
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
-  SyncShareNudge();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
+  EXPECT_TRUE(SyncShareNudge());
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Directory::Metahandles children;
@@ -2631,7 +2631,7 @@ TEST_F(SyncerTest, CommittingNewDeleted) {
     entry.PutIsUnsynced(true);
     entry.PutIsDel(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0u, mock_server_->committed_ids().size());
 }
 
@@ -2659,7 +2659,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) {
     entry.PutServerSpecifics(DefaultBookmarkSpecifics());
     entry.PutIsDel(false);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, session_->status_controller().TotalNumConflictingItems());
   saw_syncer_event_ = false;
 }
@@ -2686,7 +2686,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
     entry.PutIsUnsynced(true);
     existing_metahandle = entry.GetMetahandle();
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry newfolder(&trans, CREATE, BOOKMARKS, trans.root_id(), "new");
@@ -2704,7 +2704,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
     newfolder.PutIsDel(true);
     existing.PutIsDel(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, GetCommitCounters(BOOKMARKS).num_commits_conflict);
 }
 
@@ -2713,7 +2713,7 @@ TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
 
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry newfolder(
@@ -2740,12 +2740,12 @@ TEST_F(SyncerTest, FolderSwapUpdate) {
                                    foreign_cache_guid(), "-7801");
   mock_server_->AddUpdateDirectory(1024, 0, "fred", 1, 10,
                                    foreign_cache_guid(), "-1024");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   mock_server_->AddUpdateDirectory(1024, 0, "bob", 2, 20,
                                    foreign_cache_guid(), "-1024");
   mock_server_->AddUpdateDirectory(7801, 0, "fred", 2, 20,
                                    foreign_cache_guid(), "-7801");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2767,7 +2767,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) {
                                    foreign_cache_guid(), "-1024");
   mock_server_->AddUpdateDirectory(4096, 0, "alice", 1, 10,
                                    foreign_cache_guid(), "-4096");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2789,7 +2789,7 @@ TEST_F(SyncerTest, NameCollidingFolderSwapWorksFine) {
                                    foreign_cache_guid(), "-7801");
   mock_server_->AddUpdateDirectory(4096, 0, "bob", 2, 20,
                                    foreign_cache_guid(), "-4096");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry id1(&trans, GET_BY_ID, ids_.FromNumber(7801));
@@ -2827,7 +2827,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_Success) {
   }
   ASSERT_EQ(items_to_commit, directory()->unsynced_entity_count());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(num_batches, mock_server_->commit_messages().size());
   EXPECT_EQ(0, directory()->unsynced_entity_count());
 }
@@ -2853,7 +2853,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_PostBufferFail) {
   // The second commit should fail.  It will be preceded by one successful
   // GetUpdate and one succesful commit.
   mock_server_->FailNthPostBufferToPathCall(3);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   EXPECT_EQ(1U, mock_server_->commit_messages().size());
   EXPECT_EQ(SYNC_SERVER_ERROR,
@@ -2882,7 +2882,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_CommitConflict) {
 
   // Return a CONFLICT response for the first item.
   mock_server_->set_conflict_n_commits(1);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   // We should stop looping at the first sign of trouble.
   EXPECT_EQ(1U, mock_server_->commit_messages().size());
@@ -2895,14 +2895,14 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_HappyCase) {
   debug_info_getter_->AddDebugEvent();
   debug_info_getter_->AddDebugEvent();
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Verify we received one GetUpdates request with two debug info events.
   EXPECT_EQ(1U, mock_server_->requests().size());
   ASSERT_TRUE(mock_server_->last_request().has_get_updates());
   EXPECT_EQ(2, mock_server_->last_request().debug_info().events_size());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // See that we received another GetUpdates request, but that it contains no
   // debug info events.
@@ -2912,7 +2912,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_HappyCase) {
 
   debug_info_getter_->AddDebugEvent();
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // See that we received another GetUpdates request and it contains one debug
   // info event.
@@ -2927,7 +2927,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_PostFailsDontDrop) {
   debug_info_getter_->AddDebugEvent();
 
   mock_server_->FailNextPostBufferToPathCall();
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   // Verify we attempted to send one GetUpdates request with two debug info
   // events.
@@ -2935,7 +2935,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_PostFailsDontDrop) {
   ASSERT_TRUE(mock_server_->last_request().has_get_updates());
   EXPECT_EQ(2, mock_server_->last_request().debug_info().events_size());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // See that the client resent the two debug info events.
   EXPECT_EQ(2U, mock_server_->requests().size());
@@ -2944,25 +2944,25 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_PostFailsDontDrop) {
 
   // The previous send was successful so this next one shouldn't generate any
   // debug info events.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(3U, mock_server_->requests().size());
   ASSERT_TRUE(mock_server_->last_request().has_get_updates());
   EXPECT_EQ(0, mock_server_->last_request().debug_info().events_size());
 }
 
 // Tests that commit failure with conflict will trigger GetUpdates for next
-// sycle of sync
+// cycle of sync
 TEST_F(SyncerTest, CommitFailureWithConflict) {
   ConfigureNoGetUpdatesRequired();
   CreateUnsyncedDirectory("X", "id_X");
   EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
 
   CreateUnsyncedDirectory("Y", "id_Y");
   mock_server_->set_conflict_n_commits(1);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   EXPECT_TRUE(nudge_tracker_.IsGetUpdatesRequired());
 
   nudge_tracker_.RecordSuccessfulSyncCycle();
@@ -2978,7 +2978,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_HappyCase) {
   // Generate a debug info event and trigger a commit.
   debug_info_getter_->AddDebugEvent();
   CreateUnsyncedDirectory("X", "id_X");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Verify that the last request received is a Commit and that it contains a
   // debug info event.
@@ -2988,7 +2988,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_HappyCase) {
 
   // Generate another commit, but no debug info event.
   CreateUnsyncedDirectory("Y", "id_Y");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // See that it was received and contains no debug info events.
   EXPECT_EQ(2U, mock_server_->requests().size());
@@ -3007,7 +3007,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_PostFailsDontDrop) {
   // Generate a debug info event and trigger a commit.
   debug_info_getter_->AddDebugEvent();
   CreateUnsyncedDirectory("X", "id_X");
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   // Verify that the last request sent is a Commit and that it contains a debug
   // info event.
@@ -3016,7 +3016,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_PostFailsDontDrop) {
   EXPECT_EQ(1, mock_server_->last_request().debug_info().events_size());
 
   // Try again.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Verify that we've received another Commit and that it contains a debug info
   // event (just like the previous one).
@@ -3026,7 +3026,7 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_PostFailsDontDrop) {
 
   // Generate another commit and try again.
   CreateUnsyncedDirectory("Y", "id_Y");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // See that it was received and contains no debug info events.
   EXPECT_EQ(3U, mock_server_->requests().size());
@@ -3055,7 +3055,7 @@ TEST_F(SyncerTest, HugeConflict) {
       last_id = next_id;
     }
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Check they're in the expected conflict state.
   {
@@ -3072,7 +3072,7 @@ TEST_F(SyncerTest, HugeConflict) {
   // Add the missing parent directory.
   mock_server_->AddUpdateDirectory(parent_id, TestIdFactory::root(),
       "BOB", 2, 20, foreign_cache_guid(), "-3500");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Now they should all be OK.
   {
@@ -3089,7 +3089,7 @@ TEST_F(SyncerTest, HugeConflict) {
 TEST_F(SyncerTest, DontCrashOnCaseChange) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry e(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3099,25 +3099,25 @@ TEST_F(SyncerTest, DontCrashOnCaseChange) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateDirectory(1, 0, "BOB", 2, 20,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();  // USED TO CAUSE AN ASSERT
+  EXPECT_FALSE(SyncShareNudge());  // USED TO CAUSE AN ASSERT
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, UnsyncedItemAndUpdate) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateDirectory(2, 0, "bob", 2, 20,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();  // USED TO CAUSE AN ASSERT
+  EXPECT_TRUE(SyncShareNudge());  // USED TO CAUSE AN ASSERT
   saw_syncer_event_ = false;
 }
 
 TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   int64 local_folder_handle;
   syncable::Id local_folder_id;
   {
@@ -3136,7 +3136,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20,
                                   foreign_cache_guid(), "-1");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   saw_syncer_event_ = false;
   {
     // Update #20 should have been dropped in favor of the local version.
@@ -3155,14 +3155,14 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath) {
   }
   // Allow local changes to commit.
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   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,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3187,7 +3187,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   mock_server_->set_use_legacy_bookmarks_protocol(true);
   mock_server_->AddUpdateBookmark(1, 0, "Foo.htm", 10, 10,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   int64 local_folder_handle;
   syncable::Id local_folder_id;
   {
@@ -3206,7 +3206,7 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   mock_server_->AddUpdateBookmark(1, 0, "Bar.htm", 20, 20,
                                   foreign_cache_guid(), "-1");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   saw_syncer_event_ = false;
   {
     // Update #20 should have been dropped in favor of the local version.
@@ -3225,14 +3225,14 @@ TEST_F(SyncerTest, NewEntryAndAlteredServerEntrySharePath_OldBookmarksProto) {
   }
   // Allow local changes to commit.
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   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,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3259,7 +3259,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
                                    foreign_cache_guid(), "-1");
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
@@ -3271,7 +3271,7 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
   mock_server_->AddUpdateDirectory(2, 1, "A", 20, 20,
                                    foreign_cache_guid(), "-2");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   saw_syncer_event_ = false;
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3291,7 +3291,7 @@ TEST_F(SyncerTest, SwapEntryNames) {
   mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10,
                                    foreign_cache_guid(), "-2");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
@@ -3304,7 +3304,7 @@ TEST_F(SyncerTest, SwapEntryNames) {
     B.PutNonUniqueName("A");
     A.PutNonUniqueName("B");
   }
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   saw_syncer_event_ = false;
 }
 
@@ -3314,7 +3314,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) {
   mock_server_->AddUpdateBookmark(2, 0, "B", 10, 10,
                                   foreign_cache_guid(), "-2");
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry B(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3325,7 +3325,7 @@ TEST_F(SyncerTest, DualDeletionWithNewItemNameClash) {
   mock_server_->AddUpdateBookmark(2, 0, "A", 11, 11,
                                   foreign_cache_guid(), "-2");
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry B(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3344,7 +3344,7 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
 
   mock_server_->AddUpdateBookmark(1, 0, "bob", 1, 10,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3356,8 +3356,8 @@ TEST_F(SyncerTest, ResolveWeWroteTheyDeleted) {
                                   foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateDeleted();
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
+  EXPECT_FALSE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry bob(&trans, GET_BY_HANDLE, bob_metahandle);
@@ -3393,9 +3393,9 @@ 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();
+  EXPECT_FALSE(SyncShareNudge());
   EXPECT_EQ(1u, directory()->unsynced_entity_count());
-  SyncShareNudge();  // another bad id in here.
+  EXPECT_TRUE(SyncShareNudge());  // another bad id in here.
   EXPECT_EQ(0u, directory()->unsynced_entity_count());
   saw_syncer_event_ = false;
 }
@@ -3403,7 +3403,7 @@ TEST_F(SyncerTest, DuplicateIDReturn) {
 TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) {
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10,
                                    foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry bob(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3415,8 +3415,8 @@ TEST_F(SyncerTest, DeletedEntryWithBadParentInLoopCalculation) {
   }
   mock_server_->AddUpdateDirectory(2, 1, "fred", 1, 10,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
+  EXPECT_TRUE(SyncShareNudge());
 }
 
 TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
@@ -3441,7 +3441,7 @@ TEST_F(SyncerTest, ConflictResolverMergesLocalDeleteAndServerUpdate) {
 
   // We don't care about actually committing, just the resolution.
   mock_server_->set_conflict_all_commits(true);
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3480,7 +3480,7 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
   mock_server_->set_conflict_all_commits(true);
 
   // The syncer should not attempt to apply the invalid update.
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3500,7 +3500,7 @@ TEST_F(SyncerTest, MergingExistingItems) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(1, 0, "base", 10, 10,
                                   local_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(
@@ -3509,7 +3509,7 @@ TEST_F(SyncerTest, MergingExistingItems) {
   }
   mock_server_->AddUpdateBookmark(1, 0, "Copy of base", 50, 50,
                                   local_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 }
 
 // In this test a long changelog contains a child at the start of the changelog
@@ -3526,7 +3526,7 @@ TEST_F(SyncerTest, LongChangelistWithApplicationConflict) {
       folder_id, "stuck", 1, 1,
       foreign_cache_guid(), "-99999");
   mock_server_->SetChangesRemaining(depth - 1);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Buffer up a very long series of downloads.
   // We should never be stuck (conflict resolution shouldn't
@@ -3537,7 +3537,7 @@ TEST_F(SyncerTest, LongChangelistWithApplicationConflict) {
     mock_server_->SetChangesRemaining(depth - i);
   }
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // Ensure our folder hasn't somehow applied.
   {
@@ -3554,8 +3554,8 @@ TEST_F(SyncerTest, LongChangelistWithApplicationConflict) {
       TestIdFactory::root(), "folder", 1, 1,
       foreign_cache_guid(), "-1");
   mock_server_->SetChangesRemaining(0);
-  SyncShareNudge();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
+  EXPECT_TRUE(SyncShareNudge());
   // Check that everything is as expected after the commit.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3574,7 +3574,7 @@ TEST_F(SyncerTest, DontMergeTwoExistingItems) {
                                   foreign_cache_guid(), "-1");
   mock_server_->AddUpdateBookmark(2, 0, "base2", 10, 10,
                                   foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3584,7 +3584,7 @@ TEST_F(SyncerTest, DontMergeTwoExistingItems) {
   }
   mock_server_->AddUpdateBookmark(1, 0, "Copy of base", 50, 50,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry1(&trans, GET_BY_ID, ids_.FromNumber(1));
@@ -3605,11 +3605,11 @@ TEST_F(SyncerTest, TestUndeleteUpdate) {
                                    foreign_cache_guid(), "-1");
   mock_server_->AddUpdateDirectory(2, 1, "bar", 1, 2,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   mock_server_->AddUpdateDirectory(2, 1, "bar", 2, 3,
                                    foreign_cache_guid(), "-2");
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   int64 metahandle;
   {
@@ -3622,12 +3622,12 @@ TEST_F(SyncerTest, TestUndeleteUpdate) {
   mock_server_->AddUpdateDirectory(1, 0, "foo", 2, 4,
                                    foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // 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,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3644,7 +3644,7 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) {
                                    foreign_cache_guid(), "-1");
   mock_server_->AddUpdateDirectory(2, 0, ":::", 1, 2,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry entry(&trans, GET_BY_ID, ids_.FromNumber(2));
@@ -3652,11 +3652,11 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) {
     entry.PutParentId(ids_.FromNumber(1));
     EXPECT_TRUE(entry.PutIsUnsynced(true));
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // We use the same sync ts as before so our times match up.
   mock_server_->AddUpdateDirectory(2, 1, ":::", 2, 2,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 }
 
 // Don't crash when this occurs.
@@ -3666,7 +3666,7 @@ TEST_F(SyncerTest, UpdateWhereParentIsNotAFolder) {
   mock_server_->AddUpdateDirectory(2, 1, "BookmarkParent", 10, 10,
                                    foreign_cache_guid(), "-2");
   // Used to cause a CHECK
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     Entry good_entry(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1));
@@ -3688,7 +3688,7 @@ TEST_F(SyncerTest, DirectoryUpdateTest) {
   mock_server_->AddUpdateDirectory(in_in_root_id, in_root_id,
                                    "in_in_root_name", 3, 3,
                                    foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry in_root(&trans, GET_BY_ID, in_root_id);
@@ -3726,7 +3726,7 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
     bar_metahandle = child.GetMetahandle();
     in_dir_id = parent.GetId();
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry fail_by_old_id_entry(&trans, GET_BY_ID, in_root_id);
@@ -3761,7 +3761,7 @@ TEST_F(SyncerTest, TestClientCommandDuringUpdate) {
   mock_server_->AddUpdateDirectory(1, 0, "in_root", 1, 1,
                                    foreign_cache_guid(), "-1");
   mock_server_->SetGUClientCommand(command);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(TimeDelta::FromSeconds(8), last_short_poll_interval_received_);
   EXPECT_EQ(TimeDelta::FromSeconds(800), last_long_poll_interval_received_);
@@ -3781,7 +3781,7 @@ TEST_F(SyncerTest, TestClientCommandDuringUpdate) {
   mock_server_->AddUpdateDirectory(
       1, 0, "in_root", 1, 1, foreign_cache_guid(), "-1");
   mock_server_->SetGUClientCommand(command);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(TimeDelta::FromSeconds(180), last_short_poll_interval_received_);
   EXPECT_EQ(TimeDelta::FromSeconds(190), last_long_poll_interval_received_);
@@ -3805,7 +3805,7 @@ TEST_F(SyncerTest, TestClientCommandDuringCommit) {
   command->set_client_invalidation_hint_buffer_size(11);
   CreateUnsyncedDirectory("X", "id_X");
   mock_server_->SetCommitClientCommand(command);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(TimeDelta::FromSeconds(8), last_short_poll_interval_received_);
   EXPECT_EQ(TimeDelta::FromSeconds(800), last_long_poll_interval_received_);
@@ -3824,7 +3824,7 @@ TEST_F(SyncerTest, TestClientCommandDuringCommit) {
   command->set_client_invalidation_hint_buffer_size(9);
   CreateUnsyncedDirectory("Y", "id_Y");
   mock_server_->SetCommitClientCommand(command);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(TimeDelta::FromSeconds(180), last_short_poll_interval_received_);
   EXPECT_EQ(TimeDelta::FromSeconds(190), last_long_poll_interval_received_);
@@ -3841,7 +3841,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
       "folder_one", 1, 1, foreign_cache_guid(), "-1");
   mock_server_->AddUpdateDirectory(folder_two_id, TestIdFactory::root(),
       "folder_two", 1, 1, foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   {
     // A moved entry should send an "old parent."
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
@@ -3855,7 +3855,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) {
     create.PutIsUnsynced(true);
     create.PutSpecifics(DefaultBookmarkSpecifics());
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   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");
@@ -3891,7 +3891,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
   // Let there be an entry from the server.
   mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Check it out and delete it.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3903,7 +3903,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
     // Delete it locally.
     entry.PutIsDel(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Confirm we see IS_DEL and not SERVER_IS_DEL.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3914,12 +3914,12 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
     EXPECT_TRUE(entry.GetIsDel());
     EXPECT_FALSE(entry.GetServerIsDel());
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Update from server confirming deletion.
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 11,
                                   foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateDeleted();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // IS_DEL AND SERVER_IS_DEL now both true.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3933,7 +3933,7 @@ TEST_F(SyncerTest, TestSimpleUndelete) {
   // Undelete from server.
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // IS_DEL and SERVER_IS_DEL now both false.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3952,7 +3952,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
   mock_server_->set_conflict_all_commits(true);
   mock_server_->AddUpdateBookmark(id, root, "foo", 1, 10,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Check it out and delete it.
   {
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
@@ -3964,7 +3964,7 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
     // Delete it locally.
     entry.PutIsDel(true);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Confirm we see IS_DEL and not SERVER_IS_DEL.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -3975,12 +3975,12 @@ TEST_F(SyncerTest, TestUndeleteWithMissingDeleteUpdate) {
     EXPECT_TRUE(entry.GetIsDel());
     EXPECT_FALSE(entry.GetServerIsDel());
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // Say we do not get an update from server confirming deletion. Undelete
   // from server
   mock_server_->AddUpdateBookmark(id, root, "foo", 2, 12,
                                   foreign_cache_guid(), "-1");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // IS_DEL and SERVER_IS_DEL now both false.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4002,10 +4002,10 @@ TEST_F(SyncerTest, TestUndeleteIgnoreCorrectlyUnappliedUpdate) {
                                   foreign_cache_guid(), "-1");
   mock_server_->AddUpdateBookmark(id2, root, "foo", 1, 10,
                                   foreign_cache_guid(), "-2");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   mock_server_->AddUpdateBookmark(id2, root, "foo2", 2, 20,
                                   foreign_cache_guid(), "-2");
-  SyncShareNudge();  // Now just don't explode.
+  EXPECT_TRUE(SyncShareNudge());  // Now just don't explode.
 }
 
 TEST_F(SyncerTest, ClientTagServerCreatedUpdatesWork) {
@@ -4013,7 +4013,7 @@ TEST_F(SyncerTest, ClientTagServerCreatedUpdatesWork) {
                                    foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateClientTag("permfolder");
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4029,7 +4029,7 @@ TEST_F(SyncerTest, ClientTagServerCreatedUpdatesWork) {
   mock_server_->AddUpdateDirectory(1, 0, "permitem_renamed", 10, 100,
                                    foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateClientTag("permfolder");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4049,7 +4049,7 @@ TEST_F(SyncerTest, ClientTagIllegalUpdateIgnored) {
                                    foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateClientTag("permfolder");
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4065,7 +4065,7 @@ TEST_F(SyncerTest, ClientTagIllegalUpdateIgnored) {
   mock_server_->AddUpdateDirectory(1, 0, "permitem_renamed", 10, 100,
                                    foreign_cache_guid(), "-1");
   mock_server_->SetLastUpdateClientTag("wrongtag");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4105,7 +4105,7 @@ TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
                               "tag", 10, 100);
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
   // This should cause client tag reunion, preserving the metahandle.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4124,7 +4124,7 @@ TEST_F(SyncerTest, ClientTagUncommittedTagMatchesUpdate) {
   }
 
   mock_server_->set_conflict_all_commits(false);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The resolved entry ought to commit cleanly.
   {
@@ -4167,7 +4167,7 @@ TEST_F(SyncerTest, ClientTagConflictWithDeletedLocalEntry) {
                               ids_.root().GetServerId(),
                               "tag", 10, 100);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // The local entry will be overwritten.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4197,7 +4197,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
 
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   int64 tag1_metahandle = syncable::kInvalidMetaHandle;
   int64 tag2_metahandle = syncable::kInvalidMetaHandle;
   // This should cause client tag overwrite.
@@ -4239,7 +4239,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
   mock_server_->AddUpdatePref(id2.GetServerId(), "", "tag1", 12, 120);
   syncable::Id id3 = TestIdFactory::MakeServer("3");
   mock_server_->AddUpdatePref(id3.GetServerId(), "", "tag2", 13, 130);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4315,7 +4315,7 @@ TEST_F(SyncerTest, ClientTagClashWithinBatchOfUpdates) {
 
   mock_server_->set_conflict_all_commits(true);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   // This should cause client tag overwrite.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4367,7 +4367,7 @@ TEST_F(SyncerTest, EntryWithParentIdUpdatedWithEntryWithoutParentId) {
   mock_server_->AddUpdateSpecifics(1, 0, "Folder", 10, 10, true, 1,
                                    DefaultPreferencesSpecifics());
   mock_server_->SetLastUpdateServerTag(ModelTypeToRootTag(PREFERENCES));
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   Id pref_root_id;
   {
@@ -4383,7 +4383,7 @@ TEST_F(SyncerTest, EntryWithParentIdUpdatedWithEntryWithoutParentId) {
   mock_server_->AddUpdatePref(ids_.FromNumber(2).GetServerId(),
                               ids_.FromNumber(1).GetServerId(), "tag", 1, 10);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4397,7 +4397,7 @@ TEST_F(SyncerTest, EntryWithParentIdUpdatedWithEntryWithoutParentId) {
   mock_server_->AddUpdatePref(ids_.FromNumber(2).GetServerId(), "", "tag", 2,
                               20);
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4438,7 +4438,7 @@ TEST_F(SyncerTest, UniqueServerTagUpdates) {
   mock_server_->AddUpdateDirectory(
       2, 0, "update2", 2, 20, std::string(), std::string());
   mock_server_->SetLastUpdateServerTag("bob");
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -4469,28 +4469,28 @@ TEST_F(SyncerTest, GetUpdatesSetsRequestedTypes) {
   // GetUpdates handler.  EnableDatatype sets the expectation value from our
   // set of enabled/disabled datatypes.
   EnableDatatype(BOOKMARKS);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   EnableDatatype(AUTOFILL);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   EnableDatatype(PREFERENCES);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(BOOKMARKS);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(AUTOFILL);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 
   DisableDatatype(PREFERENCES);
   EnableDatatype(AUTOFILL);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
 }
 
@@ -4503,7 +4503,7 @@ TEST_F(SyncerTest, UpdateThenCommit) {
   mock_server_->AddUpdateDirectory(to_receive, ids_.root(), "x", 1, 10,
                                    foreign_cache_guid(), "-1");
   int64 commit_handle = CreateUnsyncedDirectory("y", to_commit);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The sync cycle should have included a GetUpdate, then a commit.  By the
   // time the commit happened, we should have known for sure that there were no
@@ -4535,7 +4535,7 @@ TEST_F(SyncerTest, UpdateFailsThenDontCommit) {
                                    foreign_cache_guid(), "-1");
   int64 commit_handle = CreateUnsyncedDirectory("y", to_commit);
   mock_server_->FailNextPostBufferToPathCall();
-  SyncShareNudge();
+  EXPECT_FALSE(SyncShareNudge());
 
   syncable::ReadTransaction trans(FROM_HERE, directory());
 
@@ -4819,11 +4819,11 @@ class SyncerBookmarksTest : public SyncerTest {
 TEST_F(SyncerBookmarksTest, CreateSyncThenDeleteSync) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndCreated();
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndDeleted();
 }
 
@@ -4852,7 +4852,7 @@ TEST_F(SyncerBookmarksTest, CreateThenDeleteBeforeSync) {
 TEST_F(SyncerBookmarksTest, LocalDeleteRemoteChangeConflict) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndCreated();
   Delete();
   ExpectUnsyncedDeletion();
@@ -4862,7 +4862,7 @@ TEST_F(SyncerBookmarksTest, LocalDeleteRemoteChangeConflict) {
   mock_server_->AddUpdateBookmark(GetServerId(), Id::GetRoot(), "dummy", 10, 10,
                                   local_cache_guid(), local_id_.GetServerId());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndDeleted();
 }
 
@@ -4876,7 +4876,7 @@ TEST_F(SyncerBookmarksTest, CreateThenDeleteDuringCommit) {
   mock_server_->SetMidCommitCallback(
       base::Bind(&SyncerBookmarksTest::Delete, base::Unretained(this)));
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndDeleted();
 }
 
@@ -4890,7 +4890,7 @@ TEST_F(SyncerBookmarksTest, CreateThenUpdateAndDeleteDuringCommit) {
   mock_server_->SetMidCommitCallback(base::Bind(
       &SyncerBookmarksTest::UpdateAndDelete, base::Unretained(this)));
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   ExpectSyncedAndDeleted();
 }
 
@@ -5057,7 +5057,7 @@ class SyncerUndeletionTest : public SyncerTest {
 TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5068,7 +5068,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   ExpectUnsyncedDeletion();
   mock_server_->SetMidCommitCallback(
       base::Bind(&SyncerUndeletionTest::Undelete, base::Unretained(this)));
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // We will continue to commit until all nodes are synced, so we expect
   // that both the delete and following undelete were committed.  We haven't
@@ -5099,7 +5099,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
   update->set_originator_cache_guid(local_cache_guid());
   update->set_originator_client_item_id(local_id_.GetServerId());
 
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5108,7 +5108,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
 TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5119,7 +5119,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   ExpectUnsyncedDeletion();
   Undelete();
   ExpectUnsyncedEdit();  // Edit, not undelete: server thinks it exists.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5136,7 +5136,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
   sync_pb::SyncEntity* update = mock_server_->AddUpdateFromLastCommit();
   update->set_originator_cache_guid(local_cache_guid());
   update->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5145,7 +5145,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5154,7 +5154,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   // Delete and commit.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5168,7 +5168,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5177,7 +5177,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5186,7 +5186,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   sync_pb::SyncEntity* update = mock_server_->AddUpdateFromLastCommit();
   update->set_originator_cache_guid(local_cache_guid());
   update->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -5194,7 +5194,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   // Delete and commit.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The item ought to have committed successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5204,7 +5204,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  Should be consistent.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndDeleted();
@@ -5215,7 +5215,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
 
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.  The undeletion should prevail.
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5225,7 +5225,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5235,7 +5235,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   sync_pb::SyncEntity* update1 = mock_server_->AddUpdateFromLastCommit();
   update1->set_originator_cache_guid(local_cache_guid());
   update1->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -5246,7 +5246,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
     Entry entry(&trans, GET_BY_HANDLE, metahandle_);
     mock_server_->AddUpdateTombstone(entry.GetId(), PREFERENCES);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5256,7 +5256,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   // Undelete it locally.
   Undelete();
   ExpectUnsyncedUndeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5266,7 +5266,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
   sync_pb::SyncEntity* update2 = mock_server_->AddUpdateFromLastCommit();
   update2->set_originator_cache_guid(local_cache_guid());
   update2->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5275,7 +5275,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) {
 TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5288,7 +5288,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
     Entry entry(&trans, GET_BY_HANDLE, metahandle_);
     mock_server_->AddUpdateTombstone(entry.GetId(), PREFERENCES);
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5298,7 +5298,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   // Undelete it locally.
   Undelete();
   ExpectUnsyncedUndeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5308,7 +5308,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
   sync_pb::SyncEntity* update = mock_server_->AddUpdateFromLastCommit();
   update->set_originator_cache_guid(local_cache_guid());
   update->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5317,7 +5317,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) {
 TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5327,7 +5327,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   sync_pb::SyncEntity* update = mock_server_->AddUpdateFromLastCommit();
   update->set_originator_cache_guid(local_cache_guid());
   update->set_originator_client_item_id(local_id_.GetServerId());
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -5335,7 +5335,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   // We delete the item.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5345,7 +5345,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
   // Now, encounter a GetUpdates corresponding to the just-committed
   // deletion update.
   mock_server_->AddUpdateFromLastCommit();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndDeleted();
@@ -5360,7 +5360,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
         client_tag_, 100, 1000);
   }
   mock_server_->SetLastUpdateClientTag(client_tag_);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5369,7 +5369,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) {
 TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
   Create();
   ExpectUnsyncedCreation();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5383,7 +5383,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
     Entry entry(&trans, GET_BY_HANDLE, metahandle_);
     update->set_originator_client_item_id(local_id_.GetServerId());
   }
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   ExpectSyncedAndCreated();
@@ -5391,7 +5391,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
   // We delete the item.
   Delete();
   ExpectUnsyncedDeletion();
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
 
   // The update ought to have applied successfully.
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
@@ -5409,7 +5409,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) {
         client_tag_, 100, 1000);
   }
   mock_server_->SetLastUpdateClientTag(client_tag_);
-  SyncShareNudge();
+  EXPECT_TRUE(SyncShareNudge());
   EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
   EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
   ExpectSyncedAndCreated();
@@ -5470,7 +5470,8 @@ TEST_P(MixedResult, ExtensionsActivity) {
     context_->extensions_activity()->PutRecords(records);
   }
 
-  SyncShareNudge();
+  EXPECT_EQ(!ShouldFailBookmarkCommit() && !ShouldFailAutofillCommit(),
+            SyncShareNudge());
 
   ExtensionsActivity::Records final_monitor_records;
   context_->extensions_activity()->GetAndClearRecords(&final_monitor_records);
diff --git a/sync/internal_api/js_sync_manager_observer_unittest.cc b/sync/internal_api/js_sync_manager_observer_unittest.cc
index 02af3f7..6062403 100644
--- a/sync/internal_api/js_sync_manager_observer_unittest.cc
+++ b/sync/internal_api/js_sync_manager_observer_unittest.cc
@@ -74,6 +74,7 @@ TEST_F(JsSyncManagerObserverTest, OnSyncCycleCompleted) {
       false,
       0,
       base::Time::Now(),
+      base::Time::Now(),
       std::vector<int>(MODEL_TYPE_COUNT, 0),
       std::vector<int>(MODEL_TYPE_COUNT, 0),
       sync_pb::GetUpdatesCallerInfo::UNKNOWN);
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.cc b/sync/internal_api/public/sessions/sync_session_snapshot.cc
index 3380882..dcb3f73 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.cc
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.cc
@@ -34,6 +34,7 @@ SyncSessionSnapshot::SyncSessionSnapshot(
     bool notifications_enabled,
     size_t num_entries,
     base::Time sync_start_time,
+    base::Time poll_finish_time,
     const std::vector<int>& num_entries_by_type,
     const std::vector<int>& num_to_delete_entries_by_type,
     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source)
@@ -46,6 +47,7 @@ SyncSessionSnapshot::SyncSessionSnapshot(
       notifications_enabled_(notifications_enabled),
       num_entries_(num_entries),
       sync_start_time_(sync_start_time),
+      poll_finish_time_(poll_finish_time),
       num_entries_by_type_(num_entries_by_type),
       num_to_delete_entries_by_type_(num_to_delete_entries_by_type),
       legacy_updates_source_(legacy_updates_source),
@@ -140,6 +142,10 @@ base::Time SyncSessionSnapshot::sync_start_time() const {
   return sync_start_time_;
 }
 
+base::Time SyncSessionSnapshot::poll_finish_time() const {
+  return poll_finish_time_;
+}
+
 bool SyncSessionSnapshot::is_initialized() const {
   return is_initialized_;
 }
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.h b/sync/internal_api/public/sessions/sync_session_snapshot.h
index 8641fbd..c47b3c6 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.h
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.h
@@ -39,6 +39,7 @@ class SYNC_EXPORT SyncSessionSnapshot {
       bool notifications_enabled,
       size_t num_entries,
       base::Time sync_start_time,
+      base::Time poll_finish_time,
       const std::vector<int>& num_entries_by_type,
       const std::vector<int>& num_to_delete_entries_by_type,
       sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source);
@@ -59,6 +60,7 @@ class SYNC_EXPORT SyncSessionSnapshot {
   bool notifications_enabled() const;
   size_t num_entries() const;
   base::Time sync_start_time() const;
+  base::Time poll_finish_time() const;
   const std::vector<int>& num_entries_by_type() const;
   const std::vector<int>& num_to_delete_entries_by_type() const;
   sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source() const;
@@ -76,6 +78,7 @@ class SYNC_EXPORT SyncSessionSnapshot {
   bool notifications_enabled_;
   size_t num_entries_;
   base::Time sync_start_time_;
+  base::Time poll_finish_time_;
 
   std::vector<int> num_entries_by_type_;
   std::vector<int> num_to_delete_entries_by_type_;
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
index d19a69c..475a9c9 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
+++ b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc
@@ -51,6 +51,7 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) {
                                false,
                                0,
                                base::Time::Now(),
+                               base::Time::Now(),
                                std::vector<int>(MODEL_TYPE_COUNT,0),
                                std::vector<int>(MODEL_TYPE_COUNT, 0),
                                sync_pb::GetUpdatesCallerInfo::UNKNOWN);
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index c66a8e7..41d9cae 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -298,7 +298,8 @@ class SYNC_EXPORT SyncManager {
 
   // Put the syncer in normal mode ready to perform nudges and polls.
   virtual void StartSyncingNormally(
-      const ModelSafeRoutingInfo& routing_info) = 0;
+      const ModelSafeRoutingInfo& routing_info,
+      base::Time last_poll_time) = 0;
 
   // Switches the mode of operation to CONFIGURATION_MODE and performs
   // any configuration tasks needed as determined by the params. Once complete,
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index edad9b9..39d9ada 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -83,7 +83,8 @@ class FakeSyncManager : public SyncManager {
       ModelTypeSet types) override;
   bool PurgePartiallySyncedTypes() override;
   void UpdateCredentials(const SyncCredentials& credentials) override;
-  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info) override;
+  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info,
+                            base::Time last_poll_time) override;
   void ConfigureSyncer(ConfigureReason reason,
                        ModelTypeSet to_download,
                        ModelTypeSet to_purge,
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 387b1d2..fe0bb0f 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -218,7 +218,7 @@ void SyncManagerImpl::ConfigureSyncer(
                              ready_task,
                              retry_task);
 
-  scheduler_->Start(SyncScheduler::CONFIGURATION_MODE);
+  scheduler_->Start(SyncScheduler::CONFIGURATION_MODE, base::Time());
   scheduler_->ScheduleConfiguration(params);
 }
 
@@ -334,7 +334,7 @@ void SyncManagerImpl::Init(InitArgs* args) {
   scheduler_ = args->internal_components_factory->BuildScheduler(
       name_, session_context_.get(), args->cancelation_signal).Pass();
 
-  scheduler_->Start(SyncScheduler::CONFIGURATION_MODE);
+  scheduler_->Start(SyncScheduler::CONFIGURATION_MODE, base::Time());
 
   initialized_ = true;
 
@@ -406,7 +406,8 @@ void SyncManagerImpl::OnPassphraseTypeChanged(
 }
 
 void SyncManagerImpl::StartSyncingNormally(
-    const ModelSafeRoutingInfo& routing_info) {
+    const ModelSafeRoutingInfo& routing_info,
+    base::Time last_poll_time) {
   // Start the sync scheduler.
   // TODO(sync): We always want the newest set of routes when we switch back
   // to normal mode. Figure out how to enforce set_routing_info is always
@@ -414,7 +415,8 @@ void SyncManagerImpl::StartSyncingNormally(
   // mode.
   DCHECK(thread_checker_.CalledOnValidThread());
   session_context_->SetRoutingInfo(routing_info);
-  scheduler_->Start(SyncScheduler::NORMAL_MODE);
+  scheduler_->Start(SyncScheduler::NORMAL_MODE,
+                    last_poll_time);
 }
 
 syncable::Directory* SyncManagerImpl::directory() {
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index af9068b..6e9795b 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -74,7 +74,8 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl
       ModelTypeSet types) override;
   bool PurgePartiallySyncedTypes() override;
   void UpdateCredentials(const SyncCredentials& credentials) override;
-  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info) override;
+  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info,
+                            base::Time last_poll_time) override;
   void ConfigureSyncer(ConfigureReason reason,
                        ModelTypeSet to_download,
                        ModelTypeSet to_purge,
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 80972bf..222e7e8 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -1075,7 +1075,7 @@ class SyncManagerTest : public testing::Test,
 TEST_F(SyncManagerTest, GetAllNodesForTypeTest) {
   ModelSafeRoutingInfo routing_info;
   GetModelSafeRoutingInfo(&routing_info);
-  sync_manager_.StartSyncingNormally(routing_info);
+  sync_manager_.StartSyncingNormally(routing_info, base::Time());
 
   scoped_ptr<base::ListValue> node_list(
       sync_manager_.GetAllNodesForType(syncer::PREFERENCES));
@@ -2377,7 +2377,7 @@ class MockSyncScheduler : public FakeSyncScheduler {
   MockSyncScheduler() : FakeSyncScheduler() {}
   virtual ~MockSyncScheduler() {}
 
-  MOCK_METHOD1(Start, void(SyncScheduler::Mode));
+  MOCK_METHOD2(Start, void(SyncScheduler::Mode, base::Time));
   MOCK_METHOD1(ScheduleConfiguration, void(const ConfigurationParams&));
 };
 
@@ -2437,7 +2437,7 @@ TEST_F(SyncManagerTestWithMockScheduler, BasicConfiguration) {
   ModelTypeSet disabled_types = Difference(ModelTypeSet::All(), enabled_types);
 
   ConfigurationParams params;
-  EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE));
+  EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE, _));
   EXPECT_CALL(*scheduler(), ScheduleConfiguration(_)).
       WillOnce(SaveArg<0>(&params));
 
@@ -2489,7 +2489,7 @@ TEST_F(SyncManagerTestWithMockScheduler, ReConfiguration) {
   ModelTypeSet enabled_types = GetRoutingInfoTypes(new_routing_info);
 
   ConfigurationParams params;
-  EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE));
+  EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE, _));
   EXPECT_CALL(*scheduler(), ScheduleConfiguration(_)).
       WillOnce(SaveArg<0>(&params));
 
diff --git a/sync/internal_api/sync_rollback_manager.cc b/sync/internal_api/sync_rollback_manager.cc
index e7b8ea6..4f45d47 100644
--- a/sync/internal_api/sync_rollback_manager.cc
+++ b/sync/internal_api/sync_rollback_manager.cc
@@ -43,7 +43,7 @@ void SyncRollbackManager::Init(InitArgs* args) {
 }
 
 void SyncRollbackManager::StartSyncingNormally(
-    const ModelSafeRoutingInfo& routing_info){
+    const ModelSafeRoutingInfo& routing_info, base::Time last_poll_time){
   if (rollback_ready_types_.Empty()) {
     NotifyRollbackDone();
     return;
diff --git a/sync/internal_api/sync_rollback_manager.h b/sync/internal_api/sync_rollback_manager.h
index 0abcc27..70580e5 100644
--- a/sync/internal_api/sync_rollback_manager.h
+++ b/sync/internal_api/sync_rollback_manager.h
@@ -23,7 +23,8 @@ class SYNC_EXPORT_PRIVATE SyncRollbackManager : public SyncRollbackManagerBase {
 
   // SyncManager implementation.
   void Init(InitArgs* args) override;
-  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info) override;
+  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info,
+                            base::Time last_poll_time) override;
 
  private:
   // Deletes specified entries in local model.
diff --git a/sync/internal_api/sync_rollback_manager_base.cc b/sync/internal_api/sync_rollback_manager_base.cc
index 80a63c5..647019f 100644
--- a/sync/internal_api/sync_rollback_manager_base.cc
+++ b/sync/internal_api/sync_rollback_manager_base.cc
@@ -86,7 +86,7 @@ void SyncRollbackManagerBase::UpdateCredentials(
 }
 
 void SyncRollbackManagerBase::StartSyncingNormally(
-    const ModelSafeRoutingInfo& routing_info){
+    const ModelSafeRoutingInfo& routing_info, base::Time last_poll_time){
 }
 
 void SyncRollbackManagerBase::ConfigureSyncer(
diff --git a/sync/internal_api/sync_rollback_manager_base.h b/sync/internal_api/sync_rollback_manager_base.h
index 3fadaf4..572ba74 100644
--- a/sync/internal_api/sync_rollback_manager_base.h
+++ b/sync/internal_api/sync_rollback_manager_base.h
@@ -41,7 +41,8 @@ class SYNC_EXPORT_PRIVATE SyncRollbackManagerBase :
       ModelTypeSet types) override;
   bool PurgePartiallySyncedTypes() override;
   void UpdateCredentials(const SyncCredentials& credentials) override;
-  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info) override;
+  void StartSyncingNormally(const ModelSafeRoutingInfo& routing_info,
+                            base::Time last_poll_time) override;
   void ConfigureSyncer(ConfigureReason reason,
                        ModelTypeSet to_download,
                        ModelTypeSet to_purge,
diff --git a/sync/internal_api/sync_rollback_manager_unittest.cc b/sync/internal_api/sync_rollback_manager_unittest.cc
index 31ffd14..0b5a6b6 100644
--- a/sync/internal_api/sync_rollback_manager_unittest.cc
+++ b/sync/internal_api/sync_rollback_manager_unittest.cc
@@ -202,7 +202,7 @@ TEST_F(SyncRollbackManagerTest, RollbackBasic) {
 
   ModelSafeRoutingInfo routing_info;
   routing_info[PREFERENCES] = GROUP_UI;
-  rollback_manager.StartSyncingNormally(routing_info);
+  rollback_manager.StartSyncingNormally(routing_info, base::Time());
 }
 
 TEST_F(SyncRollbackManagerTest, NoRollbackOfTypesNotBackedUp) {
@@ -226,7 +226,7 @@ TEST_F(SyncRollbackManagerTest, NoRollbackOfTypesNotBackedUp) {
 
   ModelSafeRoutingInfo routing_info;
   routing_info[PREFERENCES] = GROUP_UI;
-  rollback_manager.StartSyncingNormally(routing_info);
+  rollback_manager.StartSyncingNormally(routing_info, base::Time());
 
   // APP entry is still valid.
   EXPECT_TRUE(VerifyEntry(rollback_manager.GetUserShare(), APPS, "app1"));
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index 43b6eb6..94ebdac 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -118,7 +118,7 @@ void FakeSyncManager::UpdateCredentials(const SyncCredentials& credentials) {
 }
 
 void FakeSyncManager::StartSyncingNormally(
-      const ModelSafeRoutingInfo& routing_info) {
+      const ModelSafeRoutingInfo& routing_info, base::Time last_poll_time) {
   // Do nothing.
 }
 
diff --git a/sync/sessions/status_controller.cc b/sync/sessions/status_controller.cc
index 4610a51..acb7f73 100644
--- a/sync/sessions/status_controller.cc
+++ b/sync/sessions/status_controller.cc
@@ -36,6 +36,10 @@ void StatusController::UpdateStartTime() {
   sync_start_time_ = base::Time::Now();
 }
 
+void StatusController::UpdatePollTime() {
+  poll_finish_time_ = base::Time::Now();
+}
+
 void StatusController::set_num_successful_bookmark_commits(int value) {
   model_neutral_.num_successful_bookmark_commits = value;
 }
diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h
index 9844929..eea5872 100644
--- a/sync/sessions/status_controller.h
+++ b/sync/sessions/status_controller.h
@@ -56,11 +56,17 @@ class SYNC_EXPORT_PRIVATE StatusController {
 
   int num_server_overwrites() const;
 
+  // The time at which we started the first sync cycle in this session.
   base::Time sync_start_time() const {
-    // The time at which we sent the first GetUpdates command for this sync.
     return sync_start_time_;
   }
 
+  // If a poll was performed in this session, the time at which it finished.
+  // Not set if no poll was performed.
+  base::Time poll_finish_time() const {
+    return poll_finish_time_;
+  }
+
   const ModelNeutralState& model_neutral_state() const {
     return model_neutral_;
   }
@@ -91,12 +97,18 @@ class SYNC_EXPORT_PRIVATE StatusController {
   void set_commit_result(const SyncerError result);
 
   void UpdateStartTime();
+  void UpdatePollTime();
 
  private:
   ModelNeutralState model_neutral_;
 
+  // Time the last sync cycle began.
   base::Time sync_start_time_;
 
+  // If a poll was performed, the time it finished. Not set if not poll was
+  // performed.
+  base::Time poll_finish_time_;
+
   DISALLOW_COPY_AND_ASSIGN(StatusController);
 };
 
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index e352e70..70c1a82 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -60,6 +60,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshotWithSource(
       context_->notifications_enabled(),
       dir->GetEntriesCount(),
       status_controller_->sync_start_time(),
+      status_controller_->poll_finish_time(),
       num_entries_by_type,
       num_to_delete_entries_by_type,
       legacy_updates_source);
diff --git a/sync/test/engine/fake_sync_scheduler.cc b/sync/test/engine/fake_sync_scheduler.cc
index a9b6f57..26707dc 100644
--- a/sync/test/engine/fake_sync_scheduler.cc
+++ b/sync/test/engine/fake_sync_scheduler.cc
@@ -10,7 +10,7 @@ FakeSyncScheduler::FakeSyncScheduler() {}
 
 FakeSyncScheduler::~FakeSyncScheduler() {}
 
-void FakeSyncScheduler::Start(Mode mode) {
+void FakeSyncScheduler::Start(Mode mode, base::Time last_poll_time) {
 }
 
 void FakeSyncScheduler::Stop() {
diff --git a/sync/test/engine/fake_sync_scheduler.h b/sync/test/engine/fake_sync_scheduler.h
index 8e44d13..475a9eb 100644
--- a/sync/test/engine/fake_sync_scheduler.h
+++ b/sync/test/engine/fake_sync_scheduler.h
@@ -19,7 +19,7 @@ class FakeSyncScheduler : public SyncScheduler {
   FakeSyncScheduler();
   ~FakeSyncScheduler() override;
 
-  void Start(Mode mode) override;
+  void Start(Mode mode, base::Time last_poll_time) override;
   void Stop() override;
   void ScheduleLocalNudge(
       ModelTypeSet types,
diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc
index af3af65..8422d2e 100644
--- a/sync/tools/sync_client.cc
+++ b/sync/tools/sync_client.cc
@@ -447,7 +447,7 @@ int SyncClientMain(int argc, char* argv[]) {
   invalidator->RegisterHandler(shim.get());
   invalidator->UpdateRegisteredIds(
       shim.get(), ModelTypeSetToObjectIdSet(model_types));
-  sync_manager->StartSyncingNormally(routing_info);
+  sync_manager->StartSyncingNormally(routing_info, base::Time());
 
   sync_loop.Run();