Associate the native- and java AutocompleteMatch objects.

This change enables the native AutocompleteMatch object to create and
retain a copy of its corresponding java counterpart.

The change essentially shifts code around, moving logic away from
AutocompleteController into AutocompleteMatch (both Java and Native).

The change optimizes slightly creation of the Java counterparts by
retaining an object if it's already created. This comes in handy for
every match that is backfilled on every keystroke (about 3 times every
time we call AutocompleteController::Start() to get new matches).

Java AutocompleteMatch objects are lazily constructed to reduce
penalty when native objects end up thrown away -- see:
* AutcompleteController#classify(),
* AutocompleteController#qualifyPartialUrlQuery()

Bug: 1138587
Change-Id: I8c5437cfb0203d5c7f5e1d79747eee3f15b7b0b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538499
Commit-Queue: Tomasz Wiszkowski <ender@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828488}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java

@@ -10,7 +10,6 @@ import android.util.SparseArray;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-import androidx.collection.ArraySet;
 
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
@@ -25,19 +24,14 @@ import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.components.omnibox.AutocompleteMatch;
-import org.chromium.components.omnibox.AutocompleteMatch.MatchClassification;
-import org.chromium.components.omnibox.AutocompleteMatch.NavsuggestTile;
 import org.chromium.components.omnibox.AutocompleteResult;
 import org.chromium.components.omnibox.AutocompleteResult.GroupDetails;
