Commit 365b0274 authored by manukh's avatar manukh Committed by Commit Bot

[omnibox] [max-suggest] Add dynamic suggestions limit per # of URLs.

Follow up to crrev.com/c/2274466 . The previous CL updated the
autocomplete results to conditionally boost the max suggestions.
However, the search provider limits the # of suggestions it provides to
the unboosted limit, rendering the boost ineffective.

This CL updates the search provider to send the max of GetMaxMatches
(e.g. 8) and kDynamicMaxAutocompleteIncreasedLimitParam (e.g. 10).

Change-Id: Iaa9933b4445874488d42d831d76ef30a2d9e3e8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293458Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787856}
parent 17e4c3c7
...@@ -112,6 +112,14 @@ size_t AutocompleteResult::GetMaxMatches(bool input_from_omnibox_focus) { ...@@ -112,6 +112,14 @@ size_t AutocompleteResult::GetMaxMatches(bool input_from_omnibox_focus) {
return field_trial_value; return field_trial_value;
} }
// static
size_t AutocompleteResult::GetDynamicMaxMatches() {
return base::GetFieldTrialParamByFeatureAsInt(
omnibox::kDynamicMaxAutocomplete,
OmniboxFieldTrial::kDynamicMaxAutocompleteIncreasedLimitParam,
AutocompleteResult::GetMaxMatches());
}
AutocompleteResult::AutocompleteResult() { AutocompleteResult::AutocompleteResult() {
// Reserve enough space for the maximum number of matches we'll show in either // Reserve enough space for the maximum number of matches we'll show in either
// on-focus or prefix-suggest mode. // on-focus or prefix-suggest mode.
...@@ -639,13 +647,11 @@ size_t AutocompleteResult::CalculateNumMatchesPerUrlCount( ...@@ -639,13 +647,11 @@ size_t AutocompleteResult::CalculateNumMatchesPerUrlCount(
const ACMatches& matches, const ACMatches& matches,
const CompareWithDemoteByType<AutocompleteMatch>& comparing_object) { const CompareWithDemoteByType<AutocompleteMatch>& comparing_object) {
size_t base_limit = GetMaxMatches(); size_t base_limit = GetMaxMatches();
size_t increased_limit = GetDynamicMaxMatches();
size_t url_cutoff = base::GetFieldTrialParamByFeatureAsInt( size_t url_cutoff = base::GetFieldTrialParamByFeatureAsInt(
omnibox::kDynamicMaxAutocomplete, omnibox::kDynamicMaxAutocomplete,
OmniboxFieldTrial::kDynamicMaxAutocompleteUrlCutoffParam, 0); OmniboxFieldTrial::kDynamicMaxAutocompleteUrlCutoffParam, 0);
size_t increased_limit = base::GetFieldTrialParamByFeatureAsInt( DCHECK(increased_limit > base_limit);
omnibox::kDynamicMaxAutocomplete,
OmniboxFieldTrial::kDynamicMaxAutocompleteIncreasedLimitParam,
base_limit);
size_t num_matches = 0; size_t num_matches = 0;
size_t num_url_matches = 0; size_t num_url_matches = 0;
......
...@@ -33,8 +33,12 @@ class AutocompleteResult { ...@@ -33,8 +33,12 @@ class AutocompleteResult {
// Max number of matches we'll show from the various providers. This limit may // Max number of matches we'll show from the various providers. This limit may
// be different for zero suggest (i.e. when |input_from_omnibox_focus| is // be different for zero suggest (i.e. when |input_from_omnibox_focus| is
// true) and non zero suggest. // true) and non zero suggest. Does not take into account the boost
// conditionally provided by the omnibox::kDynamicMaxAutocomplete feature.
static size_t GetMaxMatches(bool input_from_omnibox_focus = false); static size_t GetMaxMatches(bool input_from_omnibox_focus = false);
// Defaults to GetMaxMatches if omnibox::kDynamicMaxAutocomplete is disabled;
// otherwise returns the boosted dynamic limit.
static size_t GetDynamicMaxMatches();
AutocompleteResult(); AutocompleteResult();
~AutocompleteResult(); ~AutocompleteResult();
......
...@@ -1090,12 +1090,13 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { ...@@ -1090,12 +1090,13 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
// always include in that set a legal default match if possible. If Instant // always include in that set a legal default match if possible. If Instant
// Extended is enabled and we have server-provided (and thus hopefully more // Extended is enabled and we have server-provided (and thus hopefully more
// accurate) scores for some suggestions, we allow more of those, until we // accurate) scores for some suggestions, we allow more of those, until we
// reach AutocompleteResult::GetMaxMatches() total matches (that is, enough to // reach AutocompleteResult::GetDynamicMaxMatches() total matches (that is,
// fill the whole popup). // enough to fill the whole popup).
// //
// We will always return any verbatim matches, no matter how we obtained their // We will always return any verbatim matches, no matter how we obtained their
// scores, unless we have already accepted AutocompleteResult::GetMaxMatches() // scores, unless we have already accepted
// higher-scoring matches under the conditions above. // AutocompleteResult::GetDynamicMaxMatches() higher-scoring matches under
// the conditions above.
std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant); std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant);
// Guarantee that if there's a legal default match anywhere in the result // Guarantee that if there's a legal default match anywhere in the result
...@@ -1117,7 +1118,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { ...@@ -1117,7 +1118,7 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
size_t num_suggestions = 0; size_t num_suggestions = 0;
for (ACMatches::const_iterator i(matches.begin()); for (ACMatches::const_iterator i(matches.begin());
(i != matches.end()) && (i != matches.end()) &&
(matches_.size() < AutocompleteResult::GetMaxMatches()); (matches_.size() < AutocompleteResult::GetDynamicMaxMatches());
++i) { ++i) {
// SEARCH_OTHER_ENGINE is only used in the SearchProvider for the keyword // SEARCH_OTHER_ENGINE is only used in the SearchProvider for the keyword
// verbatim result, so this condition basically means "if this match is a // verbatim result, so this condition basically means "if this match is a
...@@ -1612,7 +1613,7 @@ void SearchProvider::PrefetchImages(SearchSuggestionParser::Results* results) { ...@@ -1612,7 +1613,7 @@ void SearchProvider::PrefetchImages(SearchSuggestionParser::Results* results) {
// are processed in descending order of relevance so the first suggestions are // are processed in descending order of relevance so the first suggestions are
// the ones to be shown; prefetching images for the rest would be wasteful. // the ones to be shown; prefetching images for the rest would be wasteful.
std::vector<GURL> prefetch_image_urls; std::vector<GURL> prefetch_image_urls;
int prefetch_limit = AutocompleteResult::GetMaxMatches(); int prefetch_limit = AutocompleteResult::GetDynamicMaxMatches();
for (const auto& suggestion : results->suggest_results) { for (const auto& suggestion : results->suggest_results) {
if (prefetch_limit <= 0) if (prefetch_limit <= 0)
break; break;
......
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