Skip to content

Commit

Permalink
[omnibox] Prefer non-shortcut matches over shortcuts when deduping.
Browse files Browse the repository at this point in the history
Bug: 1418143
Change-Id: I5df981f747af718b3287b9e6bc7e5756f0219711
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4292299
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1111756}
  • Loading branch information
justindonnelly authored and Chromium LUCI CQ committed Mar 1, 2023
1 parent c644e68 commit e6814e8
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 3 deletions.
11 changes: 10 additions & 1 deletion components/omnibox/browser/autocomplete_match.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ bool RichAutocompletionApplicable(bool enabled_all_providers,
// Gives a basis for match comparison that prefers some providers over others
// while remaining neutral with a default score of zero for most providers.
int GetDeduplicationProviderPreferenceScore(AutocompleteProvider::Type type) {
const static int shortcuts_preference =
base::FeatureList::IsEnabled(
omnibox::kPreferNonShortcutMatchesWhenDeduping)
? -1
: 0;
const static std::unordered_map<AutocompleteProvider::Type, int>
provider_preference = {
{// Prefer live document suggestions. We check provider type instead
Expand All @@ -106,8 +111,12 @@ int GetDeduplicationProviderPreferenceScore(AutocompleteProvider::Type type) {
// set, and 2) they may display enhanced information such as the
// bookmark folders path.
AutocompleteProvider::TYPE_BOOKMARK, 1},
{// Prefer non-shorcut matches over shortcuts, the latter of which may
// have stale or missing URL titles (the latter from what-you-typed
// matches).
AutocompleteProvider::TYPE_SHORTCUTS, shortcuts_preference},
{// Prefer non-fuzzy matches over fuzzy matches.
AutocompleteProvider::TYPE_HISTORY_FUZZY, -1},
AutocompleteProvider::TYPE_HISTORY_FUZZY, -2},
};
const auto it = provider_preference.find(type);
if (it == provider_preference.end()) {
Expand Down
10 changes: 10 additions & 0 deletions components/omnibox/browser/autocomplete_match_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,9 @@ TEST(AutocompleteMatchTest, BetterDuplicate) {
new FakeAutocompleteProvider(
AutocompleteProvider::Type::TYPE_HISTORY_QUICK);

scoped_refptr<FakeAutocompleteProvider> shortcuts_provider =
new FakeAutocompleteProvider(AutocompleteProvider::Type::TYPE_SHORTCUTS);

// Prefer document provider matches over other providers, even if scored
// lower.
EXPECT_TRUE(
Expand All @@ -973,6 +976,13 @@ TEST(AutocompleteMatchTest, BetterDuplicate) {
create_match(document_provider, 0),
create_match(bookmark_provider, 1000)));

// Prefer non-shortcuts provider matches over shortcuts provider matches.
EXPECT_TRUE(AutocompleteMatch::BetterDuplicate(
create_match(history_provider, 0),
create_match(shortcuts_provider, 1000)));

// Prefer non-shortcuts provider matches over shortcuts provider matches.

// Prefer more relevant matches.
EXPECT_FALSE(
AutocompleteMatch::BetterDuplicate(create_match(history_provider, 500),
Expand Down
6 changes: 6 additions & 0 deletions components/omnibox/common/omnibox_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ BASE_FEATURE(kOmniboxRemoveExcessiveRecycledViewClearCalls,
"OmniboxRemoveExcessiveRecycledViewClearCalls",
base::FEATURE_ENABLED_BY_DEFAULT);

// When enabled, deduping prefers non-shortcut provider matches, while still
// treating fuzzy provider matches as the least preferred.
BASE_FEATURE(kPreferNonShortcutMatchesWhenDeduping,
"OmniboxPreferNonShortcutMatchesWhenDeduping",
base::FEATURE_ENABLED_BY_DEFAULT);

// Determines which are culled when both tail and history cluster suggestions
// are available. See `MaybeCullTailSuggestions()`.
BASE_FEATURE(kPreferTailOverHistoryClusterSuggestions,
Expand Down
5 changes: 3 additions & 2 deletions components/omnibox/common/omnibox_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ BASE_DECLARE_FEATURE(kExperimentalKeywordMode);
BASE_DECLARE_FEATURE(kImageSearchSuggestionThumbnail);
BASE_DECLARE_FEATURE(kOmniboxRemoveSuggestionsFromClipboard);

// Features that affect the "twiddle" step of AutocompleteResult, e.g.,
// `SortAndCull()`.
// Features that affect the "twiddle" step of AutocompleteController, e.g.,
// deduping or `SortAndCull()`.
BASE_DECLARE_FEATURE(kGroupingFramework);
BASE_DECLARE_FEATURE(kOmniboxDemoteByType);
BASE_DECLARE_FEATURE(kOmniboxRemoveExcessiveRecycledViewClearCalls);
BASE_DECLARE_FEATURE(kPreferNonShortcutMatchesWhenDeduping);
BASE_DECLARE_FEATURE(kPreferTailOverHistoryClusterSuggestions);
BASE_DECLARE_FEATURE(kPreserveDefault);
BASE_DECLARE_FEATURE(kSingleSortAndCullPass);
Expand Down

0 comments on commit e6814e8

Please sign in to comment.