FYI: Remove PerModelSafeGroupState + move ConflictResolution

sync/engine/syncer_unittest.cc

 // Copyright (c) 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 87 matching lines @@
 using syncable::SERVER_PARENT_ID;
 using syncable::SERVER_POSITION_IN_PARENT;
 using syncable::SERVER_SPECIFICS;
 using syncable::SERVER_VERSION;
 using syncable::UNIQUE_CLIENT_TAG;
 using syncable::UNIQUE_SERVER_TAG;
 using syncable::SPECIFICS;
 using syncable::SYNCING;
 using syncable::UNITTEST;
 
-using sessions::ConflictProgress;
 using sessions::ScopedSetSessionWriteTransaction;
 using sessions::StatusController;
 using sessions::SyncSessionContext;
 using sessions::SyncSession;
 
 class SyncerTest : public testing::Test,
                    public SyncSession::Delegate,
                    public SyncEngineEventListener {
  protected:
   SyncerTest()
@@ skipping 59 matching lines @@
     std::vector<ModelSafeWorker*> workers;
     GetModelSafeRoutingInfo(&info);
     GetWorkers(&workers);
     ModelTypeStateMap types =
         ModelSafeRoutingInfoToStateMap(info, std::string());
     return new SyncSession(context_.get(), this,
         sessions::SyncSourceInfo(sync_pb::GetUpdatesCallerInfo::UNKNOWN, types),
         info, workers);
   }
 
-  bool SyncShareAsDelegate(
+  void SyncShareAsDelegate(
       SyncSchedulerImpl::SyncSessionJob::SyncSessionJobPurpose purpose) {
     SyncerStep start;
     SyncerStep end;
     SyncSchedulerImpl::SetSyncerStepsForPurpose(purpose, &start, &end);
 
     session_.reset(MakeSession());
     syncer_->SyncShare(session_.get(), start, end);
-    return session_->HasMoreToSync();
   }
 
-  bool SyncShareNudge() {
+  void SyncShareNudge() {
     session_.reset(MakeSession());
-    return SyncShareAsDelegate(SyncSchedulerImpl::SyncSessionJob::NUDGE);
+    SyncShareAsDelegate(SyncSchedulerImpl::SyncSessionJob::NUDGE);
   }
 
-  bool SyncShareConfigure() {
+  void SyncShareConfigure() {
     session_.reset(MakeSession());
-    return SyncShareAsDelegate(
-        SyncSchedulerImpl::SyncSessionJob::CONFIGURATION);
-  }
-
-  void LoopSyncShare() {
-    bool should_loop = false;
-    int loop_iterations = 0;
-    do {
-      ASSERT_LT(++loop_iterations, 100) << "infinite loop detected. please fix";
-      should_loop = SyncShareNudge();
-    } while (should_loop);
+    SyncShareAsDelegate(SyncSchedulerImpl::SyncSessionJob::CONFIGURATION);
   }
 
   virtual void SetUp() {
     dir_maker_.SetUp();
     mock_server_.reset(new MockConnectionManager(directory()));
     EnableDatatype(BOOKMARKS);
     EnableDatatype(NIGORI);
     EnableDatatype(PREFERENCES);
     EnableDatatype(NIGORI);
     worker_ = new FakeModelWorker(GROUP_PASSIVE);
@@ skipping 156 matching lines @@
             case MOVED_FROM_ROOT:
               entry.Put(SERVER_PARENT_ID, trans.root_id());
               break;
             default:
               FAIL() << "Bad value in CommitOrderingTest list";
           }
         }
         test++;
       }
     }
-    LoopSyncShare();
+    SyncShareNudge();
     ASSERT_TRUE(expected_positions.size() ==
                 mock_server_->committed_ids().size());
     // If this test starts failing, be aware other sort orders could be valid.
     for (size_t i = 0; i < expected_positions.size(); ++i) {
       EXPECT_EQ(1u, expected_positions.count(i));
       EXPECT_TRUE(expected_positions[i] == mock_server_->committed_ids()[i]);
     }
   }
 
   void DoTruncationTest(const vector<int64>& unsynced_handle_view,
@@ skipping 1294 matching lines @@
   // because any string sent by the server is a legal server ID in the sync
   // protocol, but it's not the ID of any item known to the client.  This
   // update should succeed validation, but be stuck in the unapplied state
   // until an item with the server ID "-80" arrives.
   mock_server_->AddUpdateDirectory(3, -80, "bad_parent", 10, 10);
 
   SyncShareNudge();
 
   // Id 3 should be in conflict now.
   EXPECT_EQ(1, status().TotalNumConflictingItems());
-  {
-    sessions::ScopedModelSafeGroupRestriction r(
-        session_->mutable_status_controller(), GROUP_PASSIVE);
-    ASSERT_TRUE(status().conflict_progress());
-    EXPECT_EQ(1, status().conflict_progress()->HierarchyConflictingItemsSize());
-  }
+  EXPECT_EQ(1, status().num_hierarchy_conflicts());
 
   // These entries will be used in the second set of updates.
   mock_server_->AddUpdateDirectory(4, 0, "newer_version", 20, 10);
   mock_server_->AddUpdateDirectory(5, 0, "circular1", 10, 10);
   mock_server_->AddUpdateDirectory(6, 5, "circular2", 10, 10);
   mock_server_->AddUpdateDirectory(9, 3, "bad_parent_child", 10, 10);
   mock_server_->AddUpdateDirectory(100, 9, "bad_parent_child2", 10, 10);
   mock_server_->AddUpdateDirectory(10, 0, "dir_to_bookmark", 10, 10);
 
   SyncShareNudge();
   // The three items with an unresolved parent should be unapplied (3, 9, 100).
   // The name clash should also still be in conflict.
   EXPECT_EQ(3, status().TotalNumConflictingItems());
-  {
-    sessions::ScopedModelSafeGroupRestriction r(
-        session_->mutable_status_controller(), GROUP_PASSIVE);
-    ASSERT_TRUE(status().conflict_progress());
-    EXPECT_EQ(3, status().conflict_progress()->HierarchyConflictingItemsSize());
-  }
+  EXPECT_EQ(3, status().num_hierarchy_conflicts());
 
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     // Even though it has the same name, it should work.
     Entry name_clash(&trans, GET_BY_ID, ids_.FromNumber(2));
     ASSERT_TRUE(name_clash.good());
     EXPECT_FALSE(name_clash.Get(IS_UNAPPLIED_UPDATE))
         << "Duplicate name SHOULD be OK.";
 
     Entry bad_parent(&trans, GET_BY_ID, ids_.FromNumber(3));
@@ skipping 69 matching lines @@
     Entry circular_parent_target(&trans, GET_BY_ID, ids_.FromNumber(6));
     ASSERT_TRUE(circular_parent_target.good());
     EXPECT_FALSE(circular_parent_target.Get(IS_UNAPPLIED_UPDATE));
     EXPECT_TRUE(circular_parent_issue.Get(ID) ==
         circular_parent_target.Get(PARENT_ID));
     EXPECT_EQ(10u, circular_parent_target.Get(BASE_VERSION));
   }
 
   EXPECT_FALSE(saw_syncer_event_);
   EXPECT_EQ(4, status().TotalNumConflictingItems());
-  {
-    sessions::ScopedModelSafeGroupRestriction r(
-        session_->mutable_status_controller(), GROUP_PASSIVE);
-    ASSERT_TRUE(status().conflict_progress());
-    EXPECT_EQ(4, status().conflict_progress()->HierarchyConflictingItemsSize());
-  }
+  EXPECT_EQ(4, status().num_hierarchy_conflicts());
 }
 
 TEST_F(SyncerTest, CommitTimeRename) {
   int64 metahandle_folder;
   int64 metahandle_new_entry;
 
   // Create a folder and an entry.
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&trans, CREATE, root_id_, "Folder");
@@ skipping 290 matching lines @@
     ASSERT_TRUE(A.good());
     A.Put(IS_UNSYNCED, true);
     A.Put(IS_UNAPPLIED_UPDATE, true);
     A.Put(SERVER_VERSION, 20);
 
     MutableEntry B(&wtrans, GET_BY_ID, ids_.FromNumber(2));
     ASSERT_TRUE(B.good());
     B.Put(IS_UNAPPLIED_UPDATE, true);
     B.Put(SERVER_VERSION, 20);
   }
