Fix the false-positive detection of commit errors

chrome/browser/sync/sessions/sync_session.cc

Index: chrome/browser/sync/sessions/sync_session.cc
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index f399d7d7904ed1fee8a5425a8641cfaaab04675e..42ee29845e6104aa9af7a96fcc2fdaeb9ef8310b 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -141,7 +141,71 @@ bool SyncSession::HasMoreToSync() const {
       status->conflicts_resolved();
       // Or, we have conflicting updates, but we're making progress on
       // resolving them...
+}
+
+static bool IndicatesTransientError(SyncOperationResult result) {
+  switch (result.error_type) {
+    // Not errors at all
+    case OPERATION_SUCCESS:
+      return false;
+
+    // These errors require some action on our part.  They'll still be there if
+    // we retry the request without changing anything.
+    case DIRECTORY_LOOKUP_FAILED:
+    case SYNC_AUTH_ERROR:
+    case NOT_MY_BIRTHDAY:
+    case CLEAR_PENDING:
+    case MIGRATION_DONE:
+      return false;
+
+    // Technically, this will go away on its own.  However, we don't want to
+    // simply retry the request.  We can handle this better if we pretend it's
+    // not a transient error in need of exponential backoff.
+    case THROTTLED:
+      return false;
+
+    // We don't know whether or not this is transient.  To avoid overloading the
+    // server, let's assume that it isn't.
+    case UNKNOWN_ERROR:
+      return false;
+
+    // Network errors might go away on their own.
+    case NETWORK_CONNECTION_UNAVAILABLE:
+    case NETWORK_IO_ERROR:
+    case SYNC_SERVER_ERROR:
+      return true;
+
+    // The server thinks we should try again later.
+    case TRANSIENT_ERROR:
+      return true;
+
+    default:
+      NOTREACHED();
+      return false;
   }
+}
+
+bool SyncSession::ExperiencedTransientError() const {
+  const StatusController* status = status_controller_.get();
+
+  // In the future, we should be able to report more about the cause of the
+  // errors and return an enum value.  Right now that's not possible because
+  // our error detection is based on state that can tell us only that an
+  // error exists, but not much else.
+
+  // We were unable to download all updates, for some unknown reason.
+  if (status->num_server_changes_remaining() > 0)
+    return true;
+
+  // Did we run into any temporary network or server issues?
+  if (IndicatesTransientError(status->last_download_updates_result()) ||
+      IndicatesTransientError(status->last_post_commit_result()) ||
+      IndicatesTransientError(status->last_clear_data_result())) {
+    return true;
+  }
+
+  return false;
+}
 
 }  // namespace sessions
 }  // namespace browser_sync