Delete syncer commit ordering tests

Commit ordering requirements across entities are relevant for bookmarks
only, and no longer implemented within the sync engine. The tests
deleted in this patch exercised the legacy Sync Directory, to be deleted
soon. Equivalent tests, with modern requirements in mind, already exist
in SyncedBookmarkTrackerTest.

Change-Id: I4abf6c5231d545d0bd34d397b0855563821bea18
Bug: 923287
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2257237
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781222}

components/sync/engine_impl/syncer_unittest.cc

@@ -356,94 +356,6 @@ class SyncerTest : public testing::Test,
     AddDefaultFieldValue(PREFERENCES, &result);
     return result;
   }
-  // Enumeration of alterations to entries for commit ordering tests.
-  enum EntryFeature {
-    LIST_END = 0,  // Denotes the end of the list of features from below.
-    SYNCED,        // Items are unsynced by default
-    DELETED,
-    OLD_MTIME,
-    MOVED_FROM_ROOT,
-  };
-
-  struct CommitOrderingTest {
-    // expected commit index.
-    int commit_index;
-    // Details about the item
-    syncable::Id id;
-    syncable::Id parent_id;
-    EntryFeature features[10];
-
-    static CommitOrderingTest MakeLastCommitItem() {
-      CommitOrderingTest last_commit_item;
-      last_commit_item.commit_index = -1;
-      last_commit_item.id = TestIdFactory::root();
-      return last_commit_item;
-    }
-  };
-
-  void RunCommitOrderingTest(CommitOrderingTest* test) {
-    map<int, syncable::Id> expected_positions;
-    {  // Transaction scope.
-      syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-      while (!test->id.IsRoot()) {
-        if (test->commit_index >= 0) {
-          map<int, syncable::Id>::value_type entry(test->commit_index,
-                                                   test->id);
-          bool double_position = !expected_positions.insert(entry).second;
-          ASSERT_FALSE(double_position) << "Two id's expected at one position";
-        }
-        string utf8_name = test->id.GetServerId();
-        string name(utf8_name.begin(), utf8_name.end());
-        MutableEntry entry(&trans, CREATE, BOOKMARKS, test->parent_id, name);
-
-        entry.PutId(test->id);
-        if (test->id.ServerKnows()) {
-          entry.PutBaseVersion(5);
-          entry.PutServerVersion(5);
-          entry.PutServerParentId(test->parent_id);
-        }
-        entry.PutIsDir(true);
-        entry.PutIsUnsynced(true);
-        entry.PutSpecifics(DefaultBookmarkSpecifics());
-        // Set the time to 30 seconds in the future to reduce the chance of
-        // flaky tests.
-        const base::Time& now_plus_30s =
-            base::Time::Now() + base::TimeDelta::FromSeconds(30);
-        const base::Time& now_minus_2h =
-            base::Time::Now() - base::TimeDelta::FromHours(2);
-        entry.PutMtime(now_plus_30s);
-        for (size_t i = 0; i < base::size(test->features); ++i) {
-          switch (test->features[i]) {
-            case LIST_END:
-              break;
-            case SYNCED:
-              entry.PutIsUnsynced(false);
-              break;
-            case DELETED:
-              entry.PutIsDel(true);
-              break;
-            case OLD_MTIME:
-              entry.PutMtime(now_minus_2h);
-              break;
-            case MOVED_FROM_ROOT:
-              entry.PutServerParentId(trans.root_id());
-              break;
-            default:
-              FAIL() << "Bad value in CommitOrderingTest list";
-          }
-        }
-        test++;
-      }
-    }
-    EXPECT_TRUE(SyncShareNudge());
-    ASSERT_EQ(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) {
-      SCOPED_TRACE(i);
-      EXPECT_EQ(1u, expected_positions.count(i));
-      EXPECT_EQ(expected_positions[i], mock_server_->committed_ids()[i]);
-    }
-  }
 
   CommitCounters GetCommitCounters(ModelType type) {
     return debug_info_cache_.GetLatestCommitCounters(type);
@@ -1054,478 +966,6 @@ TEST_F(SyncerTest, ResetVersions) {
   }
 }
 
