Commit 2e945df8 authored by Min Qin's avatar Min Qin Committed by Commit Bot

Send AQS params when query tile is selected from omnibox

If clicking on a tile triggers a search, currently the AQS params are
missing as the search URL is built directly from TemplateUrlService.
This CL fixes that behavior by going through the AutocompleteController
to find the right AutocompleteMatch, and build the search URL from it.

BUG=1110034

Change-Id: I6479726c93b8dd71d21c2753c35284d7fd5abc35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324770
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: Ender 💬 = 🕐 (ping me for faster reviews) <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#794667}
parent e698c86a
...@@ -365,22 +365,51 @@ public class AutocompleteController { ...@@ -365,22 +365,51 @@ public class AutocompleteController {
/** /**
* Updates aqs parameters on the selected match that we will navigate to and returns the * Updates aqs parameters on the selected match that we will navigate to and returns the
* updated URL. |selected_index| is the position of the selected match and * updated URL. |selectedIndex| and |hashCode| is the position and hash code of the selected
* |elapsed_time_since_input_change| is the time in ms between the first typed input and match * match. |elapsedTimeSinceInputChange| is the time in ms between the first typed input
* selection. * and match selection.
* *
* @param selectedIndex The index of the autocomplete entry selected. * @param selectedIndex The index of the autocomplete entry selected.
* @param hashCode Hash code of the OmniboxSuggestion object that is selected.
* @param elapsedTimeSinceInputChange The number of ms between the time the user started * @param elapsedTimeSinceInputChange The number of ms between the time the user started
* typing in the omnibox and the time the user has selected * typing in the omnibox and the time the user has selected
* a suggestion. * a suggestion.
* @return The url to navigate to for this match with aqs parameter updated, if we are
* making a Google search query.
*/ */
GURL updateMatchDestinationUrlWithQueryFormulationTime( GURL updateMatchDestinationUrlWithQueryFormulationTime(
int selectedIndex, int hashCode, long elapsedTimeSinceInputChange) { int selectedIndex, int hashCode, long elapsedTimeSinceInputChange) {
return updateMatchDestinationUrlWithQueryFormulationTime(
selectedIndex, hashCode, elapsedTimeSinceInputChange, null, null);
}
/**
* Updates destination url on the selected match that we will navigate to and returns the
* updated URL. |selectedIndex| and |hashCode| is the position and hash code of the selected
* match. |elapsedTimeSinceInputChange| is the time in ms between the first typed input
* and match selection. If |newQueryText| and |newQueryParams| is not empty, they will be
* used to replace the existing query string and query params.
* For example, if |elapsedTimeSinceInputChange| > 0, |newQyeryText| is "Politics news".
* and the existing destination URL is "www.google.com/search?q=News+&aqs=chrome.0.69i...l3",
* the returned new URL will be of the format
* "www.google.com/search?q=Politics+news&aqs=chrome.0.69i...l3.1409j0j9" where
* ".1409j0j9" is the encoded elapsed time.
*
* @param selectedIndex The index of the autocomplete entry selected.
* @param hashCode Hash code of the OmniboxSuggestion object that is selected.
* @param elapsedTimeSinceInputChange The number of ms between the time the user started
* typing in the omnibox and the time the user has selected
* a suggestion.
* @param newQueryText The new query string that will replace the existing one.
* @param newQueryParams A list of search params to be appended to the query.
* @return The url to navigate to for this match with aqs parameter, query string and parameters
* updated, if we are making a Google search query.
*/
GURL updateMatchDestinationUrlWithQueryFormulationTime(int selectedIndex, int hashCode,
long elapsedTimeSinceInputChange, String newQueryText, List<String> newQueryParams) {
return AutocompleteControllerJni.get().updateMatchDestinationURLWithQueryFormulationTime( return AutocompleteControllerJni.get().updateMatchDestinationURLWithQueryFormulationTime(
mNativeAutocompleteControllerAndroid, AutocompleteController.this, selectedIndex, mNativeAutocompleteControllerAndroid, AutocompleteController.this, selectedIndex,
hashCode, elapsedTimeSinceInputChange); hashCode, elapsedTimeSinceInputChange, newQueryText,
newQueryParams == null ? null
: newQueryParams.toArray(new String[newQueryParams.size()]));
} }
/** /**
...@@ -418,7 +447,8 @@ public class AutocompleteController { ...@@ -418,7 +447,8 @@ public class AutocompleteController {
AutocompleteController caller, int selectedIndex, int hashCode); AutocompleteController caller, int selectedIndex, int hashCode);
GURL updateMatchDestinationURLWithQueryFormulationTime( GURL updateMatchDestinationURLWithQueryFormulationTime(
long nativeAutocompleteControllerAndroid, AutocompleteController caller, long nativeAutocompleteControllerAndroid, AutocompleteController caller,
int selectedIndex, int hashCode, long elapsedTimeSinceInputChange); int selectedIndex, int hashCode, long elapsedTimeSinceInputChange,
String newQueryText, String[] newQueryParams);
Tab findMatchingTabWithUrl( Tab findMatchingTabWithUrl(
long nativeAutocompleteControllerAndroid, AutocompleteController caller, GURL url); long nativeAutocompleteControllerAndroid, AutocompleteController caller, GURL url);
/** /**
......
...@@ -49,7 +49,6 @@ import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewDeleg ...@@ -49,7 +49,6 @@ import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewDeleg
import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler; import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.query_tiles.QueryTileUtils; import org.chromium.chrome.browser.query_tiles.QueryTileUtils;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.share.ShareDelegate; import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabSelectionType; import org.chromium.chrome.browser.tab.TabSelectionType;
...@@ -509,10 +508,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -509,10 +508,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
// For last level tile, start a search query, unless we want to let user have a chance to // For last level tile, start a search query, unless we want to let user have a chance to
// edit the query. // edit the query.
if (queryTile.children.isEmpty() && !QueryTileUtils.isQueryEditingEnabled()) { if (queryTile.children.isEmpty() && !QueryTileUtils.isQueryEditingEnabled()) {
String url = TemplateUrlServiceFactory.get().getUrlForSearchQuery( launchSearchUrlForQueryTileSuggestion(queryTile);
queryTile.queryText, queryTile.searchParams);
mDelegate.loadUrl(url, PageTransition.LINK, mLastActionUpTimestamp);
mDelegate.setKeyboardVisibility(false);
return; return;
} }
...@@ -688,7 +684,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -688,7 +684,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
// receive suggestions until the native side is ready, so this is safe // receive suggestions until the native side is ready, so this is safe
assert mNativeInitialized assert mNativeInitialized
: "updateSuggestionUrlIfNeeded called before native initialization"; : "updateSuggestionUrlIfNeeded called before native initialization";
if (suggestion.getType() == OmniboxSuggestionType.VOICE_SUGGEST if (suggestion.getType() == OmniboxSuggestionType.VOICE_SUGGEST
|| suggestion.getType() == OmniboxSuggestionType.TILE_SUGGESTION) { || suggestion.getType() == OmniboxSuggestionType.TILE_SUGGESTION) {
return suggestion.getUrl(); return suggestion.getUrl();
...@@ -704,11 +699,8 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -704,11 +699,8 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
// TODO(mariakhomenko): Ideally we want to update match destination URL with new aqs // TODO(mariakhomenko): Ideally we want to update match destination URL with new aqs
// for query in the omnibox and voice suggestions, but it's currently difficult to do. // for query in the omnibox and voice suggestions, but it's currently difficult to do.
long elapsedTimeSinceInputChange = mNewOmniboxEditSessionTimestamp > 0
? (SystemClock.elapsedRealtime() - mNewOmniboxEditSessionTimestamp)
: -1;
GURL updatedUrl = mAutocomplete.updateMatchDestinationUrlWithQueryFormulationTime( GURL updatedUrl = mAutocomplete.updateMatchDestinationUrlWithQueryFormulationTime(
verifiedIndex, suggestion.hashCode(), elapsedTimeSinceInputChange); verifiedIndex, suggestion.hashCode(), getElapsedTimeSinceInputChange());
return updatedUrl == null ? suggestion.getUrl() : updatedUrl; return updatedUrl == null ? suggestion.getUrl() : updatedUrl;
} }
...@@ -1149,9 +1141,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -1149,9 +1141,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
String currentPageUrl = mDataProvider.getCurrentUrl(); String currentPageUrl = mDataProvider.getCurrentUrl();
int pageClassification = int pageClassification =
mDataProvider.getPageClassification(mDelegate.didFocusUrlFromFakebox()); mDataProvider.getPageClassification(mDelegate.didFocusUrlFromFakebox());
long elapsedTimeSinceModified = mNewOmniboxEditSessionTimestamp > 0 long elapsedTimeSinceModified = getElapsedTimeSinceInputChange();
? (SystemClock.elapsedRealtime() - mNewOmniboxEditSessionTimestamp)
: -1;
int autocompleteLength = mUrlBarEditingTextProvider.getTextWithAutocomplete().length() int autocompleteLength = mUrlBarEditingTextProvider.getTextWithAutocomplete().length()
- mUrlBarEditingTextProvider.getTextWithoutAutocomplete().length(); - mUrlBarEditingTextProvider.getTextWithoutAutocomplete().length();
WebContents webContents = WebContents webContents =
...@@ -1168,4 +1158,39 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -1168,4 +1158,39 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
mDelegate.setKeyboardVisibility(true); mDelegate.setKeyboardVisibility(true);
} }
} }
/**
* @return elapsed time (in milliseconds) since last input or -1 if user has chosen
* a zero-prefix suggestion.
*/
private long getElapsedTimeSinceInputChange() {
return mNewOmniboxEditSessionTimestamp > 0
? (SystemClock.elapsedRealtime() - mNewOmniboxEditSessionTimestamp)
: -1;
}
/**
* Launches the search URL for the query tile suggestion.
* @param queryTile The query tile user selected.
*/
@SuppressWarnings("VisibleForTests")
private void launchSearchUrlForQueryTileSuggestion(QueryTile queryTile) {
int position = -1;
int hashCode = 0;
int suggestionCount = getSuggestionCount();
// Find the suggestion position and hashCode.
for (int i = 0; i < suggestionCount; ++i) {
OmniboxSuggestion suggestion = getSuggestionAt(i);
if (suggestion.getType() == OmniboxSuggestionType.TILE_SUGGESTION) {
position = i;
hashCode = suggestion.hashCode();
break;
}
}
GURL updatedUrl = mAutocomplete.updateMatchDestinationUrlWithQueryFormulationTime(position,
hashCode, getElapsedTimeSinceInputChange(), queryTile.queryText,
queryTile.searchParams);
mDelegate.loadUrl(updatedUrl.getSpec(), PageTransition.LINK, mLastActionUpTimestamp);
mDelegate.setKeyboardVisibility(false);
}
} }
...@@ -290,7 +290,6 @@ void AutocompleteControllerAndroid::OnSuggestionSelected( ...@@ -290,7 +290,6 @@ void AutocompleteControllerAndroid::OnSuggestionSelected(
const auto& match = const auto& match =
autocomplete_controller_->result().match_at(selected_index); autocomplete_controller_->result().match_at(selected_index);
SuggestionAnswer::LogAnswerUsed(match.answer); SuggestionAnswer::LogAnswerUsed(match.answer);
TemplateURLService* template_url_service = TemplateURLService* template_url_service =
TemplateURLServiceFactory::GetForProfile(profile_); TemplateURLServiceFactory::GetForProfile(profile_);
if (template_url_service && if (template_url_service &&
...@@ -352,12 +351,33 @@ ScopedJavaLocalRef<jobject> AutocompleteControllerAndroid:: ...@@ -352,12 +351,33 @@ ScopedJavaLocalRef<jobject> AutocompleteControllerAndroid::
const JavaParamRef<jobject>& obj, const JavaParamRef<jobject>& obj,
jint selected_index, jint selected_index,
jint hash_code, jint hash_code,
jlong elapsed_time_since_input_change) { jlong elapsed_time_since_input_change,
const base::android::JavaParamRef<jstring>& jnew_query_text,
const base::android::JavaParamRef<jobjectArray>& jnew_query_params) {
if (!IsValidMatch(env, selected_index, hash_code)) if (!IsValidMatch(env, selected_index, hash_code))
return ScopedJavaLocalRef<jstring>(); return ScopedJavaLocalRef<jstring>();
AutocompleteMatch match( AutocompleteMatch match(
autocomplete_controller_->result().match_at(selected_index)); autocomplete_controller_->result().match_at(selected_index));
if (!jnew_query_text.is_null()) {
base::string16 query =
base::android::ConvertJavaStringToUTF16(env, jnew_query_text);
if (!match.search_terms_args) {
match.search_terms_args.reset(new TemplateURLRef::SearchTermsArgs(query));
} else {
match.search_terms_args->search_terms = query;
}
}
if (!jnew_query_params.is_null() && match.search_terms_args) {
std::vector<std::string> params;
base::android::AppendJavaStringArrayToStringVector(env, jnew_query_params,
&params);
// The query params are from the query tiles server and doesn't need to be
// escaped.
match.search_terms_args->additional_query_params =
base::JoinString(params, "&");
}
autocomplete_controller_->UpdateMatchDestinationURLWithQueryFormulationTime( autocomplete_controller_->UpdateMatchDestinationURLWithQueryFormulationTime(
base::TimeDelta::FromMilliseconds(elapsed_time_since_input_change), base::TimeDelta::FromMilliseconds(elapsed_time_since_input_change),
&match); &match);
......
...@@ -84,7 +84,9 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer, ...@@ -84,7 +84,9 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
const base::android::JavaParamRef<jobject>& obj, const base::android::JavaParamRef<jobject>& obj,
jint selected_index, jint selected_index,
jint hash_code, jint hash_code,
jlong elapsed_time_since_input_change); jlong elapsed_time_since_input_change,
const base::android::JavaParamRef<jstring>& jnew_query_text,
const base::android::JavaParamRef<jobjectArray>& jnew_query_params);
base::android::ScopedJavaLocalRef<jobject> FindMatchingTabWithUrl( base::android::ScopedJavaLocalRef<jobject> FindMatchingTabWithUrl(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj, const base::android::JavaParamRef<jobject>& obj,
......
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