-import org.chromium.components.omnibox.SuggestionAnswer;
-import org.chromium.components.query_tiles.QueryTile;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.url.GURL;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 
 /**
  * Bridge to the native AutocompleteControllerAndroid.
@@ -349,53 +343,6 @@ public class AutocompleteController {
                 groupId, new GroupDetails(headerText, collapsedByDefault));
     }
 
-    @CalledByNative
-    private static AutocompleteMatch buildOmniboxSuggestion(int nativeType, int[] nativeSubtypes,
-            boolean isSearchType, int relevance, int transition, String contents,
-            int[] contentClassificationOffsets, int[] contentClassificationStyles,
-            String description, int[] descriptionClassificationOffsets,
-            int[] descriptionClassificationStyles, SuggestionAnswer answer, String fillIntoEdit,
-            GURL url, GURL imageUrl, String imageDominantColor, boolean isStarred,
-            boolean isDeletable, String postContentType, byte[] postData, int groupId,
-            List<QueryTile> tiles, byte[] clipboardImageData, boolean hasTabMatch,
-            List<NavsuggestTile> navsuggestTiles) {
-        assert contentClassificationOffsets.length == contentClassificationStyles.length;
-        List<MatchClassification> contentClassifications = new ArrayList<>();
-        for (int i = 0; i < contentClassificationOffsets.length; i++) {
-            contentClassifications.add(new MatchClassification(
-                    contentClassificationOffsets[i], contentClassificationStyles[i]));
-        }
-
-        assert descriptionClassificationOffsets.length == descriptionClassificationStyles.length;
-        List<MatchClassification> descriptionClassifications = new ArrayList<>();
-        for (int i = 0; i < descriptionClassificationOffsets.length; i++) {
-            descriptionClassifications.add(new MatchClassification(
-                    descriptionClassificationOffsets[i], descriptionClassificationStyles[i]));
-        }
-
-        Set<Integer> subtypes = new ArraySet(nativeSubtypes.length);
-        for (int i = 0; i < nativeSubtypes.length; i++) {
-            subtypes.add(nativeSubtypes[i]);
-        }
-
-        return new AutocompleteMatch(nativeType, subtypes, isSearchType, relevance, transition,
-                contents, contentClassifications, description, descriptionClassifications, answer,
-                fillIntoEdit, url, imageUrl, imageDominantColor, isStarred, isDeletable,
-                postContentType, postData, groupId, tiles, clipboardImageData, hasTabMatch,
-                navsuggestTiles);
-    }
-
-    @CalledByNative
-    private static List<NavsuggestTile> buildOmniboxNavsuggestTileList(int capacity) {
-        return new ArrayList<>(capacity);
-    }
-
-    @CalledByNative
-    private static void addOmniboxNavsuggestTile(
-            List<NavsuggestTile> tiles, String title, GURL url) {
-        tiles.add(new NavsuggestTile(title, url));
-    }
-
     /**
      * Verifies whether the given AutocompleteMatch object has the same hashCode as another
      * suggestion. This is used to validate that the native AutocompleteMatch object is in sync

chrome/browser/android/omnibox/autocomplete_controller_android.h

@@ -141,14 +141,6 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
   void NotifySuggestionsReceived(
       const AutocompleteResult& autocomplete_result);
 
-  base::android::ScopedJavaLocalRef<jobject> BuildOmniboxSuggestion(
-      JNIEnv* env, const AutocompleteMatch& match);
-
-  // Construct Java list of NavsuggestTile objects.
-  base::android::ScopedJavaLocalRef<jobject> BuildNavsuggestTilesList(
-      JNIEnv* env,
-      const std::vector<AutocompleteMatch::NavsuggestTile>& tiles);
-
   // Construct Java GroupDetails map from supplied HeadersMap and expanded
   // state.
   void PopulateOmniboxGroupsDetails(
@@ -157,16 +149,6 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
       const SearchSuggestionParser::HeadersMap& header_map,
       const std::set<int>& hidden_group_ids);
 
-  // A helper method for fetching the top synchronous autocomplete result.
-  // The |prevent_inline_autocomplete| flag is passed to the AutocompleteInput
-  // object, see documentation there for its description.
-  base::android::ScopedJavaLocalRef<jobject> GetTopSynchronousResult(
-      JNIEnv* env,
-      const base::android::JavaRef<jobject>& obj,
-      const base::android::JavaRef<jstring>& j_text,
-      bool prevent_inline_autocomplete,
-      bool focused_from_fakebox);
-
   bool IsValidMatch(JNIEnv* env, jint selected_index, jint hash_code);
 
   std::unique_ptr<AutocompleteController> autocomplete_controller_;
@@ -175,7 +157,7 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
   AutocompleteInput input_;
 
   // Whether we're currently inside a call to Start() that's called
-  // from GetTopSynchronousResult().
+  // from Classify().
   bool inside_synchronous_start_;
 
   JavaObjectWeakGlobalRef weak_java_autocomplete_controller_android_;

components/omnibox/browser/BUILD.gn

@@ -271,7 +271,11 @@ static_library("browser") {
   }
 
   if (is_android) {
-    deps += [ ":jni_headers" ]
+    sources += [ "autocomplete_match_android.cc" ]
+    deps += [
+      ":jni_headers",
+      "//url:gurl_android",
+    ]
   }
 }
 
@@ -394,6 +398,7 @@ if (is_android) {
 
   generate_jni("jni_headers") {
     sources = [
+      "android/java/src/org/chromium/components/omnibox/AutocompleteMatch.java",
       "android/java/src/org/chromium/components/omnibox/AutocompleteSchemeClassifier.java",
       "android/java/src/org/chromium/components/omnibox/OmniboxUrlEmphasizer.java",
       "android/java/src/org/chromium/components/omnibox/SuggestionAnswer.java",

components/omnibox/browser/android/java/src/org/chromium/components/omnibox/AutocompleteMatch.java

@@ -8,12 +8,15 @@ import android.text.TextUtils;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.collection.ArraySet;
 import androidx.core.util.ObjectsCompat;
 
+import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.browser.omnibox.MatchClassificationStyle;
 import org.chromium.components.query_tiles.QueryTile;
 import org.chromium.url.GURL;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -137,6 +140,48 @@ public class AutocompleteMatch {
         mNavsuggestTiles = navsuggestTiles;
     }
 
+    @CalledByNative
+    private static AutocompleteMatch build(int nativeType, int[] nativeSubtypes,
+            boolean isSearchType, int relevance, int transition, String contents,
+            int[] contentClassificationOffsets, int[] contentClassificationStyles,
+            String description, int[] descriptionClassificationOffsets,
+            int[] descriptionClassificationStyles, SuggestionAnswer answer, String fillIntoEdit,
+            GURL url, GURL imageUrl, String imageDominantColor, boolean isStarred,
+            boolean isDeletable, String postContentType, byte[] postData, int groupId,
+            List<QueryTile> tiles, byte[] clipboardImageData, boolean hasTabMatch,
+            String[] navsuggestTitles, GURL[] navsuggestUrls) {
+        assert contentClassificationOffsets.length == contentClassificationStyles.length;
+        List<MatchClassification> contentClassifications = new ArrayList<>();
+        for (int i = 0; i < contentClassificationOffsets.length; i++) {
+            contentClassifications.add(new MatchClassification(
+                    contentClassificationOffsets[i], contentClassificationStyles[i]));
+        }
+
+        assert descriptionClassificationOffsets.length == descriptionClassificationStyles.length;
+        List<MatchClassification> descriptionClassifications = new ArrayList<>();
+        for (int i = 0; i < descriptionClassificationOffsets.length; i++) {
+            descriptionClassifications.add(new MatchClassification(
+                    descriptionClassificationOffsets[i], descriptionClassificationStyles[i]));
+        }
+
+        assert navsuggestTitles.length == navsuggestUrls.length;
+        List<NavsuggestTile> navsuggestTiles = new ArrayList<>();
+        for (int i = 0; i < navsuggestTitles.length; i++) {
+            navsuggestTiles.add(new NavsuggestTile(navsuggestTitles[i], navsuggestUrls[i]));
+        }
+
+        Set<Integer> subtypes = new ArraySet(nativeSubtypes.length);
+        for (int i = 0; i < nativeSubtypes.length; i++) {
+            subtypes.add(nativeSubtypes[i]);
+        }
+
+        return new AutocompleteMatch(nativeType, subtypes, isSearchType, relevance, transition,
+                contents, contentClassifications, description, descriptionClassifications, answer,
+                fillIntoEdit, url, imageUrl, imageDominantColor, isStarred, isDeletable,
+                postContentType, postData, groupId, tiles, clipboardImageData, hasTabMatch,
+                navsuggestTiles);
+    }
+
     public int getType() {
         return mType;
     }

components/omnibox/browser/autocomplete_match.h

@@ -26,6 +26,10 @@
 #include "ui/gfx/range/range.h"
 #include "url/gurl.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/scoped_java_ref.h"
+#endif
+
 class AutocompleteProvider;
 class OmniboxPedal;
 class SuggestionAnswer;
@@ -36,6 +40,10 @@ namespace base {
 class Time;
 }  // namespace base
 
+namespace bookmarks {
+class BookmarkModel;
+}  // namespace bookmarks
+
 namespace gfx {
 struct VectorIcon;
 }  // namespace gfx
@@ -185,6 +193,14 @@ struct AutocompleteMatch {
 
   AutocompleteMatch& operator=(const AutocompleteMatch& match);
 
+#if defined(OS_ANDROID)
+  // Returns a corresponding Java object, creating it if necessary.
+  // NOTE: Android specific methods are defined in autocomplete_match_android.cc
+  base::android::ScopedJavaLocalRef<jobject> GetOrCreateJavaObject(
+      JNIEnv* env,
+      bookmarks::BookmarkModel* model) const;
+#endif
+
 #if (!defined(OS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !defined(OS_IOS)
   // Gets the vector icon identifier for the icon to be shown for this match. If
   // |is_bookmark| is true, returns a bookmark icon rather than what the type
@@ -700,6 +716,20 @@ struct AutocompleteMatch {
       const base::string16& text,
       const ACMatchClassifications& classifications,
       const std::string& provider_name = "");
+
+ private:
+#if defined(OS_ANDROID)
+  // Corresponding Java object.
+  // This element should not be copied with the rest of the AutocompleteMatch
+  // object to ensure consistent 1:1 relationship between the objects.
+  // This object should never be accessed directly. To acquire a reference to
+  // java object, call the GetOrCreateJavaObject().
+  // Note that this object is lazily constructed to avoid creating Java matches
+  // for throw away AutocompleteMatch objects, eg. during Classify() or
+  // QualifyPartialUrlQuery() calls.
+  // See AutocompleteControllerAndroid for more details.
+  mutable base::android::ScopedJavaGlobalRef<jobject> java_match_;
+#endif
 };
 
 typedef AutocompleteMatch::ACMatchClassification ACMatchClassification;

components/omnibox/browser/autocomplete_match_android.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/omnibox/browser/autocomplete_match.h"
+
+#include <vector>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/omnibox/browser/jni_headers/AutocompleteMatch_jni.h"
+#include "components/omnibox/browser/search_suggestion_parser.h"
+#include "components/query_tiles/android/tile_conversion_bridge.h"
+#include "url/android/gurl_android.h"
+
+using base::android::ConvertUTF16ToJavaString;
+using base::android::ConvertUTF8ToJavaString;
+using base::android::ScopedJavaLocalRef;
+using base::android::ToJavaArrayOfStrings;
+using base::android::ToJavaByteArray;
+using base::android::ToJavaIntArray;
+
+ScopedJavaLocalRef<jobject> AutocompleteMatch::GetOrCreateJavaObject(
+    JNIEnv* env,
+    bookmarks::BookmarkModel* bookmark_model) const {
+  // Short circuit if we already built the match.
+  if (java_match_)
+    return ScopedJavaLocalRef<jobject>(java_match_);
+
+  std::vector<int> contents_class_offsets;
+  std::vector<int> contents_class_styles;
+  for (auto contents_class : contents_class) {
+    contents_class_offsets.push_back(contents_class.offset);
+    contents_class_styles.push_back(contents_class.style);
+  }
+
+  std::vector<int> description_class_offsets;
+  std::vector<int> description_class_styles;
+  for (auto description_class : description_class) {
+    description_class_offsets.push_back(description_class.offset);
+    description_class_styles.push_back(description_class.style);
+  }
+
+  base::android::ScopedJavaLocalRef<jobject> janswer;
+  if (answer)
+    janswer = answer->CreateJavaObject();
+  ScopedJavaLocalRef<jstring> j_image_dominant_color;
+  ScopedJavaLocalRef<jstring> j_post_content_type;
+  ScopedJavaLocalRef<jbyteArray> j_post_content;
+  std::string clipboard_image_data;
+
+  if (!image_dominant_color.empty()) {
+    j_image_dominant_color = ConvertUTF8ToJavaString(env, image_dominant_color);
+  }
+
+  if (post_content && !post_content->first.empty() &&
+      !post_content->second.empty()) {
+    j_post_content_type = ConvertUTF8ToJavaString(env, post_content->first);
+    j_post_content = ToJavaByteArray(env, post_content->second);
+  }
+
+  if (search_terms_args.get()) {
+    clipboard_image_data = search_terms_args->image_thumbnail_content;
+  }
+
+  ScopedJavaLocalRef<jobject> j_query_tiles =
+      query_tiles::TileConversionBridge::CreateJavaTiles(env, query_tiles);
+
+  std::vector<base::string16> navsuggest_titles(navsuggest_tiles.size());
+  std::vector<base::android::ScopedJavaLocalRef<jobject>> navsuggest_urls(
+      navsuggest_tiles.size());
+  for (const auto& tile : navsuggest_tiles) {
+    navsuggest_titles.push_back(tile.title);
+    navsuggest_urls.push_back(url::GURLAndroid::FromNativeGURL(env, tile.url));
+  }
+
+  std::vector<int> temp_subtypes(subtypes.begin(), subtypes.end());
+
+  java_match_ = Java_AutocompleteMatch_build(
+      env, type, ToJavaIntArray(env, temp_subtypes),
+      AutocompleteMatch::IsSearchType(type), relevance, transition,
+      ConvertUTF16ToJavaString(env, contents),
+      ToJavaIntArray(env, contents_class_offsets),
+      ToJavaIntArray(env, contents_class_styles),
+      ConvertUTF16ToJavaString(env, description),
+      ToJavaIntArray(env, description_class_offsets),
+      ToJavaIntArray(env, description_class_styles), janswer,
+      ConvertUTF16ToJavaString(env, fill_into_edit),
+      url::GURLAndroid::FromNativeGURL(env, destination_url),
+      url::GURLAndroid::FromNativeGURL(env, destination_url),
+      j_image_dominant_color,
+      bookmark_model && bookmark_model->IsBookmarked(destination_url),
+      SupportsDeletion(), j_post_content_type, j_post_content,
+      suggestion_group_id.value_or(
+          SearchSuggestionParser::kNoSuggestionGroupId),
+      j_query_tiles, ToJavaByteArray(env, clipboard_image_data), has_tab_match,
+      ToJavaArrayOfStrings(env, navsuggest_titles),
+      url::GURLAndroid::ToJavaArrayOfGURLs(env, navsuggest_urls));
+
+  return ScopedJavaLocalRef<jobject>(java_match_);
+}