Pass native AutocompleteMatch directly to OnSuggestionSelected
This change paves the way for introduction of horizontal rendering
groups. This change prepares us to report a (logical) suggestion row
that captures the autocomplete match (which could belong to a
horizontally rendered group, aka carousel).
Bug: 1474087
Change-Id: I090858be2de4fde1b48cb2c15067cfc747ee4f15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4808885
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Commit-Queue: Tomasz Wiszkowski <ender@google.com>
Cr-Commit-Position: refs/heads/main@{#1188051}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/mostvisited/MostVisitedTilesTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/mostvisited/MostVisitedTilesTest.java
index 6014ff7..9153675a 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/mostvisited/MostVisitedTilesTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/mostvisited/MostVisitedTilesTest.java
@@ -83,6 +83,7 @@
private static final String START_PAGE_LOCATION = "/echo/start.html";
private static final String SEARCH_QUERY = "related search query";
private static final int MV_TILE_CAROUSEL_MATCH_POSITION = 1;
+ private static final long MV_TILE_NATIVE_HANDLE = 0xfce2;
public final @Rule ChromeTabbedActivityTestRule mActivityTestRule =
new ChromeTabbedActivityTestRule();
@@ -163,7 +164,9 @@
builder.setType(OmniboxSuggestionType.TILE_NAVSUGGEST);
builder.setSuggestTiles(Arrays.asList(new SuggestTile[] {mTile1, mTile2, mTile3}));
builder.setDeletable(true);
- autocompleteResult.getSuggestionsList().add(builder.build());
+ var match = builder.build();
+ match.updateNativeObjectRef(MV_TILE_NATIVE_HANDLE);
+ autocompleteResult.getSuggestionsList().add(match);
builder.reset();
// Third suggestion - search query with a header.
@@ -312,7 +315,7 @@
CriteriaHelper.pollUiThread(() -> { return manager.getCurrentDialogForTest() == null; });
verify(mAutocompleteControllerJniMock, times(1))
- .deleteMatchElement(anyLong(), eq(0L), eq(tileToDelete));
+ .deleteMatchElement(anyLong(), eq(MV_TILE_NATIVE_HANDLE), eq(tileToDelete));
}
@Test
diff --git a/chrome/browser/android/omnibox/autocomplete_controller_android.cc b/chrome/browser/android/omnibox/autocomplete_controller_android.cc
index 67c6c20..b52ec5f4 100644
--- a/chrome/browser/android/omnibox/autocomplete_controller_android.cc
+++ b/chrome/browser/android/omnibox/autocomplete_controller_android.cc
@@ -297,7 +297,8 @@
void AutocompleteControllerAndroid::OnSuggestionSelected(
JNIEnv* env,
- jint match_index,
+ uintptr_t match_ptr,
+ int suggestion_line,
const jint j_window_open_disposition,
const JavaParamRef<jstring>& j_current_url,
jint j_page_classification,
@@ -310,7 +311,7 @@
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(j_web_contents);
- const auto& match = autocomplete_controller_->result().match_at(match_index);
+ const auto& match = *reinterpret_cast<AutocompleteMatch*>(match_ptr);
SuggestionAnswer::LogAnswerUsed(match.answer);
TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile_);
@@ -345,7 +346,7 @@
: input_.text(),
false, /* don't know */
input_.type(), false, /* not keyword mode */
- OmniboxEventProto::INVALID, true, OmniboxPopupSelection(match_index),
+ OmniboxEventProto::INVALID, true, OmniboxPopupSelection(suggestion_line),
static_cast<WindowOpenDisposition>(j_window_open_disposition), false,
sessions::SessionTabHelper::IdForTab(web_contents),
OmniboxEventProto::PageClassification(j_page_classification),
diff --git a/chrome/browser/android/omnibox/autocomplete_controller_android.h b/chrome/browser/android/omnibox/autocomplete_controller_android.h
index 77d4bce6..fa0bdaa 100644
--- a/chrome/browser/android/omnibox/autocomplete_controller_android.h
+++ b/chrome/browser/android/omnibox/autocomplete_controller_android.h
@@ -59,7 +59,8 @@
void OnSuggestionSelected(
JNIEnv* env,
- jint match_index,
+ uintptr_t match_ptr,
+ int suggestion_line,
const jint j_window_open_disposition,
const base::android::JavaParamRef<jstring>& j_current_url,
jint j_page_classification,
diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java
index 4b236f9..85e9d0c 100644
--- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java
+++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java
@@ -198,6 +198,8 @@
AutocompleteResult.NO_SUGGESTION_INDEX, VerificationPoint.DELETE_MATCH)) {
return;
}
+ // Skip suggestions from cache.
+ if (match.getNativeObjectRef() == 0L) return;
AutocompleteControllerJni.get().deleteMatchElement(
mNativeController, match.getNativeObjectRef(), elementIndex);
}
@@ -213,6 +215,8 @@
AutocompleteResult.NO_SUGGESTION_INDEX, VerificationPoint.DELETE_MATCH)) {
return;
}
+ // Skip suggestions from cache.
+ if (match.getNativeObjectRef() == 0L) return;
AutocompleteControllerJni.get().deleteMatch(mNativeController, match.getNativeObjectRef());
}
@@ -239,27 +243,26 @@
* Called whenever a navigation happens from the omnibox to record metrics about the user's
* interaction with the omnibox.
*
- * @param matchIndex The index of the suggestion that was selected.
- * @param disposition The window open disposition.
- * @param type The type of the selected suggestion.
- * @param currentPageUrl The URL of the current page.
- * @param pageClassification The page classification of the current tab.
- * @param elapsedTimeSinceModified The number of ms that passed between the user first
- * modifying text in the omnibox and selecting a suggestion.
- * @param completedLength The length of the default match's inline autocompletion if any.
- * @param webContents The web contents for the tab where the selected suggestion will be shown.
+ * @param match AutocompleteMatch that was selected by the user
+ * @param suggestionLine the index of the line the match is presented on
+ * @param disposition the window open disposition
+ * @param currentPageUrl the URL of the current page
+ * @param pageClassification the page classification of the current tab
+ * @param elapsedTimeSinceModified the number of ms that passed between the user first
+ * modifying text in the omnibox and selecting a suggestion
+ * @param completedLength the length of the default match's inline autocompletion if any
+ * @param webContents the web contents for the tab where the selected suggestion will be shown
*/
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
- public void onSuggestionSelected(int matchIndex, int disposition, int type,
+ public void onSuggestionSelected(AutocompleteMatch match, int suggestionLine, int disposition,
@NonNull GURL currentPageUrl, int pageClassification, long elapsedTimeSinceModified,
int completedLength, @Nullable WebContents webContents) {
if (mNativeController == 0) return;
- if (!mAutocompleteResult.verifyCoherency(matchIndex, VerificationPoint.SELECT_MATCH)) {
- return;
- }
- AutocompleteControllerJni.get().onSuggestionSelected(mNativeController, matchIndex,
- disposition, currentPageUrl.getSpec(), pageClassification, elapsedTimeSinceModified,
- completedLength, webContents);
+ // Skip suggestions from cache.
+ if (match.getNativeObjectRef() == 0L) return;
+ AutocompleteControllerJni.get().onSuggestionSelected(mNativeController,
+ match.getNativeObjectRef(), suggestionLine, disposition, currentPageUrl.getSpec(),
+ pageClassification, elapsedTimeSinceModified, completedLength, webContents);
}
/**
@@ -375,9 +378,10 @@
long nativeAutocompleteControllerAndroid, String text, boolean focusedFromFakebox);
void stop(long nativeAutocompleteControllerAndroid, boolean clearResults);
void resetSession(long nativeAutocompleteControllerAndroid);
- void onSuggestionSelected(long nativeAutocompleteControllerAndroid, int matchIndex,
- int disposition, String currentPageUrl, int pageClassification,
- long elapsedTimeSinceModified, int completedLength, WebContents webContents);
+ void onSuggestionSelected(long nativeAutocompleteControllerAndroid,
+ long nativeAutocompleteMatch, int matchIndex, int disposition,
+ String currentPageUrl, int pageClassification, long elapsedTimeSinceModified,
+ int completedLength, WebContents webContents);
boolean onSuggestionTouchDown(
long nativeAutocompleteControllerAndroid, int matchIndex, WebContents webContents);
void onOmniboxFocused(long nativeAutocompleteControllerAndroid, String omniboxText,
diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java
index 26d1f4c..b1a1494 100644
--- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java
+++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java
@@ -504,7 +504,7 @@
@Override
public void onSwitchToTab(AutocompleteMatch suggestion, int matchIndex) {
if (maybeSwitchToTab(matchIndex)) {
- recordMetrics(matchIndex, WindowOpenDisposition.SWITCH_TO_TAB, suggestion);
+ recordMetrics(suggestion, matchIndex, WindowOpenDisposition.SWITCH_TO_TAB);
} else {
onSuggestionClicked(suggestion, matchIndex, suggestion.getUrl());
}
@@ -871,7 +871,7 @@
int transition = suggestion.getTransition();
int type = suggestion.getType();
- recordMetrics(matchIndex, WindowOpenDisposition.CURRENT_TAB, suggestion);
+ recordMetrics(suggestion, matchIndex, WindowOpenDisposition.CURRENT_TAB);
if (((transition & PageTransition.CORE_MASK) == PageTransition.TYPED)
&& url.equals(mDataProvider.getCurrentGurl())) {
// When the user hit enter on the existing permanent URL, treat it like a
@@ -1047,13 +1047,13 @@
* Called whenever a navigation happens from the omnibox to record metrics about the user's
* interaction with the omnibox.
*
- * @param matchIndex The index of the suggestion that was selected.
- * @param disposition The window open disposition.
- * @param suggestion The suggestion selected.
+ * @param match the selected AutocompleteMatch
+ * @param suggestionLine the index of the suggestion line that holds selected match
+ * @param disposition the window open disposition
*/
- private void recordMetrics(int matchIndex, int disposition, AutocompleteMatch suggestion) {
+ private void recordMetrics(AutocompleteMatch match, int suggestionLine, int disposition) {
OmniboxMetrics.recordUsedSuggestionFromCache(mAutocompleteResult.isFromCachedResult());
- OmniboxMetrics.recordTouchDownPrefetchResult(suggestion, mLastPrefetchStartedSuggestion);
+ OmniboxMetrics.recordTouchDownPrefetchResult(match, mLastPrefetchStartedSuggestion);
// Do not attempt to record other metrics for cached suggestions if the source of the list
// is local cache. These suggestions do not have corresponding native objects and will fail
@@ -1069,9 +1069,8 @@
WebContents webContents =
mDataProvider.hasTab() ? mDataProvider.getTab().getWebContents() : null;
- mAutocomplete.onSuggestionSelected(matchIndex, disposition, suggestion.getType(),
- currentPageUrl, pageClassification, elapsedTimeSinceModified, autocompleteLength,
- webContents);
+ mAutocomplete.onSuggestionSelected(match, suggestionLine, disposition, currentPageUrl,
+ pageClassification, elapsedTimeSinceModified, autocompleteLength, webContents);
}
@Override