Add return values to SyncerCommand

chrome/browser/sync/engine/model_changing_syncer_command.cc

Index: chrome/browser/sync/engine/model_changing_syncer_command.cc
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc
index 1c392cbd20877ecfc27fa47abddc076dd2627361..da59a5117b361c0537de28a4922fbaf3528c8f61 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.cc
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc
@@ -9,14 +9,16 @@
 #include "base/bind_helpers.h"
 #include "chrome/browser/sync/sessions/status_controller.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/util/unrecoverable_error_info.h"
 
 namespace browser_sync {
 
-void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
+SyncerError ModelChangingSyncerCommand::ExecuteImpl(
+    sessions::SyncSession* session) {
   work_session_ = session;
-  if (!ModelNeutralExecuteImpl(work_session_)) {
-    return;
+  SyncerError result = ModelNeutralExecuteImpl(work_session_);
+
+  if (result != NO_ERROR) {

[tim (not reviewing), 2012/01/06 16:33:42]

In general most of the sync code uses no-brace-on-single-line if. The only
"rule" is to at least make files consistent with themselves. So please change
either this one or your new code on line 48.

[rlarocque, 2012/01/06 19:44:21]

Done.  Removed braces from this if statement.

+    return result;
   }
 
   const std::set<ModelSafeGroup>& groups_to_change =
@@ -40,15 +42,19 @@ void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
         // unretained.
         base::Unretained(this));
 
-    // TODO(lipalani): Check the return value for an unrecoverable error.
-    ignore_result(worker->DoWorkAndWaitUntilDone(c));
-
+    SyncerError this_worker_result = worker->DoWorkAndWaitUntilDone(c);
+    // TODO(rlarocque): Figure out a better way to deal with errors from
+    // multiple models at once.

[tim (not reviewing), 2012/01/06 16:33:42]

Indeed.  Can you file a bug to track this?

[rlarocque, 2012/01/06 19:44:21]

Done.  I've opened crbug.com/109422 to track this.  I probably should have
opened a bug like that long ago.  It's getting harder to relate this patch to
crbug.com/94670.

See also: Lingesh's comment in the previous version of this patch.

+    if (this_worker_result != NO_ERROR)
+      result = this_worker_result;
   }
+
+  return result;
 }
 
-bool ModelChangingSyncerCommand::ModelNeutralExecuteImpl(
+SyncerError ModelChangingSyncerCommand::ModelNeutralExecuteImpl(
     sessions::SyncSession* session) {
-  return true;
+  return NO_ERROR;
 }
 
 }  // namespace browser_sync