Remove root resource id aliasing from DriveResourceMetadata.

chrome/browser/chromeos/drive/drive_file_system_unittest.cc

Index: chrome/browser/chromeos/drive/drive_file_system_unittest.cc
diff --git a/chrome/browser/chromeos/drive/drive_file_system_unittest.cc b/chrome/browser/chromeos/drive/drive_file_system_unittest.cc
index 2855c8fdd943bb06ac0a63219bda3e793b91c149..c66b7b51a549f9cb841843f17c81368e333c8489 100644
--- a/chrome/browser/chromeos/drive/drive_file_system_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_file_system_unittest.cc
@@ -142,7 +142,6 @@ class DriveFileSystemTest : public testing::Test {
 
   void SetUpResourceMetadataAndFileSystem() {
     resource_metadata_.reset(new DriveResourceMetadata(
-        fake_drive_service_->GetRootResourceId(),
         cache_->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_META),
         blocking_task_runner_));
 
@@ -319,7 +318,6 @@ class DriveFileSystemTest : public testing::Test {
         fake_drive_service_->GetRootResourceId();
     scoped_ptr<DriveResourceMetadata, test_util::DestroyHelperForTests>
         resource_metadata(new DriveResourceMetadata(
-            root_resource_id,
             cache_->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_META),
             blocking_task_runner_));
 
@@ -342,6 +340,14 @@ class DriveFileSystemTest : public testing::Test {
     // ChangeListLoader.
     base::FilePath file_path;
 
+    // drive/root
+    resource_metadata->AddEntry(
+        util::CreateMyDriveRootEntry(root_resource_id),
+        google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+    google_apis::test_util::RunBlockingPoolTask();
+    if (error != DRIVE_FILE_OK)
+      return false;
+
     // drive/root/File1
     DriveEntryProto file1;
     file1.set_title("File1");
@@ -484,7 +490,7 @@ void AsyncInitializationCallback(
 TEST_F(DriveFileSystemTest, DuplicatedAsyncInitialization) {

[kinaba, 2013/04/10 04:34:44]

As far as I recall, original intent of this test was to
assure that more than one full feed fetch will never run parallel.

Probably we should use GetEntryInfoByPath("drive/root")
instead of ReadDirectoryByPath("drive") or ReadDirectoryByPath("drive/root"),
so that it is not complicated by the fast-fetch mode.

[hidehiko, 2013/04/10 06:11:19]

Good to know. Done. Also removed EXPECT_CALL below, because full fetch won't
trigger the OnDirectoryChanged event.

   // The root directory will be loaded that triggers the event.
   EXPECT_CALL(*mock_directory_observer_, OnDirectoryChanged(
-      Eq(base::FilePath(FILE_PATH_LITERAL("drive/root"))))).Times(1);
+      Eq(base::FilePath(FILE_PATH_LITERAL("drive"))))).Times(1);
 
   int counter = 0;
   ReadDirectoryWithSettingCallback callback = base::Bind(
@@ -494,16 +500,19 @@ TEST_F(DriveFileSystemTest, DuplicatedAsyncInitialization) {
       &message_loop_);
 
   file_system_->ReadDirectoryByPath(
-      base::FilePath(FILE_PATH_LITERAL("drive/root")), callback);
+      base::FilePath(FILE_PATH_LITERAL("drive")), callback);
   file_system_->ReadDirectoryByPath(
-      base::FilePath(FILE_PATH_LITERAL("drive/root")), callback);
+      base::FilePath(FILE_PATH_LITERAL("drive")), callback);
   message_loop_.Run();  // Wait to get our result
   EXPECT_EQ(2, counter);
 
   // ReadDirectoryByPath() was called twice, but the account metadata should
   // only be loaded once. In the past, there was a bug that caused it to be
   // loaded twice.
-  EXPECT_EQ(1, fake_drive_service_->about_resource_load_count());
+  // The actual number of GetAboutResource invocation should be two;
+  // one for checking if it is necessary to load the data from server or not,
+  // and the other for creating mydrive root entry.
+  EXPECT_EQ(2, fake_drive_service_->about_resource_load_count());
   // On the other hand, the resource list could be loaded twice. One for
   // just the directory contents, and one for the entire resource list.
   //
@@ -535,10 +544,9 @@ TEST_F(DriveFileSystemTest, GetMyDriveRootEntry) {
   ASSERT_TRUE(entry.get());
   EXPECT_EQ(fake_drive_service_->GetRootResourceId(), entry->resource_id());
 
-  // Getting the "My Drive" root entry should not cause the resource load to
-  // happen.
-  EXPECT_EQ(0, fake_drive_service_->about_resource_load_count());
-  EXPECT_EQ(0, fake_drive_service_->resource_list_load_count());
+  // Getting the "My Drive" root entry should cause the resource load to happen.

[kinaba, 2013/04/10 04:34:44]

As far as I recall, original intent of this test was to
assure that GetEntryInfoByPath to an existing entry does not
cause remote query.

Probably it is better to change kFilePath="drive" and adjust
expectations accordingly.

[hidehiko, 2013/04/10 06:11:19]

Good to know. Just removed this case. Note that the /drive entry is tested
GetGrandRootEntry above.

+  EXPECT_EQ(1, fake_drive_service_->about_resource_load_count());
+  EXPECT_EQ(1, fake_drive_service_->resource_list_load_count());
 }
 
 TEST_F(DriveFileSystemTest, GetOtherDirEntry) {
@@ -725,14 +733,14 @@ TEST_F(DriveFileSystemTest, SearchOrphanFile) {
 
 TEST_F(DriveFileSystemTest, ReadDirectoryByPath_Root) {
   EXPECT_CALL(*mock_directory_observer_, OnDirectoryChanged(
-      Eq(base::FilePath(FILE_PATH_LITERAL("drive/root"))))).Times(1);
+      Eq(base::FilePath(FILE_PATH_LITERAL("drive"))))).Times(1);
 
   // ReadDirectoryByPath() should kick off the resource list loading.
   scoped_ptr<DriveEntryProtoVector> entries(
-      ReadDirectoryByPathSync(base::FilePath::FromUTF8Unsafe("drive/root")));
+      ReadDirectoryByPathSync(base::FilePath::FromUTF8Unsafe("drive")));
   // The root directory should be read correctly.
   ASSERT_TRUE(entries.get());
-  EXPECT_EQ(8U, entries->size());
+  EXPECT_EQ(2U, entries->size());
 }

[kinaba, 2013/04/10 04:34:44]

May want to add a test verifying that "drive/root" exists
after reading "drive".

[hidehiko, 2013/04/10 06:11:19]

Done.

 
 TEST_F(DriveFileSystemTest, ReadDirectoryByPath_NonRootDirectory) {