Dont retry sync in case of auth errors.

chrome/browser/sync/engine/sync_scheduler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/engine/sync_scheduler.h"
 
 #include <algorithm>
 #include <cstring>
 
 #include "base/compiler_specific.h"
@@ skipping 223 matching lines @@
 }
 
 void SyncScheduler::StartImpl(Mode mode, ModeChangeCallback* callback) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   SVLOG(2) << "In StartImpl with mode " << GetModeString(mode);
 
   scoped_ptr<ModeChangeCallback> scoped_callback(callback);
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   DCHECK(!session_context_->account_name().empty());
   DCHECK(syncer_.get());
+  Mode old_mode = mode_;
   mode_ = mode;
   AdjustPolling(NULL);  // Will kick start poll timer if needed.
   if (scoped_callback.get())
     scoped_callback->Run();
 
-  // We just changed our mode. See if there are any pending jobs that we could
-  // execute in the new mode.
-  DoPendingJobIfPossible(false);
+  if (old_mode != mode_) {
+    // We just changed our mode. See if there are any pending jobs that we could
+    // execute in the new mode.
+    DoPendingJobIfPossible(false);
+  }
 }
 
 SyncScheduler::JobProcessDecision SyncScheduler::DecideWhileInWaitInterval(
     const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   DCHECK(wait_interval_.get());
   DCHECK_NE(job.purpose, SyncSessionJob::CLEAR_USER_DATA);
   DCHECK_NE(job.purpose, SyncSessionJob::CLEANUP_DISABLED_TYPES);
 
   SVLOG(2) << "DecideWhileInWaitInterval with WaitInterval mode "
            << WaitInterval::GetModeString(wait_interval_->mode)
            << (wait_interval_->had_nudge ? " (had nudge)" : "")
            << (job.is_canary_job ? " (canary)" : "");
 
   if (job.purpose == SyncSessionJob::POLL)
     return DROP;
 
   DCHECK(job.purpose == SyncSessionJob::NUDGE ||
          job.purpose == SyncSessionJob::CONFIGURATION);
   if (wait_interval_->mode == WaitInterval::THROTTLED)
     return SAVE;
 
   DCHECK_EQ(wait_interval_->mode, WaitInterval::EXPONENTIAL_BACKOFF);
   if (job.purpose == SyncSessionJob::NUDGE) {
     if (mode_ == CONFIGURATION_MODE)
       return SAVE;
 
     // If we already had one nudge then just drop this nudge. We will retry
     // later when the timer runs out.
-    return wait_interval_->had_nudge ? DROP : CONTINUE;
+    if (!job.is_canary_job)
+      return wait_interval_->had_nudge ? DROP : CONTINUE;
+    else // We are here because timer ran out. So retry.
+      return CONTINUE;

[tim (not reviewing), 2011/08/04 00:24:40]

so we always continue with canary_jobs?  don't we set that in more places than
purely the exp. backoff callback?

[lipalani1, 2011/08/04 01:14:33]

We call that when network connection changes. We still want to continue in that
case as well. No other places set that.
On 2011/08/04 00:24:40, timsteele wrote:

   }
-  // This is a config job.
   return job.is_canary_job ? CONTINUE : SAVE;
 }
 
 SyncScheduler::JobProcessDecision SyncScheduler::DecideOnJob(
     const SyncSessionJob& job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   if (job.purpose == SyncSessionJob::CLEAR_USER_DATA ||
       job.purpose == SyncSessionJob::CLEANUP_DISABLED_TYPES)
     return CONTINUE;
 
@@ skipping 504 matching lines @@
     // "one-off" job (i.e. purpose ==
     // SyncSessionJob::{CLEAR_USER_DATA,CLEANUP_DISABLED_TYPES}), if
     // there was work_to_do before it ran this wont have changed, as
     // jobs like this don't run a full sync cycle.  So we don't need
     // special code here.
     wait_interval_.reset();
     SVLOG(2) << "Job succeeded so not scheduling more jobs";
     return;
   }
 
-  if (old_job.session->source().updates_source ==
-      GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION) {
-    SVLOG(2) << "Job failed with source continuation";
-    // We don't seem to have made forward progress. Start or extend backoff.
-    HandleConsecutiveContinuationError(old_job);
-  } else if (IsBackingOff()) {
+  if (IsBackingOff() && wait_interval_->timer.IsRunning() &&

[tim (not reviewing), 2011/08/04 00:24:40]

comment

[lipalani1, 2011/08/04 01:14:33]

Done.

+      mode_ == NORMAL_MODE) {
     SVLOG(2) << "A nudge during backoff failed";
     // We weren't continuing but we're in backoff; must have been a nudge.
     DCHECK_EQ(SyncSessionJob::NUDGE, old_job.purpose);
     DCHECK(!wait_interval_->had_nudge);
     wait_interval_->had_nudge = true;
+    // Old job did not finish. So make it the pending job.
+    InitOrCoalescePendingJob(old_job);
     // Resume waiting.
     RestartWaiting();
+  } else if (old_job.session->source().updates_source ==
+             GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION) {
+    SVLOG(2) << "Job failed with source continuation";
+    // We don't seem to have made forward progress. Start or extend backoff.
+    HandleConsecutiveContinuationError(old_job);
   } else {
     SVLOG(2) << "Failed. Schedule a job with continuation as source";
     // We weren't continuing and we aren't in backoff.  Schedule a normal
     // continuation.
     if (old_job.purpose == SyncSessionJob::CONFIGURATION) {
       ScheduleConfigImpl(old_job.session->routing_info(),
           old_job.session->workers(),
           GetUpdatesFromNudgeSource(NUDGE_SOURCE_CONTINUATION));
     } else  {
       // For all other purposes(nudge and poll) we schedule a retry nudge.
@@ skipping 244 matching lines @@
 
 #undef SLOG
 
 #undef VLOG_LOC
 
 #undef VLOG_LOC_STREAM
 
 #undef ENUM_CASE
 
 }  // browser_sync