Commit 2623886a authored by Kevin Bailey's avatar Kevin Bailey Committed by Commit Bot

[omnibox] Raise max suggestions default and enable limit URLs by default

But allow flag override. Uses max of 8 and URL limit of 7.
Desktop only. Will be managed by a Finch roll-out.

Bug: 964049
Change-Id: I29db26586dda34c2eaeda2744feb5718010c2648
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1891458
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711483}
parent e1a3f28c
...@@ -1339,6 +1339,14 @@ TEST_F(SearchProviderTest, DefaultProviderNoSuggestRelevanceInKeywordMode) { ...@@ -1339,6 +1339,14 @@ TEST_F(SearchProviderTest, DefaultProviderNoSuggestRelevanceInKeywordMode) {
// case to this test, please consider adding it to the tests in // case to this test, please consider adding it to the tests in
// KeywordFetcherSuggestRelevance below. // KeywordFetcherSuggestRelevance below.
TEST_F(SearchProviderTest, DefaultFetcherSuggestRelevance) { TEST_F(SearchProviderTest, DefaultFetcherSuggestRelevance) {
// This test was written assuming a different default.
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters(
{
{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
},
{/* nothing disabled */});
struct { struct {
const std::string json; const std::string json;
const ExpectedMatch matches[6]; const ExpectedMatch matches[6];
...@@ -1577,6 +1585,14 @@ TEST_F(SearchProviderTest, DefaultFetcherSuggestRelevance) { ...@@ -1577,6 +1585,14 @@ TEST_F(SearchProviderTest, DefaultFetcherSuggestRelevance) {
// test is added to this TEST_F, please consider if it would be // test is added to this TEST_F, please consider if it would be
// appropriate to add to DefaultFetcherSuggestRelevance as well. // appropriate to add to DefaultFetcherSuggestRelevance as well.
TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) { TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) {
// This test was written assuming a different default.
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters(
{
{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
},
{/* nothing disabled */});
struct KeywordFetcherMatch { struct KeywordFetcherMatch {
std::string contents; std::string contents;
bool from_keyword; bool from_keyword;
...@@ -2315,6 +2331,14 @@ TEST_F(SearchProviderTest, DontCacheCalculatorSuggestions) { ...@@ -2315,6 +2331,14 @@ TEST_F(SearchProviderTest, DontCacheCalculatorSuggestions) {
} }
TEST_F(SearchProviderTest, LocalAndRemoteRelevances) { TEST_F(SearchProviderTest, LocalAndRemoteRelevances) {
// This test was written assuming a different default.
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters(
{
{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
},
{/* nothing disabled */});
// We hardcode the string "term1" below, so ensure that the search term that // We hardcode the string "term1" below, so ensure that the search term that
// got added to history already is that string. // got added to history already is that string.
ASSERT_EQ(ASCIIToUTF16("term1"), term1_); ASSERT_EQ(ASCIIToUTF16("term1"), term1_);
......
...@@ -78,9 +78,11 @@ size_t AutocompleteResult::GetMaxMatches(bool is_zero_suggest) { ...@@ -78,9 +78,11 @@ size_t AutocompleteResult::GetMaxMatches(bool is_zero_suggest) {
constexpr size_t kDefaultMaxAutocompleteMatches = 5; constexpr size_t kDefaultMaxAutocompleteMatches = 5;
if (is_zero_suggest) if (is_zero_suggest)
return kDefaultMaxAutocompleteMatches; return kDefaultMaxAutocompleteMatches;
#else #elif defined(OS_IOS) // !defined(OS_ANDROID)
constexpr size_t kDefaultMaxAutocompleteMatches = 6; constexpr size_t kDefaultMaxAutocompleteMatches = 6;
#endif // defined(OS_ANDROID) #else // !defined(OS_ANDROID) && !defined(OS_IOS)
constexpr size_t kDefaultMaxAutocompleteMatches = 8;
#endif
static_assert(kMaxAutocompletePositionValue > kDefaultMaxAutocompleteMatches, static_assert(kMaxAutocompletePositionValue > kDefaultMaxAutocompleteMatches,
"kMaxAutocompletePositionValue must be larger than the largest " "kMaxAutocompletePositionValue must be larger than the largest "
"possible autocomplete result size."); "possible autocomplete result size.");
......
...@@ -1458,6 +1458,14 @@ TEST_F(AutocompleteResultTest, SortAndCullPromoteDuplicateSearchURLs) { ...@@ -1458,6 +1458,14 @@ TEST_F(AutocompleteResultTest, SortAndCullPromoteDuplicateSearchURLs) {
} }
TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) { TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) {
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters(
{
{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
{omnibox::kOmniboxGroupSuggestionsBySearchVsUrl, {/* no params */}},
},
{/* nothing disabled */});
TestData data[] = { TestData data[] = {
{ 0, 1, 500, false }, { 0, 1, 500, false },
{ 1, 2, 600, false }, { 1, 2, 600, false },
...@@ -1480,10 +1488,6 @@ TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) { ...@@ -1480,10 +1488,6 @@ TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) {
for (size_t i = 0; i < base::size(data); ++i) for (size_t i = 0; i < base::size(data); ++i)
matches[i].type = match_types[i]; matches[i].type = match_types[i];
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
omnibox::kOmniboxGroupSuggestionsBySearchVsUrl);
AutocompleteInput input(base::ASCIIToUTF16("a"), AutocompleteInput input(base::ASCIIToUTF16("a"),
metrics::OmniboxEventProto::OTHER, metrics::OmniboxEventProto::OTHER,
TestSchemeClassifier()); TestSchemeClassifier());
...@@ -1505,7 +1509,9 @@ TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) { ...@@ -1505,7 +1509,9 @@ TEST_F(AutocompleteResultTest, SortAndCullGroupSuggestionsByType) {
TEST_F(AutocompleteResultTest, SortAndCullMaxURLMatches) { TEST_F(AutocompleteResultTest, SortAndCullMaxURLMatches) {
base::test::ScopedFeatureList feature_list; base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters( feature_list.InitWithFeaturesAndParameters(
{{omnibox::kOmniboxMaxURLMatches, {{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
{omnibox::kOmniboxMaxURLMatches,
{{OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, "3"}}}}, {{OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, "3"}}}},
{omnibox::kOmniboxGroupSuggestionsBySearchVsUrl, {omnibox::kOmniboxGroupSuggestionsBySearchVsUrl,
omnibox::kOmniboxPreserveDefaultMatchScore}); omnibox::kOmniboxPreserveDefaultMatchScore});
...@@ -1591,7 +1597,9 @@ TEST_F( ...@@ -1591,7 +1597,9 @@ TEST_F(
MOBILE_DISABLED(SortAndCullMaxURLMatchesWithPreserveAndGroupingFeatures)) { MOBILE_DISABLED(SortAndCullMaxURLMatchesWithPreserveAndGroupingFeatures)) {
base::test::ScopedFeatureList feature_list; base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeaturesAndParameters( feature_list.InitWithFeaturesAndParameters(
{{omnibox::kOmniboxMaxURLMatches, {{omnibox::kUIExperimentMaxAutocompleteMatches,
{{OmniboxFieldTrial::kUIMaxAutocompleteMatchesParam, "6"}}},
{omnibox::kOmniboxMaxURLMatches,
{{OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, "3"}}}, {{OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, "3"}}},
{omnibox::kOmniboxGroupSuggestionsBySearchVsUrl, {}}, {omnibox::kOmniboxGroupSuggestionsBySearchVsUrl, {}},
{omnibox::kOmniboxPreserveDefaultMatchScore, {}}}, {omnibox::kOmniboxPreserveDefaultMatchScore, {}}},
......
...@@ -624,10 +624,10 @@ OmniboxFieldTrial::GetEmphasizeTitlesConditionForInput( ...@@ -624,10 +624,10 @@ OmniboxFieldTrial::GetEmphasizeTitlesConditionForInput(
} }
size_t OmniboxFieldTrial::GetMaxURLMatches() { size_t OmniboxFieldTrial::GetMaxURLMatches() {
constexpr size_t kDefaultMaxURLMatches = 7;
return base::GetFieldTrialParamByFeatureAsInt( return base::GetFieldTrialParamByFeatureAsInt(
omnibox::kOmniboxMaxURLMatches, omnibox::kOmniboxMaxURLMatches,
OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, OmniboxFieldTrial::kOmniboxMaxURLMatchesParam, kDefaultMaxURLMatches);
0); // default
} }
bool OmniboxFieldTrial::IsPreserveDefaultMatchScoreEnabled() { bool OmniboxFieldTrial::IsPreserveDefaultMatchScoreEnabled() {
......
...@@ -78,8 +78,14 @@ const base::Feature kOmniboxLocalEntitySuggestions{ ...@@ -78,8 +78,14 @@ const base::Feature kOmniboxLocalEntitySuggestions{
// there are no more non-URL matches available.) If enabled, there is a // there are no more non-URL matches available.) If enabled, there is a
// companion parameter - OmniboxMaxURLMatches - which specifies the maximum // companion parameter - OmniboxMaxURLMatches - which specifies the maximum
// desired number of URL-type matches. // desired number of URL-type matches.
const base::Feature kOmniboxMaxURLMatches{"OmniboxMaxURLMatches", const base::Feature kOmniboxMaxURLMatches {
base::FEATURE_DISABLED_BY_DEFAULT}; "OmniboxMaxURLMatches",
#if defined(OS_IOS) || defined(OS_ANDROID)
base::FEATURE_DISABLED_BY_DEFAULT
#else
base::FEATURE_ENABLED_BY_DEFAULT
#endif
};
// Feature used to enable entity suggestion images and enhanced presentation // Feature used to enable entity suggestion images and enhanced presentation
// showing more context and descriptive text about the entity. // showing more context and descriptive text about the entity.
......
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