Revert of Sync commit errors should temporarily re-enable trigger pre-commit getupdates (patchset #2 id:20001 of https://codereview.chromium.org/905853002/)

Reason for revert:
Looks like this broke the build

Undefined symbols for architecture armv7:
  "syncer::sessions::test_util::SimulateDownloadUpdatesFailed(syncer::EnumSet<syncer::ModelType, (syncer::ModelType)2, (syncer::ModelType)33>, syncer::sessions::NudgeTracker const&, syncer::sessions::SyncSession*)", referenced from:
      syncer::BackoffTriggersSyncSchedulerTest_FailDownloadOnceThenSucceed_Test::TestBody() in sync_scheduler_unittest.o
      syncer::BackoffTriggersSyncSchedulerTest_FailDownloadTwice_Test::TestBody() in sync_scheduler_unittest.o
      syncer::SyncSchedulerTest_FailedRetry_Test::TestBody() in sync_scheduler_unittest.o

Original issue's description:
> Sync commit errors should temporarily re-enable
> trigger pre-commit getupdates
>
> Add a boolean variable in class DataTypeTracker, so every
> time when a data type got conflict response from server
> during commit, will set that boolean variable to true, then
> next time sync, GetUpdate will check the boolean to see if
> need to GetUpdate to resolve conflict locally.
>
> BUG=324893
>
> Committed: https://crrev.com/21f43c5af27e24c34565df26bb51fcc704c97597
> Cr-Commit-Position: refs/heads/master@{#315339}

TBR=zea@chromium.org,stanisc@chromium.org,maniscalco@chromium.org,pvalenzuela@chromium.org,gangwu@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=324893

Review URL: https://codereview.chromium.org/891123003

Cr-Commit-Position: refs/heads/master@{#315346}

7 files changed, 31 insertions, 50 deletions

diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc
index 0561e9b..509e08d 100644
--- a/sync/engine/commit.cc
+++ b/sync/engine/commit.cc
@@ -83,7 +83,6 @@ Commit* Commit::Init(
 }
 
 SyncerError Commit::PostAndProcessResponse(
-    sessions::NudgeTracker* nudge_tracker,
     sessions::SyncSession* session,
     sessions::StatusController* status,
     ExtensionsActivity* extensions_activity) {
@@ -154,9 +153,6 @@ SyncerError Commit::PostAndProcessResponse(
                  "type", ModelTypeToString(it->first));
     SyncerError type_result =
         it->second->ProcessCommitResponse(response_, status);
-    if (type_result == SERVER_RETURN_CONFLICT) {
-      nudge_tracker->RecordCommitConflict(it->first);
-    }
     if (processing_result == SYNCER_OK && type_result != SYNCER_OK) {
       processing_result = type_result;
     }
diff --git a/sync/engine/commit.h b/sync/engine/commit.h
index ee81577..993bd872 100644
--- a/sync/engine/commit.h
+++ b/sync/engine/commit.h
@@ -14,7 +14,6 @@
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/util/syncer_error.h"
 #include "sync/protocol/sync.pb.h"
-#include "sync/sessions/nudge_tracker.h"
 #include "sync/util/extensions_activity.h"
 
 namespace syncer {
@@ -55,10 +54,10 @@ class SYNC_EXPORT_PRIVATE Commit {
       CommitProcessor* commit_processor,
       ExtensionsActivity* extensions_activity);
 
-  SyncerError PostAndProcessResponse(sessions::NudgeTracker* nudge_tracker,
-                                     sessions::SyncSession* session,
-                                     sessions::StatusController* status,
-                                     ExtensionsActivity* extensions_activity);
+  SyncerError PostAndProcessResponse(
+      sessions::SyncSession* session,
+      sessions::StatusController* status,
+      ExtensionsActivity* extensions_activity);
 
   // Cleans up state associated with this commit.  Must be called before the
   // destructor.
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index 6f76be0..d279fd0 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -465,7 +465,9 @@ void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) {
            << ModelTypeSetToString(session_context_->GetEnabledTypes());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
   bool premature_exit = !syncer_->NormalSyncShare(
-      GetEnabledAndUnthrottledTypes(), &nudge_tracker_, session.get());
+      GetEnabledAndUnthrottledTypes(),
+      nudge_tracker_,
+      session.get());
   AdjustPolling(FORCE_RESET);
   // Don't run poll job till the next time poll timer fires.
   do_poll_after_credentials_updated_ = false;
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 90b20a0..21d7f28 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -44,10 +44,9 @@ using sync_pb::GetUpdatesCallerInfo;
 class MockSyncer : public Syncer {
  public:
   MockSyncer();
-  MOCK_METHOD3(NormalSyncShare,
-               bool(ModelTypeSet,
-                    sessions::NudgeTracker*,
-                    sessions::SyncSession*));
+  MOCK_METHOD3(NormalSyncShare, bool(ModelTypeSet,
+                                     const sessions::NudgeTracker&,
+                                     sessions::SyncSession*));
   MOCK_METHOD3(ConfigureSyncShare,
                bool(ModelTypeSet,
                     sync_pb::GetUpdatesCallerInfo::GetUpdatesSource,
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index ef759b8..bae809e 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -55,13 +55,13 @@ bool Syncer::ExitRequested() {
 }
 
 bool Syncer::NormalSyncShare(ModelTypeSet request_types,
-                             NudgeTracker* nudge_tracker,
+                             const NudgeTracker& nudge_tracker,
                              SyncSession* session) {
   HandleCycleBegin(session);
-  if (nudge_tracker->IsGetUpdatesRequired() ||
+  if (nudge_tracker.IsGetUpdatesRequired() ||
       session->context()->ShouldFetchUpdatesBeforeCommit()) {
     VLOG(1) << "Downloading types " << ModelTypeSetToString(request_types);
-    NormalGetUpdatesDelegate normal_delegate(*nudge_tracker);
+    NormalGetUpdatesDelegate normal_delegate(nudge_tracker);
     GetUpdatesProcessor get_updates_processor(
         session->context()->model_type_registry()->update_handler_map(),
         normal_delegate);
@@ -70,18 +70,18 @@ bool Syncer::NormalSyncShare(ModelTypeSet request_types,
             session,
             &get_updates_processor,
             kCreateMobileBookmarksFolder)) {
-      return HandleCycleEnd(session, nudge_tracker->GetLegacySource());
+      return HandleCycleEnd(session, nudge_tracker.GetLegacySource());
     }
   }
 
   VLOG(1) << "Committing from types " << ModelTypeSetToString(request_types);
   CommitProcessor commit_processor(
       session->context()->model_type_registry()->commit_contributor_map());
-  SyncerError commit_result = BuildAndPostCommits(request_types, nudge_tracker,
-                                                  session, &commit_processor);
+  SyncerError commit_result =
+      BuildAndPostCommits(request_types, session, &commit_processor);
   session->mutable_status_controller()->set_commit_result(commit_result);
 
-  return HandleCycleEnd(session, nudge_tracker->GetLegacySource());
+  return HandleCycleEnd(session, nudge_tracker.GetLegacySource());
 }
 
 bool Syncer::ConfigureSyncShare(
@@ -160,7 +160,6 @@ bool Syncer::DownloadAndApplyUpdates(
 }
 
 SyncerError Syncer::BuildAndPostCommits(ModelTypeSet requested_types,
-                                        sessions::NudgeTracker* nudge_tracker,
                                         sessions::SyncSession* session,
                                         CommitProcessor* commit_processor) {
   // The ExitRequested() check is unnecessary, since we should start getting
@@ -181,7 +180,8 @@ SyncerError Syncer::BuildAndPostCommits(ModelTypeSet requested_types,
     }
 
     SyncerError error = commit->PostAndProcessResponse(
-        nudge_tracker, session, session->mutable_status_controller(),
+        session,
+        session->mutable_status_controller(),
         session->context()->extensions_activity());
     commit->CleanUp();
     if (error != SYNCER_OK) {
diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h
index ca93b3608..02e66e3 100644
--- a/sync/engine/syncer.h
+++ b/sync/engine/syncer.h
@@ -48,7 +48,7 @@ class SYNC_EXPORT_PRIVATE Syncer {
   // sync.  The |nudge_tracker| contains state that describes why the client is
   // out of sync and what must be done to bring it back into sync.
   virtual bool NormalSyncShare(ModelTypeSet request_types,
-                               sessions::NudgeTracker* nudge_tracker,
+                               const sessions::NudgeTracker& nudge_tracker,
                                sessions::SyncSession* session);
 
   // Performs an initial download for the |request_types|.  It is assumed that
@@ -79,10 +79,10 @@ class SYNC_EXPORT_PRIVATE Syncer {
   // number of unsynced and ready to commit items reaches zero or an error is
   // encountered.  A request to exit early will be treated as an error and will
   // abort any blocking operations.
-  SyncerError BuildAndPostCommits(ModelTypeSet request_types,
-                                  sessions::NudgeTracker* nudge_tracker,
-                                  sessions::SyncSession* session,
-                                  CommitProcessor* commit_processor);
+  SyncerError BuildAndPostCommits(
+      ModelTypeSet request_types,
+      sessions::SyncSession* session,
+      CommitProcessor* commit_processor);
 
   void HandleCycleBegin(sessions::SyncSession* session);
   bool HandleCycleEnd(
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 5f049ec..471df65 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -262,8 +262,11 @@ class SyncerTest : public testing::Test,
     // Pretend we've seen a local change, to make the nudge_tracker look normal.
     nudge_tracker_.RecordLocalChange(ModelTypeSet(BOOKMARKS));
 
-    EXPECT_TRUE(syncer_->NormalSyncShare(context_->GetEnabledTypes(),
-                                         &nudge_tracker_, session_.get()));
+    EXPECT_TRUE(
+        syncer_->NormalSyncShare(
+            context_->GetEnabledTypes(),
+            nudge_tracker_,
+            session_.get()));
   }
 
   void SyncShareConfigure() {
@@ -639,7 +642,8 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
   ResetSession();
   syncer_->NormalSyncShare(
       Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)),
-      &nudge_tracker_, session_.get());
+      nudge_tracker_,
+      session_.get());
 
   {
     // Nothing should have been committed as bookmarks is throttled.
@@ -2863,25 +2867,6 @@ TEST_F(SyncerTest, SendDebugInfoEventsOnGetUpdates_PostFailsDontDrop) {
   EXPECT_EQ(0, mock_server_->last_request().debug_info().events_size());
 }
 
-// Tests that commit failure with conflict will trigger GetUpdates for next
-// sycle of sync
-TEST_F(SyncerTest, CommitFailureWithConflict) {
-  ConfigureNoGetUpdatesRequired();
-  CreateUnsyncedDirectory("X", "id_X");
-  EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
-
-  SyncShareNudge();
-  EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
-
-  CreateUnsyncedDirectory("Y", "id_Y");
-  mock_server_->set_conflict_n_commits(1);
-  SyncShareNudge();
-  EXPECT_TRUE(nudge_tracker_.IsGetUpdatesRequired());
-
-  nudge_tracker_.RecordSuccessfulSyncCycle();
-  EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
-}
-
 // Tests that sending debug info events on Commit works.
 TEST_F(SyncerTest, SendDebugInfoEventsOnCommit_HappyCase) {
   // Make sure GetUpdate isn't call as it would "steal" debug info events before