-  LoopSyncShare();
+  SyncShareNudge();
   saw_syncer_event_ = false;
   mock_server_->set_conflict_all_commits(false);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
 
     Entry A(&trans, GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(A.good());
     EXPECT_TRUE(A.Get(IS_UNSYNCED) == false);
     EXPECT_TRUE(A.Get(IS_UNAPPLIED_UPDATE) == false);
@@ skipping 19 matching lines @@
     ASSERT_TRUE(A.good());
     A.Put(IS_UNSYNCED, true);
     A.Put(IS_UNAPPLIED_UPDATE, true);
     A.Put(SERVER_VERSION, 20);
 
     MutableEntry B(&wtrans, GET_BY_ID, ids_.FromNumber(2));
     ASSERT_TRUE(B.good());
     B.Put(IS_UNAPPLIED_UPDATE, true);
     B.Put(SERVER_VERSION, 20);
   }
-  LoopSyncShare();
+  SyncShareNudge();
   saw_syncer_event_ = false;
   mock_server_->set_conflict_all_commits(false);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
 
     Entry A(&trans, GET_BY_ID, ids_.FromNumber(1));
     ASSERT_TRUE(A.good());
     EXPECT_TRUE(A.Get(IS_UNSYNCED) == false);
     EXPECT_TRUE(A.Get(IS_UNAPPLIED_UPDATE) == false);
     EXPECT_TRUE(A.Get(SERVER_VERSION) == 20);
 
     Entry B(&trans, GET_BY_ID, ids_.FromNumber(2));
     ASSERT_TRUE(B.good());
     EXPECT_TRUE(B.Get(IS_UNSYNCED) == false);
     EXPECT_TRUE(B.Get(IS_UNAPPLIED_UPDATE) == false);
     EXPECT_TRUE(B.Get(SERVER_VERSION) == 20);
   }
 }
 
 TEST_F(SyncerTest, ReverseFolderOrderingTest) {
   mock_server_->AddUpdateDirectory(4, 3, "ggchild", 10, 10);
   mock_server_->AddUpdateDirectory(3, 2, "gchild", 10, 10);
   mock_server_->AddUpdateDirectory(5, 4, "gggchild", 10, 10);
   mock_server_->AddUpdateDirectory(2, 1, "child", 10, 10);
   mock_server_->AddUpdateDirectory(1, 0, "parent", 10, 10);
-  LoopSyncShare();
+  SyncShareNudge();
   syncable::ReadTransaction trans(FROM_HERE, directory());
 
   Id child_id = GetOnlyEntryWithName(
         &trans, ids_.FromNumber(4), "gggchild");
   Entry child(&trans, GET_BY_ID, child_id);
   ASSERT_TRUE(child.good());
 }
 
 class EntryCreatedInNewFolderTest : public SyncerTest {
  public:
@@ skipping 100 matching lines @@
     parent.Put(syncable::BASE_VERSION, 5);
     parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     MutableEntry child(&wtrans, syncable::CREATE, parent_id_, "Pete.htm");
     ASSERT_TRUE(child.good());
     child.Put(syncable::ID, child_id_);
     child.Put(syncable::BASE_VERSION, 10);
     WriteTestDataToEntry(&wtrans, &child);
   }
   mock_server_->AddUpdateBookmark(child_id_, parent_id_, "Pete2.htm", 11, 10);
   mock_server_->set_conflict_all_commits(true);
