Refactor following sync commit loop change

This change includes some cleanups of the code introduced in r139519.
They have been kept separate from that CL in the hopes of making both
CLs easiser to read.

This commit moves some error-detection functionality from
ProcessCommitResponse's ModelNeutralExecuteImpl() into
BuildAndPostCommits().  This simplifies some of the error handling and
allows us to remove ModelChangingSyncerCommand's
ModelNeutralExecuteImpl().

This CL also combines both commit error indicators into a single
variable.

BUG=91696,36594
TEST=


Review URL: https://chromiumcodereview.appspot.com/10523003

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

5 files changed, 10 insertions, 40 deletions

diff --git a/sync/sessions/status_controller.cc b/sync/sessions/status_controller.cc
index 8f6c9ee..0e8e6a6 100644
--- a/sync/sessions/status_controller.cc
+++ b/sync/sessions/status_controller.cc
@@ -169,21 +169,8 @@ void StatusController::set_last_download_updates_result(
   shared_.error.mutate()->last_download_updates_result = result;
 }
 
-void StatusController::set_last_post_commit_result(const SyncerError result) {
-  shared_.error.mutate()->last_post_commit_result = result;
-}
-
-SyncerError StatusController::last_post_commit_result() const {
-  return shared_.error.value().last_post_commit_result;
-}
-
-void StatusController::set_last_process_commit_response_result(
-    const SyncerError result) {
-  shared_.error.mutate()->last_process_commit_response_result = result;
-}
-
-SyncerError StatusController::last_process_commit_response_result() const {
-  return shared_.error.value().last_process_commit_response_result;
+void StatusController::set_commit_result(const SyncerError result) {
+  shared_.error.mutate()->commit_result = result;
 }
 
 void StatusController::update_conflicts_resolved(bool resolved) {
diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h
index d91efec..ebb77ba 100644
--- a/sync/sessions/status_controller.h
+++ b/sync/sessions/status_controller.h
@@ -157,9 +157,6 @@ class StatusController {
     return ActiveGroupRestrictionIncludesModel(syncable::BOOKMARKS);
   }
 
-  SyncerError last_post_commit_result() const;
-  SyncerError last_process_commit_response_result() const;
-
   // A toolbelt full of methods for updating counters and flags.
   void set_num_server_changes_remaining(int64 changes_remaining);
   void set_invalid_store(bool invalid_store);
@@ -174,8 +171,7 @@ class StatusController {
   void increment_num_server_overwrites();
   void set_sync_protocol_error(const SyncProtocolError& error);
   void set_last_download_updates_result(const SyncerError result);
-  void set_last_post_commit_result(const SyncerError result);
-  void set_last_process_commit_response_result(const SyncerError result);
+  void set_commit_result(const SyncerError result);
 
   void update_conflicts_resolved(bool resolved);
   void reset_conflicts_resolved();
diff --git a/sync/sessions/status_controller_unittest.cc b/sync/sessions/status_controller_unittest.cc
index 9f38e80..2e0f44d 100644
--- a/sync/sessions/status_controller_unittest.cc
+++ b/sync/sessions/status_controller_unittest.cc
@@ -66,12 +66,8 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   status.set_last_download_updates_result(SYNCER_OK);
   EXPECT_EQ(SYNCER_OK, status.error().last_download_updates_result);
 
-  status.set_last_post_commit_result(SYNC_AUTH_ERROR);
-  EXPECT_EQ(SYNC_AUTH_ERROR, status.error().last_post_commit_result);
-
-  status.set_last_process_commit_response_result(SYNC_SERVER_ERROR);
-  EXPECT_EQ(SYNC_SERVER_ERROR,
-            status.error().last_process_commit_response_result);
+  status.set_commit_result(SYNC_AUTH_ERROR);
+  EXPECT_EQ(SYNC_AUTH_ERROR, status.error().commit_result);
 
   for (int i = 0; i < 14; i++)
     status.increment_num_successful_commits();
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index ac874d1..36bc2ca 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -235,12 +235,8 @@ bool IsError(SyncerError error) {
 bool HadErrors(const ErrorCounters& error) {
   const bool download_updates_error =
       IsError(error.last_download_updates_result);
-  const bool post_commit_error = IsError(error.last_post_commit_result);
-  const bool process_commit_response_error =
-      IsError(error.last_process_commit_response_result);
-  return download_updates_error ||
-         post_commit_error ||
-         process_commit_response_error;
+  const bool commit_error = IsError(error.commit_result);
+  return download_updates_error || commit_error;
 }
 }  // namespace
 
@@ -251,9 +247,7 @@ bool SyncSession::Succeeded() const {
 
 bool SyncSession::SuccessfullyReachedServer() const {
   const ErrorCounters& error = status_controller_->error();
-  bool reached_server = error.last_download_updates_result == SYNCER_OK ||
-                        error.last_post_commit_result == SYNCER_OK ||
-                        error.last_process_commit_response_result == SYNCER_OK;
+  bool reached_server = error.last_download_updates_result == SYNCER_OK;
   // It's possible that we reached the server on one attempt, then had an error
   // on the next (or didn't perform some of the server-communicating commands).
   // We want to verify that, for all commands attempted, we successfully spoke
diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc
index 701072e..a54c8f00 100644
--- a/sync/sessions/test_util.cc
+++ b/sync/sessions/test_util.cc
@@ -22,7 +22,7 @@ void SimulateDownloadUpdatesFailed(sessions::SyncSession* session,
 
 void SimulateCommitFailed(sessions::SyncSession* session,
                           SyncerStep begin, SyncerStep end) {
-  session->mutable_status_controller()->set_last_post_commit_result(
+  session->mutable_status_controller()->set_commit_result(
       SERVER_RETURN_TRANSIENT_ERROR);
 }
 
@@ -42,10 +42,7 @@ void SimulateSuccess(sessions::SyncSession* session,
   if (end == SYNCER_END) {
     session->mutable_status_controller()->set_last_download_updates_result(
         SYNCER_OK);
-    session->mutable_status_controller()->set_last_post_commit_result(
-        SYNCER_OK);
-    session->mutable_status_controller()->
-        set_last_process_commit_response_result(SYNCER_OK);
+    session->mutable_status_controller()->set_commit_result(SYNCER_OK);
   }
 }