-TEST_F(SyncerTest, TestCommitListOrderingTwoItemsTall) {
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(-1001), ids_.FromNumber(-1000)},
-      {0, ids_.FromNumber(-1000), ids_.FromNumber(0)},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingThreeItemsTall) {
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(-2001), ids_.FromNumber(-2000)},
-      {0, ids_.FromNumber(-2000), ids_.FromNumber(0)},
-      {2, ids_.FromNumber(-2002), ids_.FromNumber(-2001)},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingFourItemsTall) {
-  CommitOrderingTest items[] = {
-      {3, ids_.FromNumber(-2003), ids_.FromNumber(-2002)},
-      {1, ids_.FromNumber(-2001), ids_.FromNumber(-2000)},
-      {0, ids_.FromNumber(-2000), ids_.FromNumber(0)},
-      {2, ids_.FromNumber(-2002), ids_.FromNumber(-2001)},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingThreeItemsTallLimitedSize) {
-  context_->set_max_commit_batch_size(2);
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(-2001), ids_.FromNumber(-2000)},
-      {0, ids_.FromNumber(-2000), ids_.FromNumber(0)},
-      {2, ids_.FromNumber(-2002), ids_.FromNumber(-2001)},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingSingleDeletedItem) {
-  CommitOrderingTest items[] = {
-      {0, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingSingleUncommittedDeletedItem) {
-  CommitOrderingTest items[] = {
-      {-1, ids_.FromNumber(-1000), ids_.FromNumber(0), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingSingleDeletedItemWithUnroll) {
-  CommitOrderingTest items[] = {
-      {0, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingSingleLongDeletedItemWithUnroll) {
-  CommitOrderingTest items[] = {
-      {0, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingTwoLongDeletedItemWithUnroll) {
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      {0, ids_.FromNumber(1001), ids_.FromNumber(1000), {DELETED, OLD_MTIME}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrdering3LongDeletedItemsWithSizeLimit) {
-  context_->set_max_commit_batch_size(2);
-  CommitOrderingTest items[] = {
-      {2, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      {1, ids_.FromNumber(1001), ids_.FromNumber(1000), {DELETED, OLD_MTIME}},
-      {0, ids_.FromNumber(1002), ids_.FromNumber(1001), {DELETED, OLD_MTIME}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingTwoDeletedItemsWithUnroll) {
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED}},
-      {0, ids_.FromNumber(1001), ids_.FromNumber(1000), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingComplexDeletionScenario) {
-  CommitOrderingTest items[] = {
-      {2, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      {-1, ids_.FromNumber(1001), ids_.FromNumber(0), {SYNCED}},
-      {1, ids_.FromNumber(1002), ids_.FromNumber(1001), {DELETED, OLD_MTIME}},
-      {-1, ids_.FromNumber(1003), ids_.FromNumber(1001), {SYNCED}},
-      {0, ids_.FromNumber(1004), ids_.FromNumber(1003), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest,
-       TestCommitListOrderingComplexDeletionScenarioWith2RecentDeletes) {
-  CommitOrderingTest items[] = {
-      {3, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      {-1, ids_.FromNumber(1001), ids_.FromNumber(0), {SYNCED}},
-      {2, ids_.FromNumber(1002), ids_.FromNumber(1001), {DELETED, OLD_MTIME}},
-      {-1, ids_.FromNumber(1003), ids_.FromNumber(1001), {SYNCED}},
-      {1, ids_.FromNumber(1004), ids_.FromNumber(1003), {DELETED}},
-      {0, ids_.FromNumber(1005), ids_.FromNumber(1003), {DELETED}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingDeleteMovedItems) {
-  CommitOrderingTest items[] = {
-      {1, ids_.FromNumber(1000), ids_.FromNumber(0), {DELETED, OLD_MTIME}},
-      {0,
-       ids_.FromNumber(1001),
-       ids_.FromNumber(1000),
-       {DELETED, OLD_MTIME, MOVED_FROM_ROOT}},
-      CommitOrderingTest::MakeLastCommitItem(),
-  };
-  RunCommitOrderingTest(items);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
-  const base::Time& now_minus_2h =
-      base::Time::Now() - base::TimeDelta::FromHours(2);
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    {
-      MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "Bob");
-      ASSERT_TRUE(parent.good());
-      parent.PutIsUnsynced(true);
-      parent.PutIsDir(true);
-      parent.PutSpecifics(DefaultBookmarkSpecifics());
-      parent.PutId(ids_.FromNumber(100));
-      parent.PutBaseVersion(1);
-      MutableEntry child(&wtrans, CREATE, BOOKMARKS, ids_.FromNumber(100),
-                         "Bob");
-      ASSERT_TRUE(child.good());
-      child.PutIsUnsynced(true);
-      child.PutIsDir(true);
-      child.PutSpecifics(DefaultBookmarkSpecifics());
-      child.PutId(ids_.FromNumber(101));
-      child.PutBaseVersion(1);
-      MutableEntry grandchild(&wtrans, CREATE, BOOKMARKS, ids_.FromNumber(101),
-                              "Bob");
-      ASSERT_TRUE(grandchild.good());
-      grandchild.PutId(ids_.FromNumber(102));
-      grandchild.PutIsUnsynced(true);
-      grandchild.PutSpecifics(DefaultBookmarkSpecifics());
-      grandchild.PutBaseVersion(1);
-    }
-    {
-      // Create three deleted items which deletions we expect to be sent to the
-      // server.
-      MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "Pete");
-      ASSERT_TRUE(parent.good());
-      parent.PutId(ids_.FromNumber(103));
-      parent.PutIsUnsynced(true);
-      parent.PutIsDir(true);
-      parent.PutSpecifics(DefaultBookmarkSpecifics());
-      parent.PutIsDel(true);
-      parent.PutBaseVersion(1);
-      parent.PutMtime(now_minus_2h);
-      MutableEntry child(&wtrans, CREATE, BOOKMARKS, ids_.FromNumber(103),
-                         "Pete");
-      ASSERT_TRUE(child.good());
-      child.PutId(ids_.FromNumber(104));
-      child.PutIsUnsynced(true);
-      child.PutIsDir(true);
-      child.PutSpecifics(DefaultBookmarkSpecifics());
-      child.PutIsDel(true);
-      child.PutBaseVersion(1);
-      child.PutMtime(now_minus_2h);
-      MutableEntry grandchild(&wtrans, CREATE, BOOKMARKS, ids_.FromNumber(104),
-                              "Pete");
-      ASSERT_TRUE(grandchild.good());
-      grandchild.PutId(ids_.FromNumber(105));
-      grandchild.PutIsUnsynced(true);
-      grandchild.PutIsDel(true);
-      grandchild.PutIsDir(false);
-      grandchild.PutSpecifics(DefaultBookmarkSpecifics());
-      grandchild.PutBaseVersion(1);
-      grandchild.PutMtime(now_minus_2h);
-    }
-  }
-
-  EXPECT_TRUE(SyncShareNudge());
-  ASSERT_EQ(6u, mock_server_->committed_ids().size());
-  // This test will NOT unroll deletes because SERVER_PARENT_ID is not set.
-  // It will treat these like moves.
-  vector<syncable::Id> commit_ids(mock_server_->committed_ids());
-  EXPECT_EQ(ids_.FromNumber(100), commit_ids[0]);
-  EXPECT_EQ(ids_.FromNumber(101), commit_ids[1]);
-  EXPECT_EQ(ids_.FromNumber(102), commit_ids[2]);
-  // We don't guarantee the delete orders in this test, only that they occur
-  // at the end.
-  std::sort(commit_ids.begin() + 3, commit_ids.end());
-  EXPECT_EQ(ids_.FromNumber(103), commit_ids[3]);
-  EXPECT_EQ(ids_.FromNumber(104), commit_ids[4]);
-  EXPECT_EQ(ids_.FromNumber(105), commit_ids[5]);
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
-  syncable::Id parent1_id = ids_.MakeServer("p1");
-  syncable::Id parent2_id = ids_.MakeServer("p2");
-
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "1");
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(parent1_id);
-    MutableEntry child(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "2");
-    ASSERT_TRUE(child.good());
-    child.PutIsUnsynced(true);
-    child.PutIsDir(true);
-    child.PutSpecifics(DefaultBookmarkSpecifics());
-    child.PutId(parent2_id);
-    parent.PutBaseVersion(1);
-    child.PutBaseVersion(1);
-  }
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, parent1_id, "A");
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(ids_.FromNumber(102));
-    MutableEntry child(&wtrans, CREATE, BOOKMARKS, parent1_id, "B");
-    ASSERT_TRUE(child.good());
-    child.PutIsUnsynced(true);
-    child.PutIsDir(true);
-    child.PutSpecifics(DefaultBookmarkSpecifics());
-    child.PutId(ids_.FromNumber(-103));
-    parent.PutBaseVersion(1);
-  }
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, parent2_id, "A");
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(ids_.FromNumber(-104));
-    MutableEntry child(&wtrans, CREATE, BOOKMARKS, parent2_id, "B");
-    ASSERT_TRUE(child.good());
-    child.PutIsUnsynced(true);
-    child.PutIsDir(true);
-    child.PutSpecifics(DefaultBookmarkSpecifics());
-    child.PutId(ids_.FromNumber(105));
-    child.PutBaseVersion(1);
-  }
-
-  EXPECT_TRUE(SyncShareNudge());
-  ASSERT_EQ(6u, mock_server_->committed_ids().size());
-
-  // This strange iteration and std::count() usage is to allow the order to
-  // vary.  All we really care about is that parent1_id and parent2_id are the
-  // first two IDs, and that the children make up the next four.  Other than
-  // that, ordering doesn't matter.
-
-  auto i = mock_server_->committed_ids().begin();
-  auto parents_begin = i;
-  i++;
-  i++;
-  auto parents_end = i;
-  auto children_begin = i;
-  auto children_end = mock_server_->committed_ids().end();
-
-  EXPECT_EQ(1, count(parents_begin, parents_end, parent1_id));
-  EXPECT_EQ(1, count(parents_begin, parents_end, parent2_id));
-
-  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(-103)));
-  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(102)));
-  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(105)));
-  EXPECT_EQ(1, count(children_begin, children_end, ids_.FromNumber(-104)));
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
-  syncable::Id child2_id = ids_.NewServerId();
-
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "P");
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(parent_id_);
-    MutableEntry child1(&wtrans, CREATE, BOOKMARKS, parent_id_, "1");
-    ASSERT_TRUE(child1.good());
-    child1.PutIsUnsynced(true);
-    child1.PutId(child_id_);
-    child1.PutSpecifics(DefaultBookmarkSpecifics());
-    MutableEntry child2(&wtrans, CREATE, BOOKMARKS, parent_id_, "2");
-    ASSERT_TRUE(child2.good());
-    child2.PutIsUnsynced(true);
-    child2.PutSpecifics(DefaultBookmarkSpecifics());
-    child2.PutId(child2_id);
-
-    parent.PutBaseVersion(1);
-    child1.PutBaseVersion(1);
-    child2.PutBaseVersion(1);
-  }
-
-  EXPECT_TRUE(SyncShareNudge());
-  ASSERT_EQ(3u, mock_server_->committed_ids().size());
-  EXPECT_EQ(parent_id_, mock_server_->committed_ids()[0]);
-  // There are two possible valid orderings.
-  if (child2_id == mock_server_->committed_ids()[1]) {
-    EXPECT_EQ(child2_id, mock_server_->committed_ids()[1]);
-    EXPECT_EQ(child_id_, mock_server_->committed_ids()[2]);
-  } else {
-    EXPECT_EQ(child_id_, mock_server_->committed_ids()[1]);
-    EXPECT_EQ(child2_id, mock_server_->committed_ids()[2]);
-  }
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
-  string parent1_name = "1";
-  string parent2_name = "A";
-  string child_name = "B";
-
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(),
-                        parent1_name);
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(parent_id_);
-    parent.PutBaseVersion(1);
-  }
-
-  syncable::Id parent2_id = ids_.NewLocalId();
-  syncable::Id child_id = ids_.NewServerId();
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent2(&wtrans, CREATE, BOOKMARKS, parent_id_, parent2_name);
-    ASSERT_TRUE(parent2.good());
-    parent2.PutIsUnsynced(true);
-    parent2.PutIsDir(true);
-    parent2.PutSpecifics(DefaultBookmarkSpecifics());
-    parent2.PutId(parent2_id);
-
-    MutableEntry child(&wtrans, CREATE, BOOKMARKS, parent2_id, child_name);
-    ASSERT_TRUE(child.good());
-    child.PutIsUnsynced(true);
-    child.PutIsDir(true);
-    child.PutSpecifics(DefaultBookmarkSpecifics());
-    child.PutId(child_id);
-    child.PutBaseVersion(1);
-  }
-
-  EXPECT_TRUE(SyncShareNudge());
-  ASSERT_EQ(3u, mock_server_->committed_ids().size());
-  // If this test starts failing, be aware other sort orders could be valid.
-  EXPECT_EQ(parent_id_, mock_server_->committed_ids()[0]);
-  EXPECT_EQ(parent2_id, mock_server_->committed_ids()[1]);
-  EXPECT_EQ(child_id, mock_server_->committed_ids()[2]);
-  {
-    syncable::ReadTransaction rtrans(FROM_HERE, directory());
-    // Check that things committed correctly.
-    Entry entry_1(&rtrans, GET_BY_ID, parent_id_);
-    EXPECT_EQ(parent1_name, entry_1.GetNonUniqueName());
-    // Check that parent2 is a subfolder of parent1.
-    EXPECT_EQ(1, CountEntriesWithName(&rtrans, parent_id_, parent2_name));
-
-    // Parent2 was a local ID and thus should have changed on commit!
-    Entry pre_commit_entry_parent2(&rtrans, GET_BY_ID, parent2_id);
-    ASSERT_FALSE(pre_commit_entry_parent2.good());
-
-    // Look up the new ID.
-    Id parent2_committed_id =
-        GetOnlyEntryWithName(&rtrans, parent_id_, parent2_name);
-    EXPECT_TRUE(parent2_committed_id.ServerKnows());
-
-    Entry child(&rtrans, GET_BY_ID, child_id);
-    EXPECT_EQ(parent2_committed_id, child.GetParentId());
-  }
-}
-
-TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
-  string parent_name = "1";
-  string parent2_name = "A";
-  string child_name = "B";
-
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(),
-                        parent_name);
-    ASSERT_TRUE(parent.good());
-    parent.PutIsUnsynced(true);
-    parent.PutIsDir(true);
-    parent.PutSpecifics(DefaultBookmarkSpecifics());
-    parent.PutId(parent_id_);
-    parent.PutBaseVersion(1);
-  }
-
-  int64_t meta_handle_b;
-  const Id parent2_local_id = ids_.NewLocalId();
-  const Id child_local_id = ids_.NewLocalId();
-  {
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry parent2(&wtrans, CREATE, BOOKMARKS, parent_id_, parent2_name);
-    ASSERT_TRUE(parent2.good());
-    parent2.PutIsUnsynced(true);
-    parent2.PutIsDir(true);
-    parent2.PutSpecifics(DefaultBookmarkSpecifics());
-
-    parent2.PutId(parent2_local_id);
-    MutableEntry child(&wtrans, CREATE, BOOKMARKS, parent2_local_id,
-                       child_name);
-    ASSERT_TRUE(child.good());
-    child.PutIsUnsynced(true);
-    child.PutIsDir(true);
-    child.PutSpecifics(DefaultBookmarkSpecifics());
-    child.PutId(child_local_id);
-    meta_handle_b = child.GetMetahandle();
-  }
-
-  EXPECT_TRUE(SyncShareNudge());
-  ASSERT_EQ(3u, mock_server_->committed_ids().size());
-  // If this test starts failing, be aware other sort orders could be valid.
-  EXPECT_EQ(parent_id_, mock_server_->committed_ids()[0]);
-  EXPECT_EQ(parent2_local_id, mock_server_->committed_ids()[1]);
-  EXPECT_EQ(child_local_id, mock_server_->committed_ids()[2]);
-  {
-    syncable::ReadTransaction rtrans(FROM_HERE, directory());
-
-    Entry parent(&rtrans, GET_BY_ID,
-                 GetOnlyEntryWithName(&rtrans, rtrans.root_id(), parent_name));
-    ASSERT_TRUE(parent.good());
-    EXPECT_TRUE(parent.GetId().ServerKnows());
-
-    Entry parent2(&rtrans, GET_BY_ID,
-                  GetOnlyEntryWithName(&rtrans, parent.GetId(), parent2_name));
-    ASSERT_TRUE(parent2.good());
-    EXPECT_TRUE(parent2.GetId().ServerKnows());
-
-    // Id changed on commit, so this should fail.
-    Entry local_parent2_id_entry(&rtrans, GET_BY_ID, parent2_local_id);
-    ASSERT_FALSE(local_parent2_id_entry.good());
-
-    Entry entry_b(&rtrans, GET_BY_HANDLE, meta_handle_b);
-    EXPECT_TRUE(entry_b.GetId().ServerKnows());
-    EXPECT_EQ(parent2.GetId(), entry_b.GetParentId());
-  }
-}
-
 TEST_F(SyncerTest, UpdateWithZeroLengthName) {
   // One illegal update
   mock_server_->AddUpdateDirectory(1, 0, std::string(), 1, 10,