-  LoopSyncShare();
+  SyncShareNudge();
   syncable::Directory::ChildHandles children;
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     directory()->GetChildHandlesById(&trans, parent_id_, &children);
     // We expect the conflict resolver to preserve the local entry.
     Entry child(&trans, syncable::GET_BY_ID, child_id_);
     ASSERT_TRUE(child.good());
     EXPECT_TRUE(child.Get(syncable::IS_UNSYNCED));
     EXPECT_FALSE(child.Get(syncable::IS_UNAPPLIED_UPDATE));
     EXPECT_TRUE(child.Get(SPECIFICS).has_bookmark());
@@ skipping 170 matching lines @@
     MutableEntry existing(&trans, GET_BY_HANDLE, existing_metahandle);
     ASSERT_TRUE(existing.good());
     existing.Put(PARENT_ID, newfolder.Get(ID));
     existing.Put(IS_UNSYNCED, true);
     EXPECT_TRUE(existing.Get(ID).ServerKnows());
 
     newfolder.Put(IS_DEL, true);
     existing.Put(IS_DEL, true);
   }
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
-  EXPECT_EQ(0, status().TotalNumServerConflictingItems());
+  EXPECT_EQ(0, status().num_server_conflicts());
 }
 
 TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
   int64 newfolder_metahandle;
 
   mock_server_->AddUpdateDirectory(1, 0, "bob", 1, 10);
   SyncShareNudge();
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, directory());
     MutableEntry newfolder(&trans, CREATE, ids_.FromNumber(1), "local");
@@ skipping 88 matching lines @@
       string nameutf8 = base::StringPrintf("%d", i);
       string name(nameutf8.begin(), nameutf8.end());
       MutableEntry e(&trans, CREATE, trans.root_id(), name);
       e.Put(IS_UNSYNCED, true);
       e.Put(IS_DIR, true);
       e.Put(SPECIFICS, DefaultBookmarkSpecifics());
     }
   }
   ASSERT_EQ(items_to_commit, directory()->unsynced_entity_count());
 
-  EXPECT_FALSE(SyncShareNudge());
+  SyncShareNudge();
   EXPECT_EQ(num_batches, mock_server_->commit_messages().size());
   EXPECT_EQ(0, directory()->unsynced_entity_count());
 }
 
 // Test that a single failure to contact the server will cause us to exit the
 // commit loop immediately.
 TEST_F(SyncerTest, CommitManyItemsInOneGo_PostBufferFail) {
   uint32 num_batches = 3;
   uint32 items_to_commit = kDefaultMaxCommitBatchSize * num_batches;
   {
@@ skipping 519 matching lines @@
     EXPECT_TRUE(child.good());
     EXPECT_TRUE(child.Get(IS_UNAPPLIED_UPDATE));
     EXPECT_TRUE(child.Get(IS_DEL));
     EXPECT_FALSE(child.Get(IS_UNSYNCED));
   }
 
   // And finally the folder.
   mock_server_->AddUpdateDirectory(folder_id,
       TestIdFactory::root(), "folder", 1, 1);
   mock_server_->SetChangesRemaining(0);
-  LoopSyncShare();
-  LoopSyncShare();
+  SyncShareNudge();
+  SyncShareNudge();
   // Check that everything is as expected after the commit.
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
     Entry entry(&trans, GET_BY_ID, folder_id);
     ASSERT_TRUE(entry.good());
     Entry child(&trans, GET_BY_ID, stuck_entry_id);
     EXPECT_EQ(entry.Get(ID), child.Get(PARENT_ID));
     EXPECT_EQ("stuck", child.Get(NON_UNIQUE_NAME));
     EXPECT_TRUE(child.good());
   }
@@ skipping 1601 matching lines @@
 
 TEST_F(SyncerPositionTiebreakingTest, MidLowHigh) {
   Add(mid_id_);
   Add(low_id_);
   Add(high_id_);
   SyncShareNudge();
   ExpectLocalOrderIsByServerId();
 }
 
 }  // namespace syncer