[omnibox] [max-suggest] Add dynamic suggestions limit per # of URLs.
Adds a OmniboxDynamicMaxAutocomplete feature with 2 params,
OmniboxDynamicMaxAutocompleteUrlCutoff &
OmniboxDynamicMaxAutocompleteIncreasedLimit.
- When disabled, the omnibox allows UIMaxAutocompleteMatches
suggestions.
- When dynamic max autocompletion is enabled, the omnibox allows
suggestions up to the increased limit if doing so has URL cutoff or less
URL suggestions. It could also allow a partial increase if doing so
avoids showing more than the cutoff.
E.g. a UIMaxAutocompleteMatches of 8, URL cutoff of 2, and increased
limit of 10 translates to "show 10 or 9 suggestions if doing so includes
at most 2 URLs; otherwise show 8 suggestions.
Change-Id: I2f5e589f9e5307a62158223e6f6965476e55c257
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274466
Commit-Queue: manuk hovanesian <[email protected]>
Reviewed-by: Justin Donnelly <[email protected]>
Cr-Commit-Position: refs/heads/master@{#786541}
diff --git a/components/omnibox/browser/autocomplete_result.cc b/components/omnibox/browser/autocomplete_result.cc
index 01cb3839..e39a40e1 100644
--- a/components/omnibox/browser/autocomplete_result.cc
+++ b/components/omnibox/browser/autocomplete_result.cc
@@ -187,12 +187,12 @@
break;
case OmniboxFieldTrial::EMPHASIZE_WHEN_TITLE_MATCHES:
emphasize = !i.description.empty() &&
- AutocompleteMatch::HasMatchStyle(i.description_class);
+ AutocompleteMatch::HasMatchStyle(i.description_class);
break;
case OmniboxFieldTrial::EMPHASIZE_WHEN_ONLY_TITLE_MATCHES:
emphasize = !i.description.empty() &&
- AutocompleteMatch::HasMatchStyle(i.description_class) &&
- !AutocompleteMatch::HasMatchStyle(i.contents_class);
+ AutocompleteMatch::HasMatchStyle(i.description_class) &&
+ !AutocompleteMatch::HasMatchStyle(i.contents_class);
break;
case OmniboxFieldTrial::EMPHASIZE_NEVER:
break;
@@ -262,8 +262,9 @@
LimitNumberOfURLsShown(GetMaxMatches(input.from_omnibox_focus()),
max_url_count, comparing_object);
- // Limit total matches per OmniboxUIExperimentMaxAutocompleteMatches &
- // OmniboxMaxZeroSuggestMatches.
+ // Limit total matches accounting for suggestions score <= 0, sub matches, and
+ // feature configs such as OmniboxUIExperimentMaxAutocompleteMatches,
+ // OmniboxMaxZeroSuggestMatches, and OmniboxDynamicMaxAutocomplete.
const size_t num_matches = CalculateNumMatches(input.from_omnibox_focus(),
matches_, comparing_object);
matches_.resize(num_matches);
@@ -614,6 +615,15 @@
bool input_from_omnibox_focus,
const ACMatches& matches,
const CompareWithDemoteByType<AutocompleteMatch>& comparing_object) {
+ // Use alternative CalculateNumMatchesPerUrlCount if applicable.
+ // TODO (manukh): CalculateNumMatchesPerUrlCount doesn't account for
+ // submatches. That's ok since we plan to replace submatches with dedicated
+ // row suggestions. Otherwise, CalculateNumMatchesPerUrlCount should account
+ // for them.
+ if (!input_from_omnibox_focus &&
+ base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures) &&
+ base::FeatureList::IsEnabled(omnibox::kDynamicMaxAutocomplete))
+ return CalculateNumMatchesPerUrlCount(matches, comparing_object);
// In the process of trimming, drop all matches with a demoted relevance
// score of 0.
size_t max_matches_by_policy = GetMaxMatches(input_from_omnibox_focus);
@@ -634,6 +644,35 @@
return num_matches;
}
+// static
+size_t AutocompleteResult::CalculateNumMatchesPerUrlCount(
+ const ACMatches& matches,
+ const CompareWithDemoteByType<AutocompleteMatch>& comparing_object) {
+ size_t base_limit = GetMaxMatches();
+ size_t url_cutoff = base::GetFieldTrialParamByFeatureAsInt(
+ omnibox::kDynamicMaxAutocomplete,
+ OmniboxFieldTrial::kDynamicMaxAutocompleteUrlCutoffParam, 0);
+ size_t increased_limit = base::GetFieldTrialParamByFeatureAsInt(
+ omnibox::kDynamicMaxAutocomplete,
+ OmniboxFieldTrial::kDynamicMaxAutocompleteIncreasedLimitParam,
+ base_limit);
+
+ size_t num_matches = 0;
+ size_t num_url_matches = 0;
+ for (auto match : matches) {
+ // Matches scored less than 0 won't be shown anyways, so we can break early.
+ if (comparing_object.GetDemotedRelevance(matches[num_matches]) <= 0)
+ break;
+ if (!AutocompleteMatch::IsSearchType(match.type))
+ num_url_matches++;
+ size_t limit = num_url_matches <= url_cutoff ? increased_limit : base_limit;
+ if (num_matches >= limit)
+ break;
+ num_matches++;
+ }
+ return num_matches;
+}
+
void AutocompleteResult::Reset() {
matches_.clear();
}
diff --git a/components/omnibox/browser/autocomplete_result.h b/components/omnibox/browser/autocomplete_result.h
index e6452408..339fbf7 100644
--- a/components/omnibox/browser/autocomplete_result.h
+++ b/components/omnibox/browser/autocomplete_result.h
@@ -124,6 +124,12 @@
bool input_from_omnibox_focus,
const ACMatches& matches,
const CompareWithDemoteByType<AutocompleteMatch>& comparing_object);
+ // Determines how many matches to keep depending on how many URLs would be
+ // shown. CalculateNumMatches defers to CalculateNumMatchesPerUrlCount if the
+ // kDynamicMaxAutocomplete feature is enabled.
+ static size_t CalculateNumMatchesPerUrlCount(
+ const ACMatches& matches,
+ const CompareWithDemoteByType<AutocompleteMatch>& comparing_object);
const SearchSuggestionParser::HeadersMap& headers_map() const {
return headers_map_;
diff --git a/components/omnibox/browser/autocomplete_result_unittest.cc b/components/omnibox/browser/autocomplete_result_unittest.cc
index 0763faa3..ebe4dc09 100644
--- a/components/omnibox/browser/autocomplete_result_unittest.cc
+++ b/components/omnibox/browser/autocomplete_result_unittest.cc
@@ -1999,6 +1999,51 @@
}
}
+TEST_F(AutocompleteResultTest, CalculateNumMatchesPerUrlCountTest) {
+ CompareWithDemoteByType<AutocompleteMatch> comparison_object(
+ metrics::OmniboxEventProto::OTHER);
+ enum SuggestionType { search, url };
+
+ auto test = [comparison_object](std::string base_limit,
+ std::string url_cutoff,
+ std::string increased_limit,
+ std::vector<SuggestionType> types,
+ size_t expected_num_matches) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeaturesAndParameters(
+ {{omnibox::kDynamicMaxAutocomplete,
+ {{OmniboxFieldTrial::kDynamicMaxAutocompleteUrlCutoffParam,
+ url_cutoff},
+ {OmniboxFieldTrial::kDynamicMaxAutocompleteIncreasedLimitParam,
+ increased_limit}}},
+ {omnibox::kUIExperimentMaxAutocompleteMatches,
+ {{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, base_limit}}},
+ {omnibox::kNewSearchFeatures, {}}},
+ {});
+
+ ACMatches matches;
+ for (auto type : types) {
+ AutocompleteMatch m;
+ m.relevance = 100;
+ if (type)
+ m.type = AutocompleteMatchType::URL_WHAT_YOU_TYPED;
+ matches.push_back(m);
+ }
+ const size_t num_matches = AutocompleteResult::CalculateNumMatches(
+ false, matches, comparison_object);
+ EXPECT_EQ(num_matches, expected_num_matches);
+ };
+
+ test("2", "0", "4", {search}, 1);
+ test("2", "0", "4", {search, search, search, search, search}, 4);
+ test("2", "0", "4", {search, search, search, search, url}, 4);
+ test("2", "0", "4", {search, search, search, url, search}, 3);
+ test("2", "0", "4", {search, search, url, search, search}, 2);
+ test("2", "0", "4", {search, url, search, search, search}, 2);
+ test("2", "1", "4", {search, url, search, search, search}, 4);
+ test("2", "1", "4", {search, url, search, url, search}, 3);
+}
+
TEST_F(AutocompleteResultTest, ClipboardSuggestionOnTopOfSearchSuggestionTest) {
// clang-format off
TestData data[] = {
diff --git a/components/omnibox/browser/omnibox_field_trial.cc b/components/omnibox/browser/omnibox_field_trial.cc
index e4d02c3..d069c3ec 100644
--- a/components/omnibox/browser/omnibox_field_trial.cc
+++ b/components/omnibox/browser/omnibox_field_trial.cc
@@ -397,6 +397,28 @@
return default_max_matches_per_provider;
}
+bool OmniboxFieldTrial::IsMaxURLMatchesFeatureEnabled() {
+ // If new search features are disabled, return the default/launched value for
+ // the respective platforms, independent of the state of the Feature.
+ if (!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures))
+ return omnibox::kOmniboxMaxURLMatchesEnabledByDefault;
+
+ return base::FeatureList::IsEnabled(omnibox::kOmniboxMaxURLMatches);
+}
+
+size_t OmniboxFieldTrial::GetMaxURLMatches() {
+ constexpr size_t kDefaultMaxURLMatches = 7;
+
+ // If new search features are disabled, ignore the parameter and use the
+ // default value.
+ if (!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures))
+ return kDefaultMaxURLMatches;
+
+ return base::GetFieldTrialParamByFeatureAsInt(
+ omnibox::kOmniboxMaxURLMatches,
+ OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, kDefaultMaxURLMatches);
+}
+
void OmniboxFieldTrial::GetDefaultHUPScoringParams(
HUPScoringParams* scoring_params) {
ScoreBuckets* type_score_buckets = &scoring_params->typed_count_buckets;
@@ -661,19 +683,6 @@
return static_cast<EmphasizeTitlesCondition>(value);
}
-size_t OmniboxFieldTrial::GetMaxURLMatches() {
- constexpr size_t kDefaultMaxURLMatches = 7;
-
- // If new search features are disabled, ignore the parameter and use the
- // default value.
- if (!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures))
- return kDefaultMaxURLMatches;
-
- return base::GetFieldTrialParamByFeatureAsInt(
- omnibox::kOmniboxMaxURLMatches,
- OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, kDefaultMaxURLMatches);
-}
-
bool OmniboxFieldTrial::IsReverseAnswersEnabled() {
return base::FeatureList::IsEnabled(omnibox::kOmniboxReverseAnswers);
}
@@ -711,15 +720,6 @@
return base::FeatureList::IsEnabled(omnibox::kExperimentalKeywordMode);
}
-bool OmniboxFieldTrial::IsMaxURLMatchesFeatureEnabled() {
- // If new search features are disabled, return the default/launched value for
- // the respective platforms, independent of the state of the Feature.
- if (!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures))
- return omnibox::kOmniboxMaxURLMatchesEnabledByDefault;
-
- return base::FeatureList::IsEnabled(omnibox::kOmniboxMaxURLMatches);
-}
-
bool OmniboxFieldTrial::IsRichAutocompletionEnabled() {
return base::FeatureList::IsEnabled(omnibox::kRichAutocompletion);
}
@@ -898,6 +898,10 @@
"UIMaxAutocompleteMatchesByProvider";
const char OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam[] =
"UIMaxAutocompleteMatches";
+const char OmniboxFieldTrial::kDynamicMaxAutocompleteUrlCutoffParam[] =
+ "OmniboxDynamicMaxAutocompleteUrlCutoff";
+const char OmniboxFieldTrial::kDynamicMaxAutocompleteIncreasedLimitParam[] =
+ "OmniboxDynamicMaxAutocompleteIncreasedLimit";
const char OmniboxFieldTrial::kOnDeviceHeadModelLocaleConstraint[] =
"ForceModelLocaleConstraint";
diff --git a/components/omnibox/browser/omnibox_field_trial.h b/components/omnibox/browser/omnibox_field_trial.h
index 768c2fa..38723a7e 100644
--- a/components/omnibox/browser/omnibox_field_trial.h
+++ b/components/omnibox/browser/omnibox_field_trial.h
@@ -212,13 +212,22 @@
DemotionMultipliers* demotions_by_type);
// ---------------------------------------------------------
-// For the UIMaxAutocompleteMatchesByProvider experiment that's part of the
-// bundled omnibox field trial.
+// For experiments related to the number of suggestions shown.
// If the user is in an experiment group that specifies the max results for a
// particular provider, returns the limit. Otherwise returns the default limit.
size_t GetProviderMaxMatches(AutocompleteProvider::Type provider);
+// Returns whether the feature to limit the number of shown URL matches
+// is enabled.
+bool IsMaxURLMatchesFeatureEnabled();
+
+// Returns the maximum number of URL matches that should be allowed within
+// the Omnibox if there are search-type matches available to replace them.
+// If the capping feature is not enabled, or the parameter cannot be
+// parsed, it returns 0.
+size_t GetMaxURLMatches();
+
// ---------------------------------------------------------
// For the HistoryURL provider new scoring experiment that is part of the
// bundled omnibox field trial.
@@ -368,12 +377,6 @@
EmphasizeTitlesCondition GetEmphasizeTitlesConditionForInput(
const AutocompleteInput& input);
-// Returns the maximum number of URL matches that should be allowed within
-// the Omnibox if there are search-type matches available to replace them.
-// If the capping feature is not enabled, or the parameter cannot be
-// parsed, it returns 0.
-size_t GetMaxURLMatches();
-
// ---------------------------------------------------------
// For UI experiments.
@@ -404,10 +407,6 @@
// assortment of keyword mode experiments.
bool IsExperimentalKeywordModeEnabled();
-// Returns whether the feature to limit the number of shown URL matches
-// is enabled.
-bool IsMaxURLMatchesFeatureEnabled();
-
// Rich autocompletion.
bool IsRichAutocompletionEnabled();
bool RichAutocompletionAutocompleteTitles();
@@ -497,6 +496,17 @@
extern const char kOmniboxMaxURLMatchesParam[];
extern const char kUIMaxAutocompleteMatchesByProviderParam[];
extern const char kUIMaxAutocompleteMatchesParam[];
+// The URL cutoff and increased limit for dynamic max autocompletion.
+// - When dynamic max autocompletion is disabled, the omnibox allows
+// UIMaxAutocompleteMatches suggestions.
+// - When dynamic max autocompletion is enabled, the omnibox allows
+// suggestions up to the increased limit if doing so has URL cutoff or less
+// URL suggestions.
+// E.g. a UIMaxAutocompleteMatches of 8, URL cutoff of 2, and increased limit of
+// 10 translates to "show 10 or 9 suggestions if doing so includes at most 2
+// URLs; otherwise show 8 suggestions.
+extern const char kDynamicMaxAutocompleteUrlCutoffParam[];
+extern const char kDynamicMaxAutocompleteIncreasedLimitParam[];
// Parameter names used by on device head provider.
// These four parameters are shared by both non-incognito and incognito.
diff --git a/components/omnibox/common/omnibox_features.cc b/components/omnibox/common/omnibox_features.cc
index c4c7b56..f12af1d3 100644
--- a/components/omnibox/common/omnibox_features.cc
+++ b/components/omnibox/common/omnibox_features.cc
@@ -215,6 +215,12 @@
? base::FEATURE_ENABLED_BY_DEFAULT
: base::FEATURE_DISABLED_BY_DEFAULT};
+// Feature used to cap max suggestions to a dynamic limit based on how many URLs
+// would be shown. E.g., show up to 10 suggestions if doing so would display no
+// URLs; else show up to 8 suggestions if doing so would include 1 or more URLs.
+const base::Feature kDynamicMaxAutocomplete{"OmniboxDynamicMaxAutocomplete",
+ base::FEATURE_DISABLED_BY_DEFAULT};
+
// Feature that configures ZeroSuggestProvider using the "ZeroSuggestVariant"
// per-page-classification parameter.
//
diff --git a/components/omnibox/common/omnibox_features.h b/components/omnibox/common/omnibox_features.h
index c5813af..f5e23308 100644
--- a/components/omnibox/common/omnibox_features.h
+++ b/components/omnibox/common/omnibox_features.h
@@ -52,6 +52,7 @@
// OmniboxFieldTrial.
extern const bool kOmniboxMaxURLMatchesEnabledByDefault;
extern const base::Feature kOmniboxMaxURLMatches;
+extern const base::Feature kDynamicMaxAutocomplete;
// On-Focus Suggestions a.k.a. ZeroSuggest.
extern const base::Feature kOnFocusSuggestions;
