drive: Fills no-parent entiries in "/drive/other".

BUG=174233
TEST=unittests. Open Files App and verify Google Drive is available.


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

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

chrome/browser/chromeos/drive/change_list_processor.cc

@@ -109,9 +109,6 @@ void ChangeListProcessor::ApplyFeeds(
     NOTREACHED();
   }
 
-  // TODO(haruki): Add pseudo tree structure for "drive"/root" and "drive/other"
-  // when we start using those namespaces. The root folder ID is necessary for
-  // full feed update.
   ApplyEntryProtoMap(is_delta_feed);
 
   // Shouldn't record histograms when processing delta feeds.
@@ -375,23 +372,12 @@ void ChangeListProcessor::FeedToEntryProtoMap(
       if (entry_proto->resource_id().empty())
         continue;
 
-      // TODO(haruki): Apply mapping from an empty parent to special dummy
-      // directory here or in ConvertResourceEntryToDriveEntryProto. See
-      // http://crbug.com/174233 http://crbug.com/171207. Until we implement it,
-      // ChangeListProcessor ignores such "no parent" entries.
-      // Please note that this will cause a temporal issue when
-      // - The user unselect all the parent using drive.google.com UI.
-      // ChangeListProcessor just ignores the incoming changes and keeps stale
-      // metadata. We need to work on this ASAP to reduce confusion.
-      if (entry_proto->parent_resource_id().empty()) {
-        continue;
-      }
-
       // Count the number of files.
       if (uma_stats && !entry_proto->file_info().is_directory()) {
         uma_stats->IncrementNumFiles(
             entry_proto->file_specific_info().is_hosted_document());
       }
+      // TODO(haruki): Metric for the num of the entries in "other" directory.
 
       std::pair<DriveEntryProtoMap::iterator, bool> ret = entry_proto_map_.
           insert(std::make_pair(entry_proto->resource_id(), DriveEntryProto()));

chrome/browser/chromeos/drive/drive_file_system.cc

@@ -1090,6 +1090,11 @@ void DriveFileSystem::RefreshDirectoryAfterGetEntryInfo(
     callback.Run(DRIVE_FILE_ERROR_NOT_A_DIRECTORY);
     return;
   }
+  if (util::IsSpecialResourceId(entry_proto->resource_id())) {
+    // Do not load special directories. Just return.
+    callback.Run(DRIVE_FILE_OK);
+    return;
+  }
 
   change_list_loader_->LoadDirectoryFromServer(
       entry_proto->resource_id(),

chrome/browser/chromeos/drive/drive_file_system_unittest.cc

@@ -713,6 +713,16 @@ TEST_F(DriveFileSystemTest, SearchInSubSubdir) {
   ASSERT_EQ("folder:sub_sub_directory_folder_id", entry->resource_id());
 }
 
+TEST_F(DriveFileSystemTest, SearchOrphanFile) {
+  ASSERT_TRUE(LoadRootFeedDocument());
+
+  const base::FilePath kFilePath = base::FilePath(
+      FILE_PATH_LITERAL("drive/other/Orphan File 1.txt"));
+  scoped_ptr<DriveEntryProto> entry = GetEntryInfoByPathSync(kFilePath);
+  ASSERT_TRUE(entry.get());
+  EXPECT_EQ("file:1_orphanfile_resource_id", entry->resource_id());
+}
+
 TEST_F(DriveFileSystemTest, ReadDirectoryByPath_Root) {
   EXPECT_CALL(*mock_directory_observer_, OnDirectoryChanged(
       Eq(base::FilePath(FILE_PATH_LITERAL("drive/root"))))).Times(1);

chrome/browser/chromeos/drive/resource_entry_conversion.cc

@@ -52,15 +52,20 @@ DriveEntryProto ConvertResourceEntryToDriveEntryProto(
   if (edit_link)
     entry_proto.set_edit_url(edit_link->href().spec());
 
+  // Sets parent Resource ID. On drive.google.com, a file can have multiple
+  // parents or no parent, but we are forcing a tree-shaped structure (i.e. no
+  // multi-parent or zero-parent entries). Therefore the first found "parent" is
+  // used for the entry and if the entry has no parent, we assign a special ID
+  // which represents no-parent entries. Tracked in http://crbug.com/158904.
   const google_apis::Link* parent_link =
       entry.GetLinkByType(google_apis::Link::LINK_PARENT);
   if (parent_link) {
-    // TODO(haruki): Apply mapping from an empty parent to special dummy
-    // directory. See http://crbug.com/174233. Until we implement it,
-    // ChangeListProcessor ignores such "no parent" entries.
     entry_proto.set_parent_resource_id(
         util::ExtractResourceIdFromUrl(parent_link->href()));
   }
+  // Apply mapping from an empty parent to the special dummy directory.
+  if (entry_proto.parent_resource_id().empty())
+    entry_proto.set_parent_resource_id(util::kDriveOtherDirSpecialResourceId);
 
   entry_proto.set_deleted(entry.deleted());
   entry_proto.set_shared_with_me(HasSharedWithMeLabel(entry));

chrome/browser/chromeos/drive/resource_entry_conversion_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/files/file_path.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
+#include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 #include "chrome/browser/chromeos/drive/drive_test_util.h"
 #include "chrome/browser/google_apis/gdata_wapi_parser.h"
 #include "googleurl/src/gurl.h"
@@ -32,7 +33,8 @@ TEST(ResourceEntryConversionTest, ConvertResourceEntryToDriveEntryProto_File) {
   EXPECT_EQ("https://file_content_url/",  entry_proto.download_url());
   EXPECT_EQ("https://file1_link_self/file:2_file_resource_id",
             entry_proto.edit_url());
-  EXPECT_EQ("",  entry_proto.parent_resource_id());
+  EXPECT_EQ(util::kDriveOtherDirSpecialResourceId,
+            entry_proto.parent_resource_id());
 
   EXPECT_FALSE(entry_proto.deleted());
   EXPECT_FALSE(entry_proto.shared_with_me());
@@ -113,7 +115,8 @@ TEST(ResourceEntryConversionTest,
   EXPECT_EQ("https://3_document_content/",  entry_proto.download_url());
   EXPECT_EQ("https://3_document_self_link/document:5_document_resource_id",
             entry_proto.edit_url());
-  EXPECT_EQ("",  entry_proto.parent_resource_id());
+  EXPECT_EQ(util::kDriveOtherDirSpecialResourceId,
+            entry_proto.parent_resource_id());
 
   EXPECT_FALSE(entry_proto.deleted());
   EXPECT_FALSE(entry_proto.shared_with_me());
@@ -271,7 +274,8 @@ TEST(ResourceEntryConversionTest,
   EXPECT_EQ("https://content_url/",  entry_proto.download_url());
   EXPECT_EQ("https://edit_url/document%3Adeleted_in_root_id",
             entry_proto.edit_url());
-  EXPECT_EQ("",  entry_proto.parent_resource_id());
+  EXPECT_EQ(util::kDriveOtherDirSpecialResourceId,
+            entry_proto.parent_resource_id());
 
   EXPECT_TRUE(entry_proto.deleted());  // The document was deleted.
   EXPECT_FALSE(entry_proto.shared_with_me());