chromeos: Use WriteBatch to update DriveResourceMetadata DB

DriveResourceMetadataStorage::PutEntry/RemoveEntry is responsible to manage parent-child relationship.
Rename DriveResourceMetadata::RemoveDirectoryChild to RemoveEntryRecursively.
Return error from DriveResourceMetadata::AddEntryToDirectory/RemoveEntryRecursively.
DriveResourceMetadata::AddEntryToDirectory is responsible to remove an entry from its old parent.

BUG=234110
TEST=unit_tests

Review URL: https://chromiumcodereview.appspot.com/14108009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196119 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 219 insertions, 198 deletions

diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata.cc b/chrome/browser/chromeos/drive/drive_resource_metadata.cc
index 8571a6e..0897631 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata.cc
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata.cc
@@ -222,7 +222,7 @@ void DriveResourceMetadata::SetUpDefaultEntries() {
     storage_->PutEntry(CreateEntryWithProperBaseName(root));
   }
   if (!storage_->GetEntry(util::kDriveOtherDirSpecialResourceId)) {
-    AddEntryToDirectory(util::CreateOtherDirEntry());
+    PutEntryUnderDirectory(util::CreateOtherDirEntry());
   }
 }
 
@@ -547,7 +547,9 @@ DriveResourceMetadata::RemoveEntryOnBlockingPool(
   if (!entry)
     return FileMoveResult(FILE_ERROR_NOT_FOUND);
 
-  RemoveDirectoryChild(entry->resource_id());
+  if (!RemoveEntryRecursively(entry->resource_id()))
+    return FileMoveResult(FILE_ERROR_FAILED);
+
   return FileMoveResult(FILE_ERROR_OK,
                         GetFilePath(entry->parent_resource_id()));
 }
@@ -679,8 +681,8 @@ FileError DriveResourceMetadata::RefreshEntryOnBlockingPool(
     return FILE_ERROR_NOT_FOUND;
 
   // Remove from the old parent and add it to the new parent with the new data.
-  DetachEntryFromDirectory(old_entry->resource_id());
-  AddEntryToDirectory(CreateEntryWithProperBaseName(entry));
+  if (!PutEntryUnderDirectory(CreateEntryWithProperBaseName(entry)))
+    return FILE_ERROR_FAILED;
 
   if (out_file_path)
     *out_file_path = GetFilePath(entry.resource_id());
@@ -739,12 +741,8 @@ DriveResourceMetadata::RefreshDirectoryOnBlockingPool(
       continue;
     }
 
-    scoped_ptr<DriveEntryProto> existing_entry =
-        storage_->GetEntry(entry_proto.resource_id());
-    if (existing_entry)
-      DetachEntryFromDirectory(entry_proto.resource_id());
-
-    AddEntryToDirectory(CreateEntryWithProperBaseName(entry_proto));
+    if (!PutEntryUnderDirectory(CreateEntryWithProperBaseName(entry_proto)))
+      return FileMoveResult(FILE_ERROR_FAILED);
   }
 
   // Go through the existing entries and remove deleted entries.
@@ -755,8 +753,10 @@ DriveResourceMetadata::RefreshDirectoryOnBlockingPool(
       return FileMoveResult(FILE_ERROR_NO_SPACE);
 
     const DriveEntryProto& entry_proto = entries->at(i);
-    if (entry_proto_map.count(entry_proto.resource_id()) == 0)
-      RemoveDirectoryChild(entry_proto.resource_id());
+    if (entry_proto_map.count(entry_proto.resource_id()) == 0) {
+      if (!RemoveEntryRecursively(entry_proto.resource_id()))
+        return FileMoveResult(FILE_ERROR_FAILED);
+    }
   }
 
   return FileMoveResult(FILE_ERROR_OK, GetFilePath(directory->resource_id()));
