Add return values to SyncerCommand

chrome/browser/sync/engine/process_commit_response_command.cc

Index: chrome/browser/sync/engine/process_commit_response_command.cc
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index a4c3291e041101184f59b8bb64db0d600eef49c2..395088017e2b33762ba6c5eebbfc8e44d3f27240 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -81,13 +81,13 @@ std::set<ModelSafeGroup> ProcessCommitResponseCommand::GetGroupsToChange(
   return groups_with_commits;
 }
 
-bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
+SyncerError ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
     sessions::SyncSession* session) {
   ScopedDirLookup dir(session->context()->directory_manager(),
                       session->context()->account_name());
   if (!dir.good()) {
     LOG(ERROR) << "Scoped dir lookup failed!";
-    return false;
+    return DIRECTORY_LOOKUP_FAILED;
   }
 
   const StatusController& status = session->status_controller();
@@ -98,7 +98,7 @@ bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
     // TODO(sync): What if we didn't try to commit anything?
     LOG(WARNING) << "Commit response has no commit body!";
     IncrementErrorCounters(session->mutable_status_controller());
-    return false;
+    return SERVER_RESPONSE_VALIDATION_FAILED;
   }
 
   const CommitResponse& cr = response.commit();
@@ -113,14 +113,14 @@ bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
         LOG(ERROR) << "  " << cr.entryresponse(i).error_message();
     }
     IncrementErrorCounters(session->mutable_status_controller());
-    return false;
+    return SERVER_RESPONSE_VALIDATION_FAILED;
   }
-  return true;
+  return NO_ERROR;
 }
 
-void ProcessCommitResponseCommand::ModelChangingExecuteImpl(
+SyncerError ProcessCommitResponseCommand::ModelChangingExecuteImpl(
     SyncSession* session) {
-  ProcessCommitResponse(session);
+  SyncerError result = ProcessCommitResponse(session);
   ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor();
   if (session->status_controller().HasBookmarkCommitActivity() &&
       session->status_controller().syncer_status()
@@ -128,17 +128,16 @@ void ProcessCommitResponseCommand::ModelChangingExecuteImpl(
     monitor->PutRecords(session->extensions_activity());
     session->mutable_extensions_activity()->clear();
   }
+  return result;
 }
 
-void ProcessCommitResponseCommand::ProcessCommitResponse(
+SyncerError ProcessCommitResponseCommand::ProcessCommitResponse(
     SyncSession* session) {
-  // TODO(sync): This function returns if it sees problems. We probably want
-  // to flag the need for an update or similar.
   ScopedDirLookup dir(session->context()->directory_manager(),
                       session->context()->account_name());
   if (!dir.good()) {
     LOG(ERROR) << "Scoped dir lookup failed!";
-    return;
+    return DIRECTORY_LOOKUP_FAILED;
   }
 
   StatusController* status = session->mutable_status_controller();
@@ -189,10 +188,6 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
           // We handle over quota like a retry, which is same as transient.
         case CommitResponse::RETRY:
         case CommitResponse::TRANSIENT_ERROR:
-          // TODO(tim): Now that we have SyncSession::Delegate, we
-          // should plumb this directly for exponential backoff purposes rather
-          // than trying to infer from HasMoreToSync(). See
-          // SyncerThread::CalculatePollingWaitTime.
           ++transient_error_commits;
           break;
         default:
@@ -202,6 +197,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
   }
 
   // TODO(sync): move status reporting elsewhere.
+  int commit_count = static_cast<int>(proj.size());

[lipalani1, 2012/01/05 22:59:41]

Why was this moved up here?

[rlarocque, 2012/01/06 00:24:31]

Probably rebase conflicts.  Will undo.  

   status->increment_num_conflicting_commits_by(conflicting_commits);
   if (0 == successes) {
     status->increment_num_consecutive_transient_error_commits_by(
@@ -211,14 +207,26 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
     status->set_num_consecutive_transient_error_commits(0);
     status->set_num_consecutive_errors(0);
   }
-  int commit_count = static_cast<int>(proj.size());
   if (commit_count != (conflicting_commits + error_commits +
                        transient_error_commits)) {
     ResetErrorCounters(status);
   }
   SyncerUtil::MarkDeletedChildrenSynced(dir, &deleted_folders);
 
-  return;
+  if (commit_count == (successes + conflicting_commits)) {
+    // We consider conflicting commits as a success because things will work out
+    // on their own when we receive them.  Flags will be set so that
+    // HasMoreToSync() will cause SyncScheduler to enter another sync cycle to
+    // handle this condition.
+    return NO_ERROR;
+  } else if (error_commits > 0) {
+    return SERVER_RETURN_UNKNOWN_ERROR;
+  } else if (transient_error_commits > 0) {
+    return SERVER_RETURN_TRANSIENT_ERROR;
+  } else {
+    LOG(FATAL) << "Inconsistent counts when processing commit response";
+    return NO_ERROR;
+  }
 }
 
 void LogServerError(const CommitResponse_EntryResponse& res) {