Commit ec96b829 authored by tbarzic@chromium.org's avatar tbarzic@chromium.org

[FileManager] Do drive search incrementally

Instead of fetching whole drive  search result feed at once, do it incrementally, (max) 100 documents at the time.

This way we get some results sooner (while the rest of the results is being fetched), so search feels more smooth.

BUG=138274
TEST=manual

TBR=darin

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149995 0039d316-1c4b-4281-b951-d872f2087c98
parent b87935ec
......@@ -2305,10 +2305,17 @@ bool SetGDataPreferencesFunction::RunImpl() {
return true;
}
SearchDriveFunction::SearchDriveFunction() {}
SearchDriveFunction::~SearchDriveFunction() {}
bool SearchDriveFunction::RunImpl() {
if (!args_->GetString(0, &query_))
return false;
if (!args_->GetString(1, &next_feed_))
return false;
BrowserContext::GetFileSystemContext(profile())->OpenFileSystem(
source_url_.GetOrigin(), fileapi::kFileSystemTypeExternal, false,
base::Bind(&SearchDriveFunction::OnFileSystemOpened, this));
......@@ -2335,12 +2342,13 @@ void SearchDriveFunction::OnFileSystemOpened(
}
system_service->file_system()->Search(
query_,
query_, GURL(next_feed_),
base::Bind(&SearchDriveFunction::OnSearch, this));
}
void SearchDriveFunction::OnSearch(
gdata::GDataFileError error,
const GURL& next_feed,
scoped_ptr<std::vector<gdata::SearchResultInfo> > results) {
if (error != gdata::GDATA_FILE_OK) {
SendResponse(false);
......@@ -2361,7 +2369,11 @@ void SearchDriveFunction::OnSearch(
entries->Append(entry);
}
SetResult(entries);
base::DictionaryValue* result = new DictionaryValue();
result->Set("entries", entries);
result->SetString("nextFeed", next_feed.spec());
SetResult(result);
SendResponse(true);
}
......
......@@ -648,8 +648,10 @@ class SearchDriveFunction : public AsyncExtensionFunction {
public:
DECLARE_EXTENSION_FUNCTION_NAME("fileBrowserPrivate.searchGData");
SearchDriveFunction();
protected:
virtual ~SearchDriveFunction() {}
virtual ~SearchDriveFunction();
virtual bool RunImpl() OVERRIDE;
......@@ -660,10 +662,12 @@ class SearchDriveFunction : public AsyncExtensionFunction {
const GURL& file_system_url);
// Callback for gdata::SearchAsync called after file system is opened.
void OnSearch(gdata::GDataFileError error,
const GURL& next_feed,
scoped_ptr<std::vector<gdata::SearchResultInfo> > result_paths);
// Query for which the search is being performed.
std::string query_;
std::string next_feed_;
// Information about remote file system we will need to create file entries
// to represent search results.
std::string file_system_name_;
......
......@@ -401,6 +401,7 @@ void AddEntryToSearchResults(
const base::Closure& entry_skipped_callback,
GDataFileError error,
bool run_callback,
const GURL& next_feed,
GDataEntry* entry) {
// If a result is not present in our local file system snapshot, invoke
// |entry_skipped_callback| and refreshes the snapshot with delta feed.
......@@ -417,7 +418,7 @@ void AddEntryToSearchResults(
if (run_callback) {
scoped_ptr<std::vector<SearchResultInfo> > result_vec(results);
if (!callback.is_null())
callback.Run(error, result_vec.Pass());
callback.Run(error, next_feed, result_vec.Pass());
}
}
......@@ -1022,6 +1023,7 @@ void GDataWapiFeedLoader::OnGetAccountMetadata(
true, /* should_fetch_multiple_feeds */
search_file_path,
std::string() /* no search query */,
GURL(), /* feed not explicitly set */
std::string() /* no directory resource ID */,
callback,
base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
......@@ -1051,6 +1053,7 @@ void GDataWapiFeedLoader::OnGetAccountMetadata(
true, /* should_fetch_multiple_feeds */
search_file_path,
std::string() /* no search query */,
GURL(), /* feed not explicitly set */
std::string() /* no directory resource ID */,
callback,
base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
......@@ -1091,6 +1094,7 @@ void GDataWapiFeedLoader::OnGetAccountMetadata(
true, /* should_fetch_multiple_feeds */
search_file_path,
std::string() /* no search query */,
GURL(), /* feed not explicitly set */
std::string() /* no directory resource ID */,
callback,
base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
......@@ -1104,6 +1108,7 @@ void GDataWapiFeedLoader::LoadFromServer(
bool should_fetch_multiple_feeds,
const FilePath& search_file_path,
const std::string& search_query,
const GURL& feed_to_load,
const std::string& directory_resource_id,
const FindEntryCallback& entry_found_callback,
const LoadDocumentFeedCallback& feed_load_callback) {
......@@ -1115,7 +1120,7 @@ void GDataWapiFeedLoader::LoadFromServer(
new std::vector<DocumentFeed*>);
const base::TimeTicks start_time = base::TimeTicks::Now();
documents_service_->GetDocuments(
GURL(), // root feed start.
feed_to_load,
start_changestamp,
search_query,
directory_resource_id,
......@@ -2407,6 +2412,7 @@ void GDataFileSystem::RequestDirectoryRefreshOnUIThreadAfterGetEntryInfo(
true, // multiple feeds
file_path,
std::string(), // No search query
GURL(), /* feed not explicitly set */
entry_proto->resource_id(),
FindEntryCallback(), // Not used.
base::Bind(&GDataFileSystem::OnRequestDirectoryRefresh,
......@@ -2729,7 +2735,7 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
if (error != GDATA_FILE_OK) {
if (!callback.is_null())
callback.Run(error, scoped_ptr<std::vector<SearchResultInfo> >());
callback.Run(error, GURL(), scoped_ptr<std::vector<SearchResultInfo> >());
return;
}
......@@ -2741,10 +2747,14 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
DCHECK_EQ(1u, params->feed_list->size());
DocumentFeed* feed = params->feed_list->at(0);
// TODO(tbarzic): Limit total number of returned results for the query.
GURL next_feed;
feed->GetNextFeedURL(&next_feed);
if (feed->entries().empty()) {
scoped_ptr<std::vector<SearchResultInfo> > result_vec(results);
if (!callback.is_null())
callback.Run(error, result_vec.Pass());
callback.Run(error, next_feed, result_vec.Pass());
return;
}
......@@ -2781,22 +2791,26 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
callback,
base::Bind(&GDataFileSystem::CheckForUpdates, ui_weak_ptr_),
error,
i+1 == feed->entries().size()));
i+1 == feed->entries().size(),
next_feed));
}
}
void GDataFileSystem::Search(const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
BrowserThread::CurrentlyOn(BrowserThread::IO));
RunTaskOnUIThread(base::Bind(&GDataFileSystem::SearchAsyncOnUIThread,
ui_weak_ptr_,
search_query,
next_feed,
CreateRelayCallback(callback)));
}
void GDataFileSystem::SearchAsyncOnUIThread(
const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
scoped_ptr<std::vector<DocumentFeed*> > feed_list(
......@@ -2812,6 +2826,7 @@ void GDataFileSystem::SearchAsyncOnUIThread(
// results (especially since we don't cache them).
FilePath(), // Not used.
search_query,
next_feed,
std::string(), // No directory resource ID.
FindEntryCallback(), // Not used.
base::Bind(&GDataFileSystem::OnSearch, ui_weak_ptr_, callback));
......
......@@ -109,6 +109,7 @@ class GDataWapiFeedLoader {
// after we retrieve first feed chunk.
// If invoked as a part of content search, query will be set in
// |search_query|.
// If |feed_to_load| is set, this is feed url that will be used to load feed.
void LoadFromServer(
ContentOrigin initial_origin,
int start_changestamp,
......@@ -116,6 +117,7 @@ class GDataWapiFeedLoader {
bool should_fetch_multiple_feeds,
const FilePath& search_file_path,
const std::string& search_query,
const GURL& feed_to_load,
const std::string& directory_resource_id,
const FindEntryCallback& entry_found_callback,
const LoadDocumentFeedCallback& feed_load_callback);
......@@ -220,6 +222,7 @@ class GDataFileSystem : public GDataFileSystemInterface,
const std::string& resource_id,
const GetEntryInfoWithFilePathCallback& callback) OVERRIDE;
virtual void Search(const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback) OVERRIDE;
virtual void TransferFileFromRemoteToLocal(
const FilePath& remote_src_file_path,
......@@ -822,6 +825,7 @@ class GDataFileSystem : public GDataFileSystemInterface,
// The following functions are used to forward calls to asynchronous public
// member functions to UI thread.
void SearchAsyncOnUIThread(const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback);
void OpenFileOnUIThread(const FilePath& file_path,
const OpenFileCallback& callback);
......
......@@ -70,6 +70,7 @@ typedef base::Callback<void(GDataFileError error,
// If |error| is not PLATFORM_FILE_OK, |result_paths| is empty.
typedef base::Callback<void(
GDataFileError error,
const GURL& next_feed,
scoped_ptr<std::vector<SearchResultInfo> > result_paths)> SearchCallback;
// Used to open files from the file system. |file_path| is the path on the local
......@@ -327,11 +328,13 @@ class GDataFileSystemInterface {
virtual void RequestDirectoryRefresh(const FilePath& file_path) = 0;
// Does server side content search for |search_query|.
// If |next_feed| is set, this is the feed url that will be fetched.
// Search results will be returned as a list of results' |SearchResultInfo|
// structs, which contains file's path and is_directory flag.
//
// Can be called from UI/IO thread. |callback| is run on the calling thread.
virtual void Search(const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback) = 0;
// Fetches the user's Account Metadata to find out current quota information
......
......@@ -58,6 +58,7 @@ void DriveSearchCallback(
const SearchResultPair* expected_results,
size_t expected_results_size,
GDataFileError error,
const GURL& next_feed,
scoped_ptr<std::vector<SearchResultInfo> > results) {
ASSERT_TRUE(results.get());
ASSERT_EQ(expected_results_size, results->size());
......@@ -2451,7 +2452,7 @@ TEST_F(GDataFileSystemTest, ContentSearch) {
SearchCallback callback = base::Bind(&DriveSearchCallback,
&message_loop_, kExpectedResults, ARRAYSIZE_UNSAFE(kExpectedResults));
file_system_->Search("foo", callback);
file_system_->Search("foo", GURL(), callback);
message_loop_.Run(); // Wait to get our result.
}
......@@ -2484,7 +2485,7 @@ TEST_F(GDataFileSystemTest, ContentSearchWithNewEntry) {
SearchCallback callback = base::Bind(&DriveSearchCallback,
&message_loop_, kExpectedResults, ARRAYSIZE_UNSAFE(kExpectedResults));
file_system_->Search("foo", callback);
file_system_->Search("foo", GURL(), callback);
message_loop_.Run(); // Wait to get our result.
}
......@@ -2501,7 +2502,7 @@ TEST_F(GDataFileSystemTest, ContentSearchEmptyResult) {
SearchCallback callback = base::Bind(&DriveSearchCallback,
&message_loop_, expected_results, 0u);
file_system_->Search("foo", callback);
file_system_->Search("foo", GURL(), callback);
message_loop_.Run(); // Wait to get our result.
}
......
......@@ -70,8 +70,10 @@ const char kUploadContentLength[] = "X-Upload-Content-Length: ";
// almost always. Be careful not to use something too small on account that
// have many items because server side 503 error might kick in.
const int kMaxDocumentsPerFeed = 500;
const int kMaxDocumentsPerSearchFeed = 50;
#else
const int kMaxDocumentsPerFeed = 500;
const int kMaxDocumentsPerSearchFeed = 50;
#endif
const char kFeedField[] = "feed";
......@@ -171,15 +173,18 @@ void GetDocumentsOperation::SetUrl(const GURL& url) {
}
GURL GetDocumentsOperation::GetURL() const {
int max_docs = search_string_.empty() ? kMaxDocumentsPerFeed :
kMaxDocumentsPerSearchFeed;
if (!override_url_.is_empty())
return AddFeedUrlParams(override_url_,
kMaxDocumentsPerFeed,
max_docs,
0,
std::string());
search_string_);
if (start_changestamp_ == 0) {
return AddFeedUrlParams(FormatDocumentListURL(directory_resource_id_),
kMaxDocumentsPerFeed,
max_docs,
0,
search_string_);
}
......
......@@ -30,7 +30,8 @@ class MockGDataFileSystem : public GDataFileSystemInterface {
MOCK_METHOD2(GetEntryInfoByResourceId,
void(const std::string& resource_id,
const GetEntryInfoWithFilePathCallback& callback));
MOCK_METHOD2(Search, void(const std::string& search_query,
MOCK_METHOD3(Search, void(const std::string& search_query,
const GURL& next_feed,
const SearchCallback& callback));
MOCK_METHOD3(TransferFileFromRemoteToLocal,
void(const FilePath& local_src_file_path,
......
......@@ -354,6 +354,18 @@ DirectoryContentsBasic.prototype.createDirectory = function(
onSuccess.bind(this), errorCallback);
};
/**
* Delay to be used for gdata search scan.
* The goal is to reduce the number of server requests when user is typing the
* query.
*/
DirectoryContentsGDataSearch.SCAN_DELAY = 200;
/**
* Number of results at which we stop the search.
* Note that max number of shown results is MAX_RESULTS + search feed size.
*/
DirectoryContentsGDataSearch.MAX_RESULTS = 999;
/**
* @constructor
......@@ -366,6 +378,9 @@ function DirectoryContentsGDataSearch(context, dirEntry, query) {
DirectoryContents.call(this, context);
this.query_ = query;
this.directoryEntry_ = dirEntry;
this.nextFeed_ = '';
this.done_ = false;
this.fetchedResultsNum_ = 0;
}
/**
......@@ -407,8 +422,9 @@ DirectoryContentsGDataSearch.prototype.getPath = function() {
* Start directory scan.
*/
DirectoryContentsGDataSearch.prototype.scan = function() {
chrome.fileBrowserPrivate.searchGData(this.query_,
this.onNewEntries.bind(this));
// Let's give another search a chance to cancel us before we begin.
setTimeout(this.readNextChunk.bind(this),
DirectoryContentsGDataSearch.SCAN_DELAY);
};
/**
......@@ -416,7 +432,33 @@ DirectoryContentsGDataSearch.prototype.scan = function() {
* it means we're done.
*/
DirectoryContentsGDataSearch.prototype.readNextChunk = function() {
this.onCompleted();
if (this.scanCancelled_)
return;
if (this.done_) {
this.onCompleted();
return;
}
var searchCallback = (function(entries, nextFeed) {
// TODO(tbarzic): Improve error handling.
if (!entries) {
console.log('Drive search encountered an error');
this.onCompleted();
return;
}
this.nextFeed_ = nextFeed;
this.fetchedResultsNum_ += entries.length;
if (this.fetchedResultsNum_ >= DirectoryContentsGDataSearch.MAX_RESULTS)
this.nextFeed_ = '';
this.done_ = (this.nextFeed_ == '');
this.onNewEntries(entries);
}).bind(this);
chrome.fileBrowserPrivate.searchGData(this.query_,
this.nextFeed_,
searchCallback);
};
......
......@@ -1169,6 +1169,10 @@ DirectoryModel.prototype.search = function(query,
if (this.onSearchCompleted_)
this.removeEventListener('scan-completed', this.onSearchCompleted_);
// Current search will be cancelled.
if (this.onClearSearch_)
this.onClearSearch_();
this.onSearchCompleted_ = onSearchRescan;
this.onClearSearch_ = onClearSearch;
......
......@@ -927,6 +927,11 @@
"description": "Search query.",
"type": "string"
},
{
"name": "nextFeed",
"type": "string",
"description": "ID of the search feed that should be fetched next. Value passed here should be gotten from previous searchGData call. It can be empty for the initial search request."
},
{
"name": "callback",
"type": "function",
......@@ -938,7 +943,12 @@
"type": "object",
"isInstanceOf": "Entry",
"description": "Entry representing a search result."
}
}
},
{
"name": "nextFeed",
"type": "string",
"description": "ID of the feed that contains next chunk of the search result. Should be sent to the next searchGData request to perform incremental search."
}
]
}
......
......@@ -27,12 +27,17 @@ chromeHidden.registerCustomHook('fileBrowserPrivate', function(bindingsAPI) {
apiFunctions.setCustomCallback('searchGData',
function(name, request, response) {
if (response && !response.error && response) {
for (var i = 0; i < response.length; i++)
response[i] = GetExternalFileEntry(response[i]);
if (response && !response.error && response.entries) {
for (var i = 0; i < response.entries.length; i++)
response.entries[i] = GetExternalFileEntry(response.entries[i]);
}
// So |request.callback| doesn't break if response is not defined.
if (!response)
response = {};
if (request.callback)
request.callback(response);
request.callback(response.entries, response.nextFeed);
request.callback = null;
});
});
......@@ -77,10 +77,11 @@ chrome.test.runTests([
});
},
function driveSearch() {
chrome.fileBrowserPrivate.searchGData('foo',
function(entries) {
chrome.fileBrowserPrivate.searchGData('foo', '',
function(entries, nextFeed) {
chrome.test.assertTrue(!!entries);
chrome.test.assertEq(2, entries.length);
chrome.test.assertEq('', nextFeed);
chrome.test.assertEq('/drive/Folder',
entries[0].fullPath);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment