Sync: Avoid 3 passes over SyncDarta DB when loading the directory from the disk

sync/syncable/directory_unittest.cc

Index: sync/syncable/directory_unittest.cc
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index 5f8cbed23c2225e3827f09a905586f6f8e4eaa5a..333cf6094cab47acbecbe9208eb70ac05842f0b1 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -552,6 +552,83 @@ TEST_F(SyncableDirectoryTest, ManageDeleteJournals) {
   }
 }
 
+TEST_F(SyncableDirectoryTest, TestPurgeDeletedEntriesOnReload) {
+  sync_pb::EntitySpecifics specifics;
+  AddDefaultFieldValue(PREFERENCES, &specifics);
+
+  const int CLIENT_COUNT = 2;

[Nicolas Zea, 2015/03/17 01:05:17]

nit: const variables should be of the form kCamelCase

+  const int SERVER_COUNT = 5;
+  const int TEST_COUNT = CLIENT_COUNT + SERVER_COUNT;
+  int64 handles[TEST_COUNT];
+
+  // The idea is to recreate various combinations of IDs, IS_DEL,
+  // IS_UNSYNCED, and IS_UNAPPLIED_UPDATE flags to test all combinations
+  // for DirectoryBackingStore::SafeToPurgeOnLoading.
+  // 0: client ID, IS_DEL, IS_UNSYNCED
+  // 1: client ID, IS_UNSYNCED
+  // 2: server ID, IS_DEL, IS_UNSYNCED, IS_UNAPPLIED_UPDATE
+  // 3: server ID, IS_DEL, IS_UNSYNCED
+  // 4: server ID, IS_DEL, IS_UNAPPLIED_UPDATE
+  // 5: server ID, IS_DEL
+  // 6: server ID
+  {
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
+
+    for (int i = 0; i < TEST_COUNT; i++) {
+      std::string name = base::StringPrintf("item%d", i);
+      MutableEntry item(&trans, CREATE, PREFERENCES, trans.root_id(), name);
+      ASSERT_TRUE(item.good());
+
+      handles[i] = item.GetMetahandle();
+
+      if (i < CLIENT_COUNT) {
+        item.PutId(TestIdFactory::FromNumber(i - CLIENT_COUNT));
+      } else {
+        item.PutId(TestIdFactory::FromNumber(i));
+      }
+
+      item.PutUniqueClientTag(name);
+      item.PutIsUnsynced(true);
+      item.PutSpecifics(specifics);
+      item.PutServerSpecifics(specifics);
+
+      if (i >= CLIENT_COUNT) {
+        item.PutBaseVersion(10);
+        item.PutServerVersion(10);
+      }
+
+      // Set flags
+      if (i != 1 && i != 6)
+        item.PutIsDel(true);
+
+      if (i >= 4)
+        item.PutIsUnsynced(false);
+
+      if (i == 2 || i == 4)
+        item.PutIsUnappliedUpdate(true);
+    }
+  }
+  ASSERT_EQ(OPENED, SimulateSaveAndReloadDir());
+
+  // Expect items 0 and 5 to be purged

[Nicolas Zea, 2015/03/17 01:05:17]

nit: explain why?

+  std::vector<int64> expected_purged;
+  expected_purged.push_back(0);
+  expected_purged.push_back(5);
+
+  std::vector<int64> actually_purged;
+  {
+    ReadTransaction trans(FROM_HERE, dir().get());
+    for (int i = 0; i < TEST_COUNT; i++) {
+      Entry item(&trans, GET_BY_HANDLE, handles[i]);
+      if (!item.good()) {
+        actually_purged.push_back(i);
+      }
+    }
+  }
+
+  EXPECT_EQ(expected_purged, actually_purged);
+}
+
 TEST_F(SyncableDirectoryTest, TestBasicLookupNonExistantID) {
   ReadTransaction rtrans(FROM_HERE, dir().get());
   Entry e(&rtrans, GET_BY_ID, TestIdFactory::FromNumber(-99));