Fix the false-positive detection of commit 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/bind.h"
@@ skipping 14 matching lines @@
 
 using sessions::SyncSession;
 using sessions::SyncSessionSnapshot;
 using sessions::SyncSourceInfo;
 using syncable::ModelTypePayloadMap;
 using syncable::ModelTypeBitSet;
 using sync_pb::GetUpdatesCallerInfo;
 
 namespace {
 bool ShouldRequestEarlyExit(
-    const browser_sync::SyncProtocolError& error) {
+    const browser_sync::SyncOperationResult& error) {
   switch (error.error_type) {
-    case browser_sync::SYNC_SUCCESS:
+    case browser_sync::OPERATION_SUCCESS:
     case browser_sync::MIGRATION_DONE:
     case browser_sync::THROTTLED:
     case browser_sync::TRANSIENT_ERROR:
       return false;
     case browser_sync::NOT_MY_BIRTHDAY:
     case browser_sync::CLEAR_PENDING:
       // If we send terminate sync early then |sync_cycle_ended| notification
       // would not be sent. If there were no actions then |ACTIONABLE_ERROR|
       // notification wouldnt be sent either. Then the UI layer would be left
       // waiting forever. So assert we would send something.
       DCHECK(error.action != browser_sync::UNKNOWN_ACTION);
       return true;
     case browser_sync::INVALID_CREDENTIAL:
       // The notification for this is handled by PostAndProcessHeaders|.
       // Server does no have to send any action for this.
       return true;
     // Make the default a NOTREACHED. So if a new error is introduced we
     // think about its expected functionality.
     default:
       NOTREACHED();
       return false;
   }
 }
 
 bool IsActionableError(
-    const browser_sync::SyncProtocolError& error) {
+    const browser_sync::SyncOperationResult& error) {
   return (error.action != browser_sync::UNKNOWN_ACTION);
 }
 }  // namespace
 
 SyncScheduler::DelayProvider::DelayProvider() {}
 SyncScheduler::DelayProvider::~DelayProvider() {}
 
 SyncScheduler::WaitInterval::WaitInterval()
     : mode(UNKNOWN),
       had_nudge(false) {
@@ skipping 740 matching lines @@
     return;  // Nothing to do.
   }
 
   SVLOG(2) << "Updating the next polling time after SyncMain";
   ScheduleNextSync(job);
 }
 
 void SyncScheduler::ScheduleNextSync(const SyncSessionJob& old_job) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   DCHECK(!old_job.session->HasMoreToSync());
-  // Note: |num_server_changes_remaining| > 0 here implies that we received a
-  // broken response while trying to download all updates, because the Syncer
-  // will loop until this value is exhausted. Also, if unsynced_handles exist
-  // but HasMoreToSync is false, this implies that the Syncer determined no
-  // forward progress was possible at this time (an error, such as an HTTP
-  // 500, is likely to have occurred during commit).
-  int num_server_changes_remaining =
-      old_job.session->status_controller()->num_server_changes_remaining();
-  size_t num_unsynced_handles =
-      old_job.session->status_controller()->unsynced_handles().size();
-  const bool work_to_do =
-      num_server_changes_remaining > 0 || num_unsynced_handles > 0;
-  SVLOG(2) << "num server changes remaining: " << num_server_changes_remaining
-           << ", num unsynced handles: " << num_unsynced_handles
-           << ", syncer has work to do: " << work_to_do;
 
   AdjustPolling(&old_job);
 
   // TODO(tim): Old impl had special code if notifications disabled. Needed?
-  if (!work_to_do) {
+  if (!old_job.session->ExperiencedTransientError()) {
     // Success implies backoff relief.  Note that if this was a
     // "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
+    // 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;
   }
 
   // We are in backoff mode and our time did not run out. That means we had
   // a local change, notification from server or a network connection change
   // notification. In any case set had_nudge = true so we dont retry next
@@ skipping 305 matching lines @@
 
 #undef SVLOG_LOC
 
 #undef SVLOG
 
 #undef SLOG
 
 #undef ENUM_CASE
 
 }  // browser_sync