Deprecate DriveMetadataStore.batch_sync_origins

chrome/browser/sync_file_system/drive_metadata_store_unittest.cc

Index: chrome/browser/sync_file_system/drive_metadata_store_unittest.cc
diff --git a/chrome/browser/sync_file_system/drive_metadata_store_unittest.cc b/chrome/browser/sync_file_system/drive_metadata_store_unittest.cc
index c7259c25aca58dd9bc9215904163b18b3dc379d1..c13b001ab1b5f13e899e332c52ab22260e71fbc7 100644
--- a/chrome/browser/sync_file_system/drive_metadata_store_unittest.cc
+++ b/chrome/browser/sync_file_system/drive_metadata_store_unittest.cc
@@ -132,10 +132,8 @@ class DriveMetadataStoreTest : public testing::Test {
 
   void DropSyncOriginsInStore() {
     EXPECT_TRUE(ui_task_runner_->RunsTasksOnCurrentThread());
-    drive_metadata_store_->batch_sync_origins_.clear();
     drive_metadata_store_->incremental_sync_origins_.clear();
     drive_metadata_store_->disabled_origins_.clear();
-    EXPECT_TRUE(drive_metadata_store_->batch_sync_origins().empty());
     EXPECT_TRUE(drive_metadata_store_->incremental_sync_origins().empty());
     EXPECT_TRUE(drive_metadata_store_->disabled_origins().empty());
   }
@@ -216,14 +214,12 @@ class DriveMetadataStoreTest : public testing::Test {
   }
 
   void VerifyUntrackedOrigin(const GURL& origin) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsOriginDisabled(origin));
   }
 
   void VerifyIncrementalSyncOrigin(const GURL& origin,
                                    const std::string& resource_id) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_TRUE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsOriginDisabled(origin));
     EXPECT_EQ(resource_id,
@@ -233,7 +229,6 @@ class DriveMetadataStoreTest : public testing::Test {
 
   void VerifyDisabledOrigin(const GURL& origin,
                             const std::string& resource_id) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_TRUE(metadata_store()->IsOriginDisabled(origin));
     EXPECT_EQ(resource_id,
@@ -253,8 +248,6 @@ class DriveMetadataStoreTest : public testing::Test {
   }
 
   void VerifyReverseMap() {
-    const ResourceIdByOrigin& batch_sync_origins =
-        drive_metadata_store_->batch_sync_origins_;
     const ResourceIdByOrigin& incremental_sync_origins =
         drive_metadata_store_->incremental_sync_origins_;
     const ResourceIdByOrigin& disabled_origins =
@@ -262,13 +255,10 @@ class DriveMetadataStoreTest : public testing::Test {
     const OriginByResourceId& origin_by_resource_id =
         drive_metadata_store_->origin_by_resource_id_;
 
-    size_t expected_size =
-        batch_sync_origins.size() + incremental_sync_origins.size() +
-        disabled_origins.size();
+    size_t expected_size = incremental_sync_origins.size() +
+                           disabled_origins.size();
     size_t actual_size = origin_by_resource_id.size();
     EXPECT_EQ(expected_size, actual_size);
-    EXPECT_TRUE(VerifyReverseMapInclusion(batch_sync_origins,
-                                          origin_by_resource_id));
     EXPECT_TRUE(VerifyReverseMapInclusion(incremental_sync_origins,
                                           origin_by_resource_id));
     EXPECT_TRUE(VerifyReverseMapInclusion(disabled_origins,
@@ -534,7 +524,6 @@ TEST_F(DriveMetadataStoreTest, RemoveOrigin) {
   InitializeDatabase();
 
   // kOrigin1 should be the only one left.
-  EXPECT_EQ(0u, metadata_store()->batch_sync_origins().size());
   EXPECT_EQ(1u, metadata_store()->incremental_sync_origins().size());
   EXPECT_EQ(0u, metadata_store()->disabled_origins().size());
   EXPECT_TRUE(metadata_store()->IsIncrementalSyncOrigin(kOrigin1));
@@ -652,7 +641,6 @@ TEST_F(DriveMetadataStoreTest, MigrationFromV0) {
 
   EXPECT_EQ(1, metadata_store()->GetLargestChangeStamp());
   EXPECT_EQ(kSyncRootResourceId, metadata_store()->sync_root_directory());
-  EXPECT_EQ(kResourceId1, metadata_store()->GetResourceIdForOrigin(kOrigin1));
   EXPECT_EQ(kResourceId2, metadata_store()->GetResourceIdForOrigin(kOrigin2));
 
   DriveMetadata metadata;
@@ -668,6 +656,38 @@ TEST_F(DriveMetadataStoreTest, MigrationFromV0) {
   VerifyReverseMap();
 }
 
+TEST_F(DriveMetadataStoreTest, DeprecateBatchSyncOrigins) {
+  // Make sure that previously saved batch sync origins were deleted from the DB
+  // as they are no longer used.
+  const char kDriveBatchSyncOriginKeyPrefix[] = "BSYNC_ORIGIN: ";
+  const GURL kOrigin1("chrome-extension://example1");
+  const std::string kSyncRootResourceId("sync_root_resource_id");
+  const std::string kResourceId1("hoge");
+  const std::string kFileResourceId("piyo");
+
+  // Purposely add in an old batch sync origin (from previous DB version).
+  leveldb::Options options;
+  options.create_if_missing = true;
+  leveldb::DB* db_ptr = NULL;
+  std::string db_dir = fileapi::FilePathToString(
+      base_dir().Append(DriveMetadataStore::kDatabaseName));
+  leveldb::Status status = leveldb::DB::Open(options, db_dir, &db_ptr);

[tzik, 2013/05/27 04:31:58]

We can open only one LevelDB instance for each path, that seems to cause the
Windows test failure.

Can we get it from metadata_store()?
Or, just
 - Close the DB before InitializeDatabase(),
 - DropDatabase(),
 - Reopen the DB,
may work.

[tzik, 2013/05/27 07:57:42]

For second option, we need to ensure the LevelDB instance was dropped before the
reopen. Since it is closed asynchronously on FILE thread.

Please insert something like below (it may need a helper function)
file_task_runner_->PostTaskAndReply(
    FROM_HERE,
    base::Bind(&Noop),
    base::Bind(&FunctionToCallMessageLoopQuit));
message_loop_->Run();

+  scoped_ptr<leveldb::DB> db(db_ptr);
+  leveldb::WriteOptions write_options;
+  db->Put(write_options,
+          kDriveBatchSyncOriginKeyPrefix + kOrigin1.spec(), kResourceId1);
+  InitializeDatabase();
+
+  // Confirm no batch sync origins rename after InitializeDatabase.
+  scoped_ptr<leveldb::Iterator> itr(db->NewIterator(leveldb::ReadOptions()));
+  for (itr->SeekToFirst(); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    bool batch_origin_found =
+        (key.find(kDriveBatchSyncOriginKeyPrefix) == std::string::npos);
+    EXPECT_FALSE(batch_origin_found);
+  }
+}
+
 TEST_F(DriveMetadataStoreTest, ResetOriginRootDirectory) {
   const GURL kOrigin1("chrome-extension://example1");
   const std::string kResourceId1("hoge");