Sync commit errors should temporarily re-enable trigger pre-commit getupdates

sync/engine/syncer_unittest.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Syncer unit tests. Unfortunately a lot of these tests
 // are outdated and need to be reworked and updated.
 
 #include <algorithm>
 #include <limits>
 #include <list>
@@ skipping 244 matching lines @@
   void ResetSession() {
     session_.reset(SyncSession::Build(context_.get(), this));
   }
 
   void SyncShareNudge() {
     ResetSession();
 
     // 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() {
     ResetSession();
     EXPECT_TRUE(syncer_->ConfigureSyncShare(
             context_->GetEnabledTypes(),
             sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
             session_.get()));
   }
 
@@ skipping 355 matching lines @@
     A.PutSpecifics(bookmark_data);
     A.PutNonUniqueName("bookmark");
   }
 
   // Now sync without enabling bookmarks.
   mock_server_->ExpectGetUpdatesRequestTypes(
       Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)));
   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.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     Entry entryA(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(entryA.good());
     EXPECT_TRUE(entryA.GetIsUnsynced());
   }
 
   // Sync again with bookmarks enabled.
@@ skipping 2203 matching lines @@
   EXPECT_EQ(2, mock_server_->last_request().debug_info().events_size());
 
   // The previous send was successful so this next one shouldn't generate any
   // debug info events.
   SyncShareNudge();
   EXPECT_EQ(3U, mock_server_->requests().size());
   ASSERT_TRUE(mock_server_->last_request().has_get_updates());
   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
   // Commit has a chance to send them.
   ConfigureNoGetUpdatesRequired();
 
   // Generate a debug info event and trigger a commit.
   debug_info_getter_->AddDebugEvent();
   CreateUnsyncedDirectory("X", "id_X");
   SyncShareNudge();
@@ skipping 2465 matching lines @@
     EXPECT_EQ("xyz", final_monitor_records["xyz"].extension_id);
     EXPECT_EQ(2049U, final_monitor_records["ABC"].bookmark_write_count);
     EXPECT_EQ(4U,    final_monitor_records["xyz"].bookmark_write_count);
   } else {
     EXPECT_TRUE(final_monitor_records.empty())
         << "Should not restore records after successful bookmark commit.";
   }
 }
 
 }  // namespace syncer