check if ModelNeutralExecuteImpl succeeded before running ModelChangingExecuteImpl

BUG:36200
TEST:manual

Review URL: http://codereview.chromium.org/646078

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39412 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 13 insertions, 7 deletions

diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc
index e6d34a2..f6e9dad 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.cc
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc
@@ -12,7 +12,9 @@ namespace browser_sync {
 
 void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
   work_session_ = session;
-  ModelNeutralExecuteImpl(work_session_);
+  if (!ModelNeutralExecuteImpl(work_session_)) {
+    return;
+  }
 
   for (size_t i = 0; i < session->workers().size(); ++i) {
     ModelSafeWorker* worker = session->workers()[i];
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.h b/chrome/browser/sync/engine/model_changing_syncer_command.h
index eb8003a..f0ee028 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.h
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.h
@@ -39,7 +39,10 @@ class ModelChangingSyncerCommand : public SyncerCommand {
   // belonging to multiple ModelSafeGroups, but in a way that is known to be
   // safe.  This will be called once, prior to ModelChangingExecuteImpl,
   // *without* a ModelSafeGroup restriction in place on the SyncSession.
-  virtual void ModelNeutralExecuteImpl(sessions::SyncSession* session) {}
+  // Returns true on success, false on failure.
+  virtual bool ModelNeutralExecuteImpl(sessions::SyncSession* session) {
+    return true;
+  }
 
   // Abstract method to be implemented by subclasses to handle logic that
   // operates on the model.  This is invoked with a SyncSession ModelSafeGroup
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index 33b188e..0db5def 100755
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -55,13 +55,13 @@ void ResetErrorCounters(StatusController* status) {
 ProcessCommitResponseCommand::ProcessCommitResponseCommand() {}
 ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {}
 
-void ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
+bool 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;
+    return false;
   }
 
   StatusController* status = session->status_controller();
@@ -72,7 +72,7 @@ void ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
     // TODO(sync): What if we didn't try to commit anything?
     LOG(WARNING) << "Commit response has no commit body!";
     IncrementErrorCounters(status);
-    return;
+    return false;
   }
 
   const CommitResponse& cr = response.commit();
@@ -87,8 +87,9 @@ void ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
         LOG(ERROR) << "  " << cr.entryresponse(i).error_message();
     }
     IncrementErrorCounters(status);
-    return;
+    return false;
   }
+  return true;
 }
 
 void ProcessCommitResponseCommand::ModelChangingExecuteImpl(
diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h
index e83d075..351db1c 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.h
+++ b/chrome/browser/sync/engine/process_commit_response_command.h
@@ -26,7 +26,7 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand {
   virtual ~ProcessCommitResponseCommand();
 
   // ModelChangingSyncerCommand implementation.
-  virtual void ModelNeutralExecuteImpl(sessions::SyncSession* session);
+  virtual bool ModelNeutralExecuteImpl(sessions::SyncSession* session);
   virtual void ModelChangingExecuteImpl(sessions::SyncSession* session);
 
  private: