Remove root resource id aliasing from DriveResourceMetadata.

chrome/browser/chromeos/drive/change_list_loader.cc

Index: chrome/browser/chromeos/drive/change_list_loader.cc
diff --git a/chrome/browser/chromeos/drive/change_list_loader.cc b/chrome/browser/chromeos/drive/change_list_loader.cc
index af3136d7578ae0111a3f65272f89ae723c24aef0..0081b36faa7fa4c2f05c2a96327688a0a464b7cb 100644
--- a/chrome/browser/chromeos/drive/change_list_loader.cc
+++ b/chrome/browser/chromeos/drive/change_list_loader.cc
@@ -187,7 +187,8 @@ void ChangeListLoader::CompareChangestampsAndLoadIfNeeded(
                              start_changestamp,
                              callback);
   } else if (directory_fetch_info.changestamp() < remote_changestamp &&
-             !util::IsSpecialResourceId(directory_fetch_info.resource_id())) {
+             directory_fetch_info.resource_id() !=
+                 util::kDriveOtherDirSpecialResourceId) {
     // If the caller is interested in a particular directory, and the
     // directory changestamp is older than server's, start loading the
     // directory first. Skip special entries as they are not meaningful in the
@@ -341,13 +342,31 @@ void ChangeListLoader::DoLoadDirectoryFromServer(
   DCHECK(!directory_fetch_info.empty());
   DVLOG(1) << "Start loading directory: " << directory_fetch_info.ToString();
 
-  if (util::IsSpecialResourceId(directory_fetch_info.resource_id())) {
-    // Load for a special directory is meaningless in the server.
+  if (directory_fetch_info.resource_id() ==
+          util::kDriveOtherDirSpecialResourceId) {
+    // Load for a <other> directory is meaningless in the server.
     // Let it succeed and use what we have locally.
     callback.Run(DRIVE_FILE_OK);
     return;
   }
 
+  if (directory_fetch_info.resource_id() ==
+          util::kDriveGrandRootSpecialResourceId) {
+    // Load for a grand root directory means slightly different from other
+    // directories. It should have two directories; <other> and mydrive root.
+    // <other> directory should always exists, but mydrive root should be

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

nit: exists -> exist

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

Done.

+    // created by root resource id retreived from the server.

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

retrieved

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

Done.

+    // Here, we check if mydrive root exists, and if not, create it.
+    resource_metadata_->GetEntryInfoByPath(
+        base::FilePath(util::kDriveMyDriveRootPath),
+        base::Bind(
+            &ChangeListLoader::DoLoadDirectoryFromServerAfterGetEntryInfoByPath,
+            weak_ptr_factory_.GetWeakPtr(),
+            directory_fetch_info,
+            callback));
+    return;
+  }
+
   scoped_ptr<LoadFeedParams> params(new LoadFeedParams);
   params->directory_resource_id = directory_fetch_info.resource_id();
   LoadFromServer(
@@ -358,6 +377,62 @@ void ChangeListLoader::DoLoadDirectoryFromServer(
                  callback));
 }
 
+void ChangeListLoader::DoLoadDirectoryFromServerAfterGetEntryInfoByPath(
+    const DirectoryFetchInfo& directory_fetch_info,
+    const FileOperationCallback& callback,
+    DriveFileError error,
+    scoped_ptr<DriveEntryProto> entry_proto) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+  DCHECK(!directory_fetch_info.empty());

[Haruki Sato, 2013/04/10 03:44:35]

Also this method assumes it's request for <drive>?
Is it good to denote that here as well?

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

Done.

+
+  if (error == DRIVE_FILE_OK) {
+    // MyDrive root already exists. Just return success.
+    callback.Run(DRIVE_FILE_OK);
+    return;
+  }
+
+  // Fetch root resource id from the server.
+  scheduler_->GetAboutResource(
+      base::Bind(
+          &ChangeListLoader::DoLoadDirectoryFromServerAfterGetAboutResource,
+          weak_ptr_factory_.GetWeakPtr(),
+          directory_fetch_info,
+          callback));
+}
+
+void ChangeListLoader::DoLoadDirectoryFromServerAfterGetAboutResource(
+    const DirectoryFetchInfo& directory_fetch_info,
+    const FileOperationCallback& callback,
+    google_apis::GDataErrorCode status,
+    scoped_ptr<google_apis::AboutResource> about_resource) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+  DCHECK(!directory_fetch_info.empty());
+
+  DriveFileError error = util::GDataToDriveFileError(status);
+  if (error != DRIVE_FILE_OK) {
+    callback.Run(error);
+    return;
+  }
+
+  // Build entry proto map for grand root directory, which has two entries;
+  // "/drive/root" and "/drive/other".
+  DriveEntryProtoMap grand_root_entry_proto_map;
+  const std::string& root_resource_id = about_resource->root_folder_id();
+  grand_root_entry_proto_map[root_resource_id] =
+      util::CreateMyDriveRootEntry(root_resource_id);
+  grand_root_entry_proto_map[util::kDriveOtherDirSpecialResourceId] =
+      util::CreateOtherDirEntry();
+  resource_metadata_->RefreshDirectory(

[Haruki Sato, 2013/04/10 03:44:35]

Any reason to use RefreshDirectory rather than AddEntry?
Can we remove the creation for "other" in DriveResourceMetadata with this?

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

Two reasons:
1) AddEntry returns an error if the entry already exists. It is not what we want
here. Actually, there is a change for such a case; During the fetching
AboutResource from the server, full-fetch may complete successfully.
2) I want to keep the same manner for fast-fetch of other directories, here.
Without an entry for drive/other, it would be removed.

On 2013/04/10 03:44:35, Haruki Sato wrote:

+      directory_fetch_info,
+      grand_root_entry_proto_map,
+      base::Bind(&ChangeListLoader::DoLoadDirectoryFromServerAfterRefresh,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 directory_fetch_info,
+                 callback));
+}
+
 void ChangeListLoader::DoLoadDirectoryFromServerAfterLoad(
     const DirectoryFetchInfo& directory_fetch_info,
     const FileOperationCallback& callback,