Report all Drive search results even if resource metadata is not fully loaded.

By this patch, a search result entry whose metadata is not fetched yet is
temporarily added to the hidden "drive/other" directory for making it possible
to appear in the search result UI. Normal feed loading routine will move it
back to the right place once the real metadata is loaded.

BUG=181075
TEST=Search "a" during the initial load of an account with large number of files
R=hashimoto@chromium.org

Review URL: https://codereview.chromium.org/16142003

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

chrome/browser/chromeos/drive/file_system/search_operation.cc

@@ -39,28 +39,30 @@ FileError RefreshEntriesOnBlockingPool(
   result->reserve(entries.size());
   for (size_t i = 0; i < entries.size(); ++i) {
     ResourceEntry entry = ConvertToResourceEntry(*entries[i]);
-    base::FilePath drive_file_path;
-    FileError error = resource_metadata->RefreshEntry(
-        entry, &drive_file_path, &entry);
-    if (error == FILE_ERROR_OK) {
-      result->push_back(SearchResultInfo(drive_file_path, entry));
-    } else if (error == FILE_ERROR_NOT_FOUND) {
-      // The result is absent in local resource metadata. There are two cases:
+    const std::string id = entry.resource_id();
+    FileError error = resource_metadata->RefreshEntry(entry, NULL, &entry);
+    if (error == FILE_ERROR_NOT_FOUND) {
+      // The result is absent in local resource metadata. This can happen if
+      // the metadata is not synced to the latest server state yet. In that
+      // case, we temporarily add the file to the special "drive/other"
+      // directory in order to assign a path, which is needed to access the
+      // file through FileSystem API.
       //
-      // 1) Resource metadata is not up-to-date, and the entry has recently
-      //    been added to the drive. This is not a fatal error, so just skip to
-      //    add the result. We should soon receive XMPP update notification
-      //    and refresh both the metadata and search result UI in Files.app.
-      //
-      // 2) Resource metadata is not fully loaded.
-      // TODO(kinaba) crbug.com/181075:
-      //    In this case, we are doing "fast fetch" fetching directory lists on
-      //    the fly to quickly deliver results to the user. However, we have no
-      //    such equivalent for Search.
-    } else {
-      // Otherwise, it is a fatal error. Give up to return the search result.
-      return error;
+      // It will be moved to the right place when the metadata gets synced
+      // in normal loading process in ChangeListProcessor.
+      entry.set_parent_resource_id(util::kDriveOtherDirSpecialResourceId);
+      error = resource_metadata->AddEntry(entry);
+
+      // FILE_ERROR_EXISTS may happen if we have already added the entry to
+      // "drive/other" once before. That's not an error.
+      if (error == FILE_ERROR_OK || error == FILE_ERROR_EXISTS)
+        error = resource_metadata->GetResourceEntryById(id, &entry);
     }
+    // Other errors are fatal. Give up to return the search result.
+    if (error != FILE_ERROR_OK)
+      return error;
+    result->push_back(SearchResultInfo(resource_metadata->GetFilePath(id),
+                                       entry));
   }
 
   return FILE_ERROR_OK;

chrome/browser/chromeos/drive/file_system/search_operation_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/drive/file_system/search_operation.h"
 
+#include "chrome/browser/chromeos/drive/change_list_loader.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_test_base.h"
 #include "chrome/browser/google_apis/fake_drive_service.h"
 #include "chrome/browser/google_apis/gdata_wapi_parser.h"
@@ -70,11 +71,11 @@ TEST_F(SearchOperationTest, ContentSearchWithNewEntry) {
   ASSERT_EQ(google_apis::HTTP_CREATED, gdata_error);
 
   // As the result of the first Search(), only entries in the current file
-  // system snapshot are expected to be returned (i.e. "New Directory 1!"
-  // shouldn't be included in the search result even though it matches
-  // "Directory 1".
-  const SearchResultPair kExpectedResults[] = {
-      { "drive/root/Directory 1", true }
+  // system snapshot are expected to be returned in the "right" path. New
+  // entries like "New Directory 1!" is temporarily added to "drive/other".
+  const SearchResultPair kExpectedResultsBeforeLoad[] = {
+      { "drive/root/Directory 1", true },
+      { "drive/other/New Directory 1!", true },
   };
 
   FileError error = FILE_ERROR_FAILED;
@@ -88,8 +89,41 @@ TEST_F(SearchOperationTest, ContentSearchWithNewEntry) {
 
   EXPECT_EQ(FILE_ERROR_OK, error);
   EXPECT_EQ(GURL(), next_feed);
-  ASSERT_EQ(1U, results->size());
-  EXPECT_EQ(kExpectedResults[0].path, results->at(0).path.AsUTF8Unsafe());
+  ASSERT_EQ(ARRAYSIZE_UNSAFE(kExpectedResultsBeforeLoad), results->size());
+  for (size_t i = 0; i < results->size(); i++) {
+    EXPECT_EQ(kExpectedResultsBeforeLoad[i].path,
+              results->at(i).path.AsUTF8Unsafe());
+    EXPECT_EQ(kExpectedResultsBeforeLoad[i].is_directory,
+              results->at(i).entry.file_info().is_directory());
+  }
+
+  // Load the change from FakeDriveService.
+  internal::ChangeListLoader change_list_loader(
+      blocking_task_runner(), metadata(), scheduler());
+  change_list_loader.CheckForUpdates(
+      google_apis::test_util::CreateCopyResultCallback(&error));
+  google_apis::test_util::RunBlockingPoolTask();
+
+  // Now the new entry must be reported to be in the right directory.
+  const SearchResultPair kExpectedResultsAfterLoad[] = {
+      { "drive/root/Directory 1", true },
+      { "drive/root/New Directory 1!", true },
+  };
+  error = FILE_ERROR_FAILED;
+  operation.Search("\"Directory 1\"", GURL(),
+                   google_apis::test_util::CreateCopyResultCallback(
+                       &error, &next_feed, &results));
+  google_apis::test_util::RunBlockingPoolTask();
+
+  EXPECT_EQ(FILE_ERROR_OK, error);
+  EXPECT_EQ(GURL(), next_feed);
+  ASSERT_EQ(ARRAYSIZE_UNSAFE(kExpectedResultsAfterLoad), results->size());
+  for (size_t i = 0; i < results->size(); i++) {
+    EXPECT_EQ(kExpectedResultsAfterLoad[i].path,
+              results->at(i).path.AsUTF8Unsafe());
+    EXPECT_EQ(kExpectedResultsAfterLoad[i].is_directory,
+              results->at(i).entry.file_info().is_directory());
+  }
 }
 
 TEST_F(SearchOperationTest, ContentSearchEmptyResult) {