Revert "Revert of Add Search Service in Enhanced Bookmark Bridge (patchset #6 id:100001 of https://codereview.chromium.org/637323005/)"

This reverts commit 1491f336d849dcf168cc8832e91a7c8213dd2a7f.

BUG=415774
TBR=yfriedman

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

Cr-Commit-Position: refs/heads/master@{#302165}

4 files changed, 57 insertions, 31 deletions

diff --git a/components/enhanced_bookmarks/bookmark_server_search_service.cc b/components/enhanced_bookmarks/bookmark_server_search_service.cc
index 0cf4690..e1ab78e 100644
--- a/components/enhanced_bookmarks/bookmark_server_search_service.cc
+++ b/components/enhanced_bookmarks/bookmark_server_search_service.cc
@@ -12,6 +12,7 @@
 
 namespace {
 const char kSearchUrl[] = "https://www.google.com/stars/search";
+const int kSearchCacheMaxSize = 50;
 }  // namespace
 
 namespace enhanced_bookmarks {
@@ -24,7 +25,8 @@ BookmarkServerSearchService::BookmarkServerSearchService(
     : BookmarkServerService(request_context_getter,
                             token_service,
                             signin_manager,
-                            enhanced_bookmark_model) {
+                            enhanced_bookmark_model),
+      cache_(kSearchCacheMaxSize) {
 }
 
 BookmarkServerSearchService::~BookmarkServerSearchService() {
@@ -32,26 +34,34 @@ BookmarkServerSearchService::~BookmarkServerSearchService() {
 
 void BookmarkServerSearchService::Search(const std::string& query) {
   DCHECK(query.length());
+  if (current_query_ == query)
+    return;
+
+  // If result is already stored in cache, immediately notify observers.
+  if (cache_.Get(current_query_) != cache_.end()) {
+    Cancel();
+    Notify();
+    return;
+  }
   current_query_ = query;
   TriggerTokenRequest(true);
 }
 
-std::vector<const BookmarkNode*> BookmarkServerSearchService::ResultForQuery(
-    const std::string& query) {
+scoped_ptr<std::vector<const BookmarkNode*>>
+BookmarkServerSearchService::ResultForQuery(const std::string& query) {
   DCHECK(query.length());
-  std::vector<const BookmarkNode*> result;
+  scoped_ptr<std::vector<const BookmarkNode*>> result;
 
-  std::map<std::string, std::vector<std::string> >::iterator it =
-      searches_.find(query);
-  if (it == searches_.end())
+  const auto& it = cache_.Get(query);
+  if (it == cache_.end())
     return result;
 
-  for (std::vector<std::string>::iterator clip_it = it->second.begin();
-       clip_it != it->second.end();
-       ++clip_it) {
-    const BookmarkNode* node = BookmarkForRemoteId(*clip_it);
+  result.reset(new std::vector<const BookmarkNode*>());
+
+  for (const std::string& clip_id : it->second) {
+    const BookmarkNode* node = BookmarkForRemoteId(clip_id);
     if (node)
-      result.push_back(node);
+      result->push_back(node);
   }
   return result;
 }
@@ -80,38 +90,36 @@ bool BookmarkServerSearchService::ProcessResponse(const std::string& response,
     return false;  // Not formatted properly.
 
   std::vector<std::string> clip_ids;
-  for (google::protobuf::RepeatedPtrField<
-           image::collections::CorpusSearchResult_ClipResult>::const_iterator
-           it = response_proto.results().begin();
-       it != response_proto.results().end();
-       ++it) {
-    const std::string& clip_id = it->clip_id();
+  for (const image::collections::CorpusSearchResult_ClipResult& clip_result :
+       response_proto.results()) {
+    const std::string& clip_id = clip_result.clip_id();
     if (!clip_id.length())
       continue;
     clip_ids.push_back(clip_id);
   }
-  searches_[current_query_] = clip_ids;
+  cache_.Put(current_query_, clip_ids);
   current_query_.clear();
   return true;
 }
 
 void BookmarkServerSearchService::CleanAfterFailure() {
-  searches_.clear();
+  cache_.Clear();
+  current_query_.clear();
 }
 
 void BookmarkServerSearchService::EnhancedBookmarkAdded(
     const BookmarkNode* node) {
-  searches_.clear();
+  cache_.Clear();
 }
 
 void BookmarkServerSearchService::EnhancedBookmarkAllUserNodesRemoved() {
-  searches_.clear();
+  cache_.Clear();
 }
 
 void BookmarkServerSearchService::EnhancedBookmarkRemoteIdChanged(
     const BookmarkNode* node,
     const std::string& old_remote_id,
     const std::string& remote_id) {
-  searches_.clear();
+  cache_.Clear();
 }
 }  // namespace enhanced_bookmarks
diff --git a/components/enhanced_bookmarks/bookmark_server_search_service.h b/components/enhanced_bookmarks/bookmark_server_search_service.h
index 9771318..df59f94 100644
--- a/components/enhanced_bookmarks/bookmark_server_search_service.h
+++ b/components/enhanced_bookmarks/bookmark_server_search_service.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/mru_cache.h"
 #include "components/enhanced_bookmarks/bookmark_server_service.h"
 #include "net/url_request/url_fetcher.h"
 
@@ -27,14 +28,20 @@ class BookmarkServerSearchService : public BookmarkServerService {
   ~BookmarkServerSearchService() override;
 
   // Triggers a search. The query must not be empty. A call to this method
-  // cancels any previous searches. OnChange() is garanteed to be called once
-  // per query.
+  // cancels any previous searches. If there have been multiple queries in
+  // between, onChange will only be called for the last query.
+  // Note this method will be synchronous if query hits the cache.
   void Search(const std::string& query);
 
-  // Returns the search results. The results are only available after the
-  // OnChange() observer methods has been sent. This method will return an empty
-  // result otherwise. query should be a string passed to Search() previously.
-  std::vector<const BookmarkNode*> ResultForQuery(const std::string& query);
+  // Returns search results for a query. Results for a query are only available
+  // after Search() is called and after then OnChange() observer methods has
+  // been sent.This method might return an empty vector, meaning there are no
+  // bookmarks matching the given query. Returning null means we are still
+  // loading and no results have come to the client. Previously cancelled
+  // queries will not trigger onChange(), and this method will also return null
+  // for queries that have never been passed to Search() before.
+  scoped_ptr<std::vector<const BookmarkNode*>> ResultForQuery(
+      const std::string& query);
 
  protected:
   scoped_ptr<net::URLFetcher> CreateFetcher() override;
@@ -54,8 +61,11 @@ class BookmarkServerSearchService : public BookmarkServerService {
                                        const std::string& remote_id) override;
 
  private:
-  // The search data, a map from query to a vector of stars.id.
-  std::map<std::string, std::vector<std::string> > searches_;
+  // Cache for previous search result, a map from a query string to vector of
+  // star_ids.
+  base::MRUCache<std::string, std::vector<std::string>> cache_;
+  // The query currently on the fly, and is cleared as soon as the result is
+  // available.
   std::string current_query_;
   DISALLOW_COPY_AND_ASSIGN(BookmarkServerSearchService);
 };
diff --git a/components/enhanced_bookmarks/bookmark_server_service.cc b/components/enhanced_bookmarks/bookmark_server_service.cc
index 1e6f4b0..73cd8f8 100644
--- a/components/enhanced_bookmarks/bookmark_server_service.cc
+++ b/components/enhanced_bookmarks/bookmark_server_service.cc
@@ -56,6 +56,11 @@ const std::string BookmarkServerService::RemoteIDForBookmark(
   return model_->GetRemoteId(bookmark);
 }
 
+void BookmarkServerService::Cancel() {
+  url_fetcher_.reset();
+  token_request_.reset();
+}
+
 void BookmarkServerService::Notify() {
   FOR_EACH_OBSERVER(BookmarkServerServiceObserver, observers_, OnChange(this));
 }
diff --git a/components/enhanced_bookmarks/bookmark_server_service.h b/components/enhanced_bookmarks/bookmark_server_service.h
index 41416fd..9f2fd52 100644
--- a/components/enhanced_bookmarks/bookmark_server_service.h
+++ b/components/enhanced_bookmarks/bookmark_server_service.h
@@ -57,6 +57,9 @@ class BookmarkServerService : protected net::URLFetcherDelegate,
       const std::string& remote_id) const;
   const std::string RemoteIDForBookmark(const BookmarkNode* bookmark) const;
 
+  // Cancels the ongoing request, if any.
+  void Cancel();
+
   // Notifies the observers that something changed.
   void Notify();