Add Search Service in Enhanced Bookmark Bridge

A Search client is created during initialization of
EnhancedBookmarkBridge. And when the bridge destructs, search service is
also destroyed.

BUG=415774

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

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

chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java

@@ -20,20 +20,32 @@
 @JNINamespace("enhanced_bookmarks::android")
 public final class EnhancedBookmarksBridge {
     private long mNativeEnhancedBookmarksBridge;
-    private final ObserverList<FiltersObserver> mObservers =
+    private final ObserverList<FiltersObserver> mFilterObservers =
             new ObserverList<FiltersObserver>();
+    private final ObserverList<SearchServiceObserver> mSearchObservers =
+            new ObserverList<SearchServiceObserver>();
 
     /**
      * Interface to provide consumers notifications to changes in clusters
      */
     public interface FiltersObserver {
         /**
-         * Invoked when client detects that filters have been
-         * added / removed from the server.
+         * Invoked when client detects that filters have been added/removed from the server.
          */
         void onFiltersChanged();
     }
 
+    /**
+     * Interface to provide consumers notifications to changes in search service results.
+     */
+    public interface SearchServiceObserver {
+        /**
+         * Invoked when client detects that search results have been updated. This callback is
+         * guaranteed to be called only once and only for the most recent query.
+         */
+        void onSearchResultsReturned();
+    }
+
     public EnhancedBookmarksBridge(Profile profile) {
         mNativeEnhancedBookmarksBridge = nativeInit(profile);
     }
@@ -92,15 +104,15 @@ public void setBookmarkDescription(BookmarkId id, String description) {
      * @param observer Observer to add
      */
     public void addFiltersObserver(FiltersObserver observer) {
-        mObservers.addObserver(observer);
+        mFilterObservers.addObserver(observer);
     }
 
     /**
      * Unregisters a FiltersObserver from listening to filter change notifications.
      * @param observer Observer to remove
      */
     public void removeFiltersObserver(FiltersObserver observer) {
-        mObservers.removeObserver(observer);
+        mFilterObservers.removeObserver(observer);
     }
 
     /**
@@ -114,6 +126,42 @@ public List<BookmarkId> getBookmarksForFilter(String filter) {
         return list;
     }
 
+    /**
+     * Sends request to search server for querying related bookmarks.
+     * @param query Keyword used to find related bookmarks.
+     */
+    public void sendSearchRequest(String query) {
+        nativeSendSearchRequest(mNativeEnhancedBookmarksBridge, query);
+    }
+
+    /**
+     * Get list of bookmarks as result of a search request that was sent before in
+     * {@link EnhancedBookmarksBridge#sendSearchRequest(String)}. Normally this function should be
+     * called after {@link SearchServiceObserver#onSearchResultsReturned()}
+     * @param query Keyword used to find related bookmarks.
+     * @return List of BookmarkIds that are related to query. It will be null if the request is
+     *         still on the fly, or empty list if there are no results for the query.
+     */
+    public List<BookmarkId> getSearchResultsForQuery(String query) {
+        return nativeGetSearchResults(mNativeEnhancedBookmarksBridge, query);
+    }
+
+    /**
+     * Registers a SearchObserver that listens to search request updates.
+     * @param observer Observer to add
+     */
+    public void addSearchObserver(SearchServiceObserver observer) {
+        mSearchObservers.addObserver(observer);
+    }
+
+    /**
+     * Unregisters a SearchObserver that listens to search request updates.
+     * @param observer Observer to remove
+     */
+    public void removeSearchObserver(SearchServiceObserver observer) {
+        mSearchObservers.removeObserver(observer);
+    }
+
     /**
      * @return Current set of known auto-filters for bookmarks.
      */
@@ -125,11 +173,23 @@ public List<String> getFilters() {
 
     @CalledByNative
     private void onFiltersChanged() {
-        for (FiltersObserver observer : mObservers) {
+        for (FiltersObserver observer : mFilterObservers) {
             observer.onFiltersChanged();
         }
     }
 
+    @CalledByNative
+    private void onSearchResultReturned() {
+        for (SearchServiceObserver observer : mSearchObservers) {
+            observer.onSearchResultsReturned();
+        }
+    }
+
+    @CalledByNative
+    private static List<BookmarkId> createBookmarkIdList() {
+        return new ArrayList<BookmarkId>();
+    }
+
     @CalledByNative
     private static void addToBookmarkIdList(List<BookmarkId> bookmarkIdList, long id, int type) {
         bookmarkIdList.add(new BookmarkId(id, type));
@@ -143,12 +203,14 @@ private native void nativeSetBookmarkDescription(long nativeEnhancedBookmarksBri
             int type, String description);
     private native void nativeGetBookmarksForFilter(long nativeEnhancedBookmarksBridge,
             String filter, List<BookmarkId> list);
+    private native List<BookmarkId> nativeGetSearchResults(long nativeEnhancedBookmarksBridge,
+            String query);
     private native String[] nativeGetFilters(long nativeEnhancedBookmarksBridge);
     private native BookmarkId nativeAddFolder(long nativeEnhancedBookmarksBridge, BookmarkId parent,
             int index, String title);
     private native void nativeMoveBookmark(long nativeEnhancedBookmarksBridge,
             BookmarkId bookmarkId, BookmarkId newParentId, int index);
     private native BookmarkId nativeAddBookmark(long nativeEnhancedBookmarksBridge,
             BookmarkId parent, int index, String title, String url);
-
+    private native void nativeSendSearchRequest(long nativeEnhancedBookmarksBridge, String query);
 }

chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.cc

@@ -12,13 +12,16 @@
 #include "chrome/browser/enhanced_bookmarks/chrome_bookmark_server_cluster_service_factory.h"
 #include "chrome/browser/enhanced_bookmarks/enhanced_bookmark_model_factory.h"
 #include "chrome/browser/profiles/profile_android.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/bookmarks/common/android/bookmark_id.h"
 #include "components/bookmarks/common/android/bookmark_type.h"
 #include "components/enhanced_bookmarks/enhanced_bookmark_model.h"
+#include "components/signin/core/browser/signin_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "jni/EnhancedBookmarksBridge_jni.h"
 
@@ -42,10 +45,17 @@ EnhancedBookmarksBridge::EnhancedBookmarksBridge(JNIEnv* env,
   cluster_service_ =
       ChromeBookmarkServerClusterServiceFactory::GetForBrowserContext(profile_);
   cluster_service_->AddObserver(this);
+  search_service_.reset(new BookmarkServerSearchService(
+      profile_->GetRequestContext(),
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_),
+      SigninManagerFactory::GetForProfile(profile_),
+      EnhancedBookmarkModelFactory::GetForBrowserContext(profile_)));
+  search_service_->AddObserver(this);
 }
 
 EnhancedBookmarksBridge::~EnhancedBookmarksBridge() {
   cluster_service_->RemoveObserver(this);
+  search_service_->RemoveObserver(this);
 }
 
 void EnhancedBookmarksBridge::Destroy(JNIEnv*, jobject) {
@@ -99,8 +109,7 @@ ScopedJavaLocalRef<jobjectArray> EnhancedBookmarksBridge::GetFilters(
     JNIEnv* env,
     jobject obj) {
   DCHECK(enhanced_bookmark_model_->loaded());
-  const std::vector<std::string> filters =
-      cluster_service_->GetClusters();
+  const std::vector<std::string> filters = cluster_service_->GetClusters();
   return base::android::ToJavaArrayOfStrings(env, filters);
 }
 
@@ -176,6 +185,35 @@ ScopedJavaLocalRef<jobject> EnhancedBookmarksBridge::AddBookmark(
   return new_java_obj;
 }
 
+void EnhancedBookmarksBridge::SendSearchRequest(JNIEnv* env,
+                                                jobject obj,
+                                                jstring j_query) {
+  search_service_->Search(base::android::ConvertJavaStringToUTF8(env, j_query));
+}
+
+ScopedJavaLocalRef<jobject> EnhancedBookmarksBridge::GetSearchResults(
+    JNIEnv* env,
+    jobject obj,
+    jstring j_query) {
+  DCHECK(enhanced_bookmark_model_->loaded());
+
+  ScopedJavaLocalRef<jobject> j_list =
+          Java_EnhancedBookmarksBridge_createBookmarkIdList(env);
+  scoped_ptr<std::vector<const BookmarkNode*>> results =
+      search_service_->ResultForQuery(
+          base::android::ConvertJavaStringToUTF8(env, j_query));
+
+  // If result is null, return a null java reference.
+  if (!results.get())
+    return ScopedJavaLocalRef<jobject>();
+
+  for (const BookmarkNode* node : *results) {
+    Java_EnhancedBookmarksBridge_addToBookmarkIdList(env, j_list.obj(),
+                                                     node->id(), node->type());
+  }
+  return j_list;
+}
+
 void EnhancedBookmarksBridge::OnChange(BookmarkServerService* service) {
   DCHECK(enhanced_bookmark_model_->loaded());
   JNIEnv* env = AttachCurrentThread();
@@ -184,7 +222,11 @@ void EnhancedBookmarksBridge::OnChange(BookmarkServerService* service) {
   if (obj.is_null())
     return;
 
-  Java_EnhancedBookmarksBridge_onFiltersChanged(env, obj.obj());
+  if (service == cluster_service_) {
+    Java_EnhancedBookmarksBridge_onFiltersChanged(env, obj.obj());
+  } else if (service == search_service_.get()){
+    Java_EnhancedBookmarksBridge_onSearchResultReturned(env, obj.obj());
+  }
 }
 
 bool EnhancedBookmarksBridge::IsEditable(const BookmarkNode* node) const {

chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.h

@@ -9,6 +9,7 @@
 #include "base/android/jni_weak_ref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "components/enhanced_bookmarks/bookmark_server_search_service.h"
 #include "components/enhanced_bookmarks/bookmark_server_service.h"
 
 namespace enhanced_bookmarks {
@@ -28,12 +29,12 @@ class EnhancedBookmarksBridge : public BookmarkServerServiceObserver {
       jobject obj,
       jlong id,
       jint type);
-
   void SetBookmarkDescription(JNIEnv* env,
                               jobject obj,
                               jlong id,
                               jint type,
                               jstring description);
+
   void GetBookmarksForFilter(JNIEnv* env,
                              jobject obj,
                              jstring filter,
@@ -60,16 +61,23 @@ class EnhancedBookmarksBridge : public BookmarkServerServiceObserver {
       jint index,
       jstring j_title,
       jstring j_url);
+  void SendSearchRequest(JNIEnv* env, jobject obj, jstring j_query);
+
+  base::android::ScopedJavaLocalRef<jobject> GetSearchResults(JNIEnv* env,
+                                                              jobject obj,
+                                                              jstring j_query);
+
   // BookmarkServerServiceObserver
-  // Called on changes to cluster data
+  // Called on changes to cluster data or search results are returned.
   virtual void OnChange(BookmarkServerService* service) override;
 
  private:
   bool IsEditable(const BookmarkNode* node) const;
 
   JavaObjectWeakGlobalRef weak_java_ref_;
-  EnhancedBookmarkModel* enhanced_bookmark_model_;
+  EnhancedBookmarkModel* enhanced_bookmark_model_; // weak
   BookmarkServerClusterService* cluster_service_;  // weak
+  scoped_ptr<BookmarkServerSearchService> search_service_;
   Profile* profile_;                       // weak
   DISALLOW_COPY_AND_ASSIGN(EnhancedBookmarksBridge);
 };

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,34 +25,43 @@ BookmarkServerSearchService::BookmarkServerSearchService(
     : BookmarkServerService(request_context_getter,
                             token_service,
                             signin_manager,
-                            enhanced_bookmark_model) {
+                            enhanced_bookmark_model),
+      cache_(kSearchCacheMaxSize) {
 }
 
 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