@@ -780,7 +780,9 @@ DriveResourceMetadata::AddEntryOnBlockingPool(
   if (!parent)
     return FileMoveResult(FILE_ERROR_NOT_FOUND);
 
-  AddEntryToDirectory(entry_proto);
+  if (!PutEntryUnderDirectory(entry_proto))
+    return FileMoveResult(FILE_ERROR_FAILED);
+
   return FileMoveResult(FILE_ERROR_OK, GetFilePath(entry_proto.resource_id()));
 }
 
@@ -847,7 +849,7 @@ void DriveResourceMetadata::RemoveAllOnBlockingPool() {
     return;
   }
 
-  RemoveDirectoryChildren(util::kDriveGrandRootSpecialResourceId);
+  RemoveEntryRecursively(util::kDriveGrandRootSpecialResourceId);
   SetUpDefaultEntries();
 }
 
@@ -906,7 +908,8 @@ void DriveResourceMetadata::GetEntryInfoPairByPathsAfterGetSecond(
   callback.Run(result.Pass());
 }
 
-void DriveResourceMetadata::AddEntryToDirectory(const DriveEntryProto& entry) {
+bool DriveResourceMetadata::PutEntryUnderDirectory(
+    const DriveEntryProto& entry) {
   DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
 
   DriveEntryProto updated_entry(entry);
@@ -916,12 +919,16 @@ void DriveResourceMetadata::AddEntryToDirectory(const DriveEntryProto& entry) {
   // through name de-duplication again when it is added to another directory.
   SetBaseNameFromTitle(&updated_entry);
 
-  // Do file name de-duplication - find files with the same name and
-  // append a name modifier to the name.
+  // Do file name de-duplication - Keep changing |entry|'s name until there is
+  // no other entry with the same name under the parent.
   int modifier = 1;
   std::string new_base_name = updated_entry.base_name();
-  while (!storage_->GetChild(entry.parent_resource_id(),
-                             new_base_name).empty()) {
+  while (true) {
+    const std::string existing_entry_id =
+        storage_->GetChild(entry.parent_resource_id(), new_base_name);
+    if (existing_entry_id.empty() || existing_entry_id == entry.resource_id())
+      break;
+
     base::FilePath new_path =
         base::FilePath::FromUTF8Unsafe(updated_entry.base_name());
     new_path =
@@ -930,50 +937,26 @@ void DriveResourceMetadata::AddEntryToDirectory(const DriveEntryProto& entry) {
   }
   updated_entry.set_base_name(new_base_name);
 
-  // Setup child and parent links.
-  storage_->PutChild(entry.parent_resource_id(),
-                     updated_entry.base_name(),
-                     updated_entry.resource_id());
-
   // Add the entry to resource map.
-  storage_->PutEntry(updated_entry);
-}
-
-void DriveResourceMetadata::RemoveDirectoryChild(
-    const std::string& child_resource_id) {
-  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
-
-  scoped_ptr<DriveEntryProto> entry = storage_->GetEntry(child_resource_id);
-  DCHECK(entry);
-  DetachEntryFromDirectory(child_resource_id);
-  storage_->RemoveEntry(entry->resource_id());
-  if (entry->file_info().is_directory())
-    RemoveDirectoryChildren(child_resource_id);
+  return storage_->PutEntry(updated_entry);
 }
 
-void DriveResourceMetadata::DetachEntryFromDirectory(
-    const std::string& child_resource_id) {
+bool DriveResourceMetadata::RemoveEntryRecursively(
+    const std::string& resource_id) {
   DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
 
-  scoped_ptr<DriveEntryProto> entry = storage_->GetEntry(child_resource_id);
+  scoped_ptr<DriveEntryProto> entry = storage_->GetEntry(resource_id);
   DCHECK(entry);
 
-  // entry must be present in this directory.
-  DCHECK_EQ(entry->resource_id(),
-            storage_->GetChild(entry->parent_resource_id(),
-                               entry->base_name()));
-
-  storage_->RemoveChild(entry->parent_resource_id(), entry->base_name());
-}
-
-void DriveResourceMetadata::RemoveDirectoryChildren(
-    const std::string& directory_resource_id) {
-  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
-
-  std::vector<std::string> children;
-  storage_->GetChildren(directory_resource_id, &children);
-  for (size_t i = 0; i < children.size(); ++i)
-    RemoveDirectoryChild(children[i]);
+  if (entry->file_info().is_directory()) {
+    std::vector<std::string> children;
+    storage_->GetChildren(resource_id, &children);
+    for (size_t i = 0; i < children.size(); ++i) {
+      if (!RemoveEntryRecursively(children[i]))
+        return false;
+    }
+  }
+  return storage_->RemoveEntry(resource_id);
 }
 
 scoped_ptr<DriveEntryProtoVector>
diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata.h b/chrome/browser/chromeos/drive/drive_resource_metadata.h
index 1334deb..5446dee 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata.h
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata.h
@@ -340,20 +340,14 @@ class DriveResourceMetadata {
   void GetDescendantDirectoryPaths(const std::string& resource_id,
                                    std::set<base::FilePath>* child_directories);
 
-  // Adds a child entry to its parent directory.
-  // The method will also do name de-duplication to ensure that the
-  // exposed presentation path does not have naming conflicts. Two files with
-  // the same name "Foo" will be renames to "Foo (1)" and "Foo (2)".
-  void AddEntryToDirectory(const DriveEntryProto& entry);
-
-  // Removes the entry from its parent directory.
-  void RemoveDirectoryChild(const std::string& child_resource_id);
-
-  // Detaches the entry from its parent directory.
-  void DetachEntryFromDirectory(const std::string& child_resource_id);
-
-  // Removes child elements of directory.
-  void RemoveDirectoryChildren(const std::string& directory_resource_id);
+  // Puts an entry under its parent directory. Removes the child from the old
+  // parent if there is. This method will also do name de-duplication to ensure
+  // that the exposed presentation path does not have naming conflicts. Two
+  // files with the same name "Foo" will be renames to "Foo (1)" and "Foo (2)".
+  bool PutEntryUnderDirectory(const DriveEntryProto& entry);
+
+  // Removes the entry and its descendants.
+  bool RemoveEntryRecursively(const std::string& resource_id);
 
   // Converts the children as a vector of DriveEntryProto.
   scoped_ptr<DriveEntryProtoVector> DirectoryChildrenToProtoVector(
diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata_storage.cc b/chrome/browser/chromeos/drive/drive_resource_metadata_storage.cc
index 3c887a9..ee8f178 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata_storage.cc
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata_storage.cc
@@ -10,6 +10,7 @@
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
+#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
 namespace drive {
 
@@ -31,16 +32,6 @@ std::string GetHeaderDBKey() {
   return key;
 }
 
-// Returns a string to be used as a key for child entry.
-std::string GetChildEntryKey(const std::string& parent_resource_id,
-                             const std::string& child_name) {
-  std::string key = parent_resource_id;
-  key.push_back(kDBKeyDelimeter);
-  key.append(child_name);
-  key.push_back(kDBKeyDelimeter);
-  return key;
-}
-
 // Returns true if |key| is a key for a child entry.
 bool IsChildEntryKey(const leveldb::Slice& key) {
   return !key.empty() && key[key.size() - 1] == kDBKeyDelimeter;
@@ -132,21 +123,37 @@ int64 DriveResourceMetadataStorage::GetLargestChangestamp() {
   return header->largest_changestamp();
 }
 
-void DriveResourceMetadataStorage::PutEntry(const DriveEntryProto& entry) {
+bool DriveResourceMetadataStorage::PutEntry(const DriveEntryProto& entry) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(!entry.resource_id().empty());
 
   std::string serialized_entry;
   if (!entry.SerializeToString(&serialized_entry)) {
     DLOG(ERROR) << "Failed to serialize the entry: " << entry.resource_id();
-    return;
+    return false;
   }
 
-  const leveldb::Status status = resource_map_->Put(
-      leveldb::WriteOptions(),
-      leveldb::Slice(entry.resource_id()),
-      leveldb::Slice(serialized_entry));
-  DCHECK(status.ok());
+  leveldb::WriteBatch batch;
+
+  // Remove from the old parent.
+  scoped_ptr<DriveEntryProto> old_entry = GetEntry(entry.resource_id());
+  if (old_entry && !old_entry->parent_resource_id().empty()) {
+    batch.Delete(GetChildEntryKey(old_entry->parent_resource_id(),
+                                  old_entry->base_name()));
+  }
+
+  // Add to the new parent.
+  if (!entry.parent_resource_id().empty()) {
+    batch.Put(GetChildEntryKey(entry.parent_resource_id(), entry.base_name()),
+              entry.resource_id());
+  }
+
+  // Put the entry itself.
+  batch.Put(entry.resource_id(), serialized_entry);
+
+  const leveldb::Status status = resource_map_->Write(leveldb::WriteOptions(),
+                                                      &batch);
+  return status.ok();
 }
 
 scoped_ptr<DriveEntryProto> DriveResourceMetadataStorage::GetEntry(
@@ -167,14 +174,27 @@ scoped_ptr<DriveEntryProto> DriveResourceMetadataStorage::GetEntry(
   return entry.Pass();
 }
 
-void DriveResourceMetadataStorage::RemoveEntry(const std::string& resource_id) {
+bool DriveResourceMetadataStorage::RemoveEntry(const std::string& resource_id) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(!resource_id.empty());
 
-  const leveldb::Status status = resource_map_->Delete(
-      leveldb::WriteOptions(),
-      leveldb::Slice(resource_id));
-  DCHECK(status.ok());
+  scoped_ptr<DriveEntryProto> entry = GetEntry(resource_id);
+  if (!entry)
+    return false;
+
+  leveldb::WriteBatch batch;
+
+  // Remove from the parent.
+  if (!entry->parent_resource_id().empty()) {
+    batch.Delete(GetChildEntryKey(entry->parent_resource_id(),
+                                  entry->base_name()));
+  }
+  // Remove the entry itself.
+  batch.Delete(resource_id);
+
+  const leveldb::Status status = resource_map_->Write(leveldb::WriteOptions(),
+                                                      &batch);
+  return status.ok();
 }
 
 void DriveResourceMetadataStorage::Iterate(const IterateCallback& callback) {
@@ -198,19 +218,6 @@ void DriveResourceMetadataStorage::Iterate(const IterateCallback& callback) {
   }
 }
 
-void DriveResourceMetadataStorage::PutChild(
-    const std::string& parent_resource_id,
-    const std::string& child_name,
-    const std::string& child_resource_id) {
-  base::ThreadRestrictions::AssertIOAllowed();
-
-  const leveldb::Status status = resource_map_->Put(
-      leveldb::WriteOptions(),
-      leveldb::Slice(GetChildEntryKey(parent_resource_id, child_name)),
-      leveldb::Slice(child_resource_id));
-  DCHECK(status.ok());
-}
-
 std::string DriveResourceMetadataStorage::GetChild(
     const std::string& parent_resource_id,
     const std::string& child_name) {
@@ -241,15 +248,15 @@ void DriveResourceMetadataStorage::GetChildren(
   DCHECK(it->status().ok());
 }
 
-void DriveResourceMetadataStorage::RemoveChild(
+// static
+std::string DriveResourceMetadataStorage::GetChildEntryKey(
     const std::string& parent_resource_id,
     const std::string& child_name) {
-  base::ThreadRestrictions::AssertIOAllowed();
-
-  const leveldb::Status status = resource_map_->Delete(
-      leveldb::WriteOptions(),
-      leveldb::Slice(GetChildEntryKey(parent_resource_id, child_name)));
-  DCHECK(status.ok());
+  std::string key = parent_resource_id;
+  key.push_back(kDBKeyDelimeter);
+  key.append(child_name);
+  key.push_back(kDBKeyDelimeter);
+  return key;
 }
 
 void DriveResourceMetadataStorage::PutHeader(
diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata_storage.h b/chrome/browser/chromeos/drive/drive_resource_metadata_storage.h
index 40c78ca..2566b79 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata_storage.h
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata_storage.h
@@ -45,22 +45,17 @@ class DriveResourceMetadataStorage {
   int64 GetLargestChangestamp();
 
   // Puts the entry to this storage.
-  void PutEntry(const DriveEntryProto& entry);
+  bool PutEntry(const DriveEntryProto& entry);
 
   // Returns an entry stored in this storage.
   scoped_ptr<DriveEntryProto> GetEntry(const std::string& resource_id);
 
   // Removes an entry from this storage.
-  void RemoveEntry(const std::string& resource_id);
+  bool RemoveEntry(const std::string& resource_id);
 
   // Iterates over entries stored in this storage.
   void Iterate(const IterateCallback& callback);
 
-  // Puts child under the parent.
-  void PutChild(const std::string& parent_resource_id,
-                const std::string& child_name,
-                const std::string& child_resource_id);
-
   // Returns resource ID of the parent's child.
   std::string GetChild(const std::string& parent_resource_id,
                        const std::string& child_name);
@@ -69,13 +64,13 @@ class DriveResourceMetadataStorage {
   void GetChildren(const std::string& parent_resource_id,
                    std::vector<std::string>* children);
 
-  // Removes child from the parent.
-  void RemoveChild(const std::string& parent_resource_id,
-                   const std::string& child_name);
-
  private:
   friend class DriveResourceMetadataStorageTest;
 
+  // Returns a string to be used as a key for child entry.
+  static std::string GetChildEntryKey(const std::string& parent_resource_id,
+                                      const std::string& child_name);
+
   // Puts header.
   void PutHeader(const DriveResourceMetadataHeader& header);
 
diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata_storage_unittest.cc b/chrome/browser/chromeos/drive/drive_resource_metadata_storage_unittest.cc
index 04090f2..d8453e5 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata_storage_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata_storage_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
 
 namespace drive {
 
@@ -48,6 +49,26 @@ class DriveResourceMetadataStorageTest : public testing::Test {
     return storage_->CheckValidity();
   }
 
+  // Puts a child entry.
+  void PutChild(const std::string& parent_resource_id,
+                const std::string& child_base_name,
+                const std::string& child_resource_id) {
+    storage_->resource_map_->Put(
+        leveldb::WriteOptions(),
+        DriveResourceMetadataStorage::GetChildEntryKey(parent_resource_id,
+                                                       child_base_name),
+        child_resource_id);
+  }
+
+  // Removes a child entry.
+  void RemoveChild(const std::string& parent_resource_id,
+                   const std::string& child_base_name) {
+    storage_->resource_map_->Delete(
+        leveldb::WriteOptions(),
+        DriveResourceMetadataStorage::GetChildEntryKey(parent_resource_id,
+                                                       child_base_name));
+  }
+
   base::ScopedTempDir temp_dir_;
   scoped_ptr<DriveResourceMetadataStorage> storage_;
 };
@@ -61,6 +82,9 @@ TEST_F(DriveResourceMetadataStorageTest, LargestChangestamp) {
 TEST_F(DriveResourceMetadataStorageTest, PutEntry) {
   const std::string key1 = "abcdefg";
   const std::string key2 = "abcd";
+  const std::string key3 = "efgh";
+  const std::string name2 = "ABCD";
+  const std::string name3 = "EFGH";
 
   DriveEntryProto entry1;
   entry1.set_resource_id(key1);
@@ -69,7 +93,7 @@ TEST_F(DriveResourceMetadataStorageTest, PutEntry) {
   EXPECT_FALSE(storage_->GetEntry(key1));
 
   // Put entry1.
-  storage_->PutEntry(entry1);
+  EXPECT_TRUE(storage_->PutEntry(entry1));
 
   // key1 found.
   scoped_ptr<DriveEntryProto> result;
@@ -80,8 +104,42 @@ TEST_F(DriveResourceMetadataStorageTest, PutEntry) {
   // key2 not found.
   EXPECT_FALSE(storage_->GetEntry(key2));
 
-  // Remove key1.
-  storage_->RemoveEntry(key1);
+  // Put entry2 as a child of entry1.
+  DriveEntryProto entry2;
+  entry2.set_parent_resource_id(key1);
+  entry2.set_resource_id(key2);
+  entry2.set_base_name(name2);
+  EXPECT_TRUE(storage_->PutEntry(entry2));
+
+  // key2 found.
+  EXPECT_TRUE(storage_->GetEntry(key2));
+  EXPECT_EQ(key2, storage_->GetChild(key1, name2));
+
+  // Put entry3 as a child of entry2.
+  DriveEntryProto entry3;
+  entry3.set_parent_resource_id(key2);
+  entry3.set_resource_id(key3);
+  entry3.set_base_name(name3);
+  EXPECT_TRUE(storage_->PutEntry(entry3));
+
+  // key3 found.
+  EXPECT_TRUE(storage_->GetEntry(key3));
+  EXPECT_EQ(key3, storage_->GetChild(key2, name3));
+
+  // Change entry3's parent to entry1.
+  entry3.set_parent_resource_id(key1);
+  EXPECT_TRUE(storage_->PutEntry(entry3));
+
+  // entry3 is a child of entry1 now.
+  EXPECT_TRUE(storage_->GetChild(key2, name3).empty());
+  EXPECT_EQ(key3, storage_->GetChild(key1, name3));
+
+  // Remove entries.
+  EXPECT_TRUE(storage_->RemoveEntry(key3));
+  EXPECT_FALSE(storage_->GetEntry(key3));
+  EXPECT_TRUE(storage_->RemoveEntry(key2));
+  EXPECT_FALSE(storage_->GetEntry(key2));
+  EXPECT_TRUE(storage_->RemoveEntry(key1));
   EXPECT_FALSE(storage_->GetEntry(key1));
 }
 
@@ -100,7 +158,7 @@ TEST_F(DriveResourceMetadataStorageTest, Iterate) {
   entries.push_back(entry);
 
   for (size_t i = 0; i < entries.size(); ++i)
-    storage_->PutEntry(entries[i]);
+    EXPECT_TRUE(storage_->PutEntry(entries[i]));
 
   // Call Iterate and check the result.
   std::map<std::string, DriveEntryProto> result;
@@ -111,49 +169,6 @@ TEST_F(DriveResourceMetadataStorageTest, Iterate) {
     EXPECT_EQ(1U, result.count(entries[i].resource_id()));
 }
 
-TEST_F(DriveResourceMetadataStorageTest, PutChild) {
-  const std::string parent_id1 = "abcdefg";
-  const std::string parent_id2 = "abcd";
-  const std::string child_name1 = "WXYZABC";
-  const std::string child_name2 = "efgWXYZABC";
-  const std::string child_id1 = "qwerty";
-  const std::string child_id2 = "asdfgh";
-
-  // No child found.
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name2).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name2).empty());
-
-  // Put child1 under parent1.
-  storage_->PutChild(parent_id1, child_name1, child_id1);
-  EXPECT_EQ(child_id1, storage_->GetChild(parent_id1, child_name1));
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name2).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name2).empty());
-
-  // Put child2 under parent1.
-  storage_->PutChild(parent_id1, child_name2, child_id2);
-  EXPECT_EQ(child_id1, storage_->GetChild(parent_id1, child_name1));
-  EXPECT_EQ(child_id2, storage_->GetChild(parent_id1, child_name2));
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name2).empty());
-
-  // Remove child1.
-  storage_->RemoveChild(parent_id1, child_name1);
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name1).empty());
-  EXPECT_EQ(child_id2, storage_->GetChild(parent_id1, child_name2));
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name2).empty());
-
-  // Remove child2.
-  storage_->RemoveChild(parent_id1, child_name2);
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id1, child_name2).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name1).empty());
-  EXPECT_TRUE(storage_->GetChild(parent_id2, child_name2).empty());
-}
-
 TEST_F(DriveResourceMetadataStorageTest, GetChildren) {
   const std::string parents_id[] = { "mercury", "venus", "mars", "jupiter",
                                      "saturn" };
@@ -174,21 +189,30 @@ TEST_F(DriveResourceMetadataStorageTest, GetChildren) {
   children_name_id[4].push_back(std::make_pair("titan", "saturn_vi"));
   children_name_id[4].push_back(std::make_pair("iapetus", "saturn_vii"));
 
+  // Put parents.
+  for (size_t i = 0; i < arraysize(parents_id); ++i) {
+    DriveEntryProto entry;
+    entry.set_resource_id(parents_id[i]);
+    EXPECT_TRUE(storage_->PutEntry(entry));
+  }
+
   // Put some data.
-  for (size_t i = 0; i != children_name_id.size(); ++i) {
-    for (size_t j = 0; j != children_name_id[i].size(); ++j) {
-      storage_->PutChild(parents_id[i],
-                         children_name_id[i][j].first,
-                         children_name_id[i][j].second);
+  for (size_t i = 0; i < children_name_id.size(); ++i) {
+    for (size_t j = 0; j < children_name_id[i].size(); ++j) {
+      DriveEntryProto entry;
+      entry.set_parent_resource_id(parents_id[i]);
+      entry.set_base_name(children_name_id[i][j].first);
+      entry.set_resource_id(children_name_id[i][j].second);
+      EXPECT_TRUE(storage_->PutEntry(entry));
     }
   }
 
   // Try to get children.
-  for (size_t i = 0; i != children_name_id.size(); ++i) {
+  for (size_t i = 0; i < children_name_id.size(); ++i) {
     std::vector<std::string> children;
     storage_->GetChildren(parents_id[i], &children);
     EXPECT_EQ(children_name_id[i].size(), children.size());
-    for (size_t j = 0; j != children_name_id[i].size(); ++j) {
+    for (size_t j = 0; j < children_name_id[i].size(); ++j) {
       EXPECT_EQ(1, std::count(children.begin(),
                               children.end(),
                               children_name_id[i][j].second));
@@ -209,9 +233,8 @@ TEST_F(DriveResourceMetadataStorageTest, OpenExistingDB) {
   entry2.set_base_name(child_name1);
 
   // Put some data.
-  storage_->PutEntry(entry1);
-  storage_->PutEntry(entry2);
-  storage_->PutChild(parent_id1, child_name1, child_id1);
+  EXPECT_TRUE(storage_->PutEntry(entry1));
+  EXPECT_TRUE(storage_->PutEntry(entry2));
 
   // Close DB and reopen.
   storage_.reset(new DriveResourceMetadataStorage(temp_dir_.path()));
@@ -241,7 +264,7 @@ TEST_F(DriveResourceMetadataStorageTest, IncompatibleDB) {
 
   // Put some data.
   storage_->SetLargestChangestamp(kLargestChangestamp);
-  storage_->PutEntry(entry);
+  EXPECT_TRUE(storage_->PutEntry(entry));
   EXPECT_TRUE(storage_->GetEntry(key1));
 
   // Set incompatible version and reopen DB.
@@ -279,57 +302,60 @@ TEST_F(DriveResourceMetadataStorageTest, CheckValidity) {
   DriveEntryProto entry;
   entry.set_resource_id(key1);
   entry.set_base_name(name1);
-  storage_->PutEntry(entry);
+  EXPECT_TRUE(storage_->PutEntry(entry));
   EXPECT_TRUE(CheckValidity());
 
   // Put entry with key2 under key1.
   entry.set_resource_id(key2);
   entry.set_parent_resource_id(key1);
   entry.set_base_name(name2);
-  storage_->PutEntry(entry);
+  EXPECT_TRUE(storage_->PutEntry(entry));
+  EXPECT_TRUE(CheckValidity());
+
+  RemoveChild(key1, name2);
   EXPECT_FALSE(CheckValidity());  // Missing parent-child relationship.
 
-  // Add missing parent-child relationship between key1 and key2.
-  storage_->PutChild(key1, name2, key2);
+  // Add back parent-child relationship between key1 and key2.
+  PutChild(key1, name2, key2);
   EXPECT_TRUE(CheckValidity());
 
   // Add parent-child relationship between key2 and key3.
-  storage_->PutChild(key2, name3, key3);
+  PutChild(key2, name3, key3);
   EXPECT_FALSE(CheckValidity());  // key3 is not stored in the storage.
 
   // Put entry with key3 under key2.
   entry.set_resource_id(key3);
   entry.set_parent_resource_id(key2);
   entry.set_base_name(name3);
-  storage_->PutEntry(entry);
+  EXPECT_TRUE(storage_->PutEntry(entry));
   EXPECT_TRUE(CheckValidity());
 
   // Parent-child relationship with wrong name.
-  storage_->RemoveChild(key2, name3);
+  RemoveChild(key2, name3);
   EXPECT_FALSE(CheckValidity());
-  storage_->PutChild(key2, name2, key3);
+  PutChild(key2, name2, key3);
   EXPECT_FALSE(CheckValidity());
 
   // Fix up the relationship between key2 and key3.
-  storage_->RemoveChild(key2, name2);
+  RemoveChild(key2, name2);
   EXPECT_FALSE(CheckValidity());
-  storage_->PutChild(key2, name3, key3);
+  PutChild(key2, name3, key3);
   EXPECT_TRUE(CheckValidity());
 
   // Remove key2.
-  storage_->RemoveChild(key1, name2);
+  RemoveChild(key1, name2);
   EXPECT_FALSE(CheckValidity());
-  storage_->RemoveEntry(key2);
+  EXPECT_TRUE(storage_->RemoveEntry(key2));
   EXPECT_FALSE(CheckValidity());
 
   // Remove key3.
-  storage_->RemoveChild(key2, name3);
+  RemoveChild(key2, name3);
   EXPECT_FALSE(CheckValidity());
-  storage_->RemoveEntry(key3);
+  EXPECT_TRUE(storage_->RemoveEntry(key3));
   EXPECT_TRUE(CheckValidity());
 
   // Remove key1.
-  storage_->RemoveEntry(key1);
+  EXPECT_TRUE(storage_->RemoveEntry(key1));
   EXPECT_TRUE(CheckValidity());
 }
 
diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata_unittest.cc b/chrome/browser/chromeos/drive/drive_resource_metadata_unittest.cc
index effc411b..a5b82dc 100644
--- a/chrome/browser/chromeos/drive/drive_resource_metadata_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_resource_metadata_unittest.cc
@@ -709,11 +709,27 @@ TEST_F(DriveResourceMetadataTest, RefreshEntry) {
   ASSERT_TRUE(!entry_proto->file_info().is_directory());
   EXPECT_EQ("md5:file9", entry_proto->file_specific_info().file_md5());
 
-  // Rename it and change the file size.
+  // Rename it.
   DriveEntryProto file_entry_proto(*entry_proto);
+  file_entry_proto.set_title("file100");
+  entry_proto.reset();
+  resource_metadata_->RefreshEntry(
+      file_entry_proto,
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &drive_file_path, &entry_proto));
+  google_apis::test_util::RunBlockingPoolTask();
+  EXPECT_EQ(FILE_ERROR_OK, error);
+  EXPECT_EQ(base::FilePath::FromUTF8Unsafe("drive/root/dir1/dir3/file100"),
+            drive_file_path);
+  ASSERT_TRUE(entry_proto.get());
+  EXPECT_EQ("file100", entry_proto->base_name());
+  ASSERT_TRUE(!entry_proto->file_info().is_directory());
+  EXPECT_EQ("md5:file9", entry_proto->file_specific_info().file_md5());
+
+  // Update the file md5.
   const std::string updated_md5("md5:updated");
+  file_entry_proto = *entry_proto;
   file_entry_proto.mutable_file_specific_info()->set_file_md5(updated_md5);
-  file_entry_proto.set_title("file100");
   entry_proto.reset();
   resource_metadata_->RefreshEntry(
       file_entry_proto,