Skip to content

GitLab

T
tangled
Commit 286b10c4 authored by Orin Jaworski's avatar Orin Jaworski Committed by Commit Bot
Browse files
Options

[omnibox] Eliminate match subrelevance and parent_type 

This CL removes submatch fields from AutocompleteMatch and fixes
most related logic. Tab switch and Pedal suggestions are not added
to the suggestions list anymore since doing so would make them seem
like full suggestions.

Bug: 1102619
Change-Id: I44c8cbd29220fc520b3b7ebc28140888bd3b4cbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293315Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788442}
parent 80a0f0e1
Hide whitespace changes
Inline Side-by-side
Showing with 21 additions and 301 deletions
chrome/browser/ui/views/omnibox/omnibox_result_view.cc
View file @ 286b10c4
......@@ -362,10 +362,7 @@ void OmniboxResultView::Layout() {
}
const int suggestion_indent =
(popup_contents_view_->InExplicitExperimentalKeywordMode() ||
match_.IsSubMatch())
? 70
: 0;
popup_contents_view_->InExplicitExperimentalKeywordMode() ? 70 : 0;
const int suggestion_height = suggestion_view_->GetPreferredSize().height();
suggestion_view_->SetBounds(suggestion_indent, 0,
suggestion_width - suggestion_indent,
......
components/omnibox/browser/autocomplete_controller.cc
View file @ 286b10c4
......@@ -641,11 +641,7 @@ void AutocompleteController::UpdateResult(
result_.ConvertOpenTabMatches(provider_client_.get(), &input_);
if (OmniboxFieldTrial::IsPedalSuggestionsEnabled()) {
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
result_.ConvertInSuggestionPedalMatches(provider_client_.get());
} else {
result_.AppendDedicatedPedalMatches(provider_client_.get(), input_);
}
result_.ConvertInSuggestionPedalMatches(provider_client_.get());
}
// Sort the matches and trim to a small number of "best" matches.
......
components/omnibox/browser/autocomplete_match.cc
View file @ 286b10c4
......@@ -106,9 +106,6 @@ const base::char16 AutocompleteMatch::kInvalidChars[] = {
// static
const char AutocompleteMatch::kEllipsis[] = "... ";
// static
size_t AutocompleteMatch::next_family_id_;
AutocompleteMatch::AutocompleteMatch()
: transition(ui::PAGE_TRANSITION_GENERATED) {}
......@@ -125,7 +122,6 @@ AutocompleteMatch::AutocompleteMatch(AutocompleteProvider* provider,
AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match)
: provider(match.provider),
relevance(match.relevance),
subrelevance(match.subrelevance),
typed_count(match.typed_count),
deletable(match.deletable),
fill_into_edit(match.fill_into_edit),
......@@ -151,7 +147,6 @@ AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match)
answer(match.answer),
transition(match.transition),
type(match.type),
parent_type(match.parent_type),
has_tab_match(match.has_tab_match),
subtypes(match.subtypes),
associated_keyword(match.associated_keyword
......@@ -185,7 +180,6 @@ AutocompleteMatch& AutocompleteMatch::operator=(
provider = match.provider;
relevance = match.relevance;
subrelevance = match.subrelevance;
typed_count = match.typed_count;
deletable = match.deletable;
fill_into_edit = match.fill_into_edit;
......@@ -211,7 +205,6 @@ AutocompleteMatch& AutocompleteMatch::operator=(
answer = match.answer;
transition = match.transition;
type = match.type;
parent_type = match.parent_type;
has_tab_match = match.has_tab_match;
subtypes = match.subtypes;
associated_keyword.reset(
......@@ -643,13 +636,6 @@ bool AutocompleteMatch::ShouldBeSkippedForGroupBySearchVsUrl(Type type) {
type == AutocompleteMatchType::TILE_SUGGESTION;
}
AutocompleteMatch::Type AutocompleteMatch::GetDemotionType() const {
if (!IsSubMatch())
return type;
else
return parent_type;
}
// static
TemplateURL* AutocompleteMatch::GetTemplateURLWithKeyword(
TemplateURLService* template_url_service,
......@@ -825,30 +811,6 @@ void AutocompleteMatch::LogSearchEngineUsed(
}
}
// static
size_t AutocompleteMatch::GetNextFamilyID() {
next_family_id_ += FAMILY_SIZE;
// Avoid the default value. 0 means "no submatch", so no family can use it.
if (next_family_id_ == 0)
next_family_id_ += FAMILY_SIZE;
return next_family_id_;
}
// static
bool AutocompleteMatch::IsSameFamily(size_t lhs, size_t rhs) {
return (lhs & FAMILY_SIZE_MASK) == (rhs & FAMILY_SIZE_MASK);
}
void AutocompleteMatch::SetSubMatch(size_t subrelevance,
AutocompleteMatch::Type parent_type) {
this->subrelevance = subrelevance;
this->parent_type = parent_type;
}
bool AutocompleteMatch::IsSubMatch() const {
return subrelevance & ~FAMILY_SIZE_MASK;
}
void AutocompleteMatch::ComputeStrippedDestinationURL(
const AutocompleteInput& input,
TemplateURLService* template_url_service) {
......@@ -899,37 +861,6 @@ GURL AutocompleteMatch::ImageUrl() const {
return answer ? answer->image_url() : image_url;
}
AutocompleteMatch AutocompleteMatch::DerivePedalSuggestion(
OmniboxPedal* pedal) {
AutocompleteMatch copy(*this);
copy.pedal = pedal;
if (subrelevance == 0)
subrelevance = GetNextFamilyID();
copy.SetSubMatch(subrelevance + PEDAL_FAMILY_ID, copy.type);
DCHECK(IsSameFamily(subrelevance, copy.subrelevance));
copy.type = Type::PEDAL;
copy.destination_url = copy.pedal->GetNavigationUrl();
// Normally this is computed by the match using a TemplateURLService
// but Pedal URLs are not typical and unknown, and we don't want them to
// be deduped, e.g. after stripping a query parameter that may do something
// meaningful like indicate the viewable scope of a settings page. So here
// we keep the URL exactly as the Pedal specifies it.
copy.stripped_destination_url = copy.destination_url;
// Note: Always use empty classifications for empty text and non-empty
// classifications for non-empty text.
const auto& labels = copy.pedal->GetLabelStrings();
copy.contents = labels.suggestion_contents;
copy.contents_class = {ACMatchClassification(0, ACMatchClassification::NONE)};
copy.description = labels.hint;
copy.description_class = {
ACMatchClassification(0, ACMatchClassification::NONE)};
return copy;
}
void AutocompleteMatch::RecordAdditionalInfo(const std::string& property,
const std::string& value) {
DCHECK(!property.empty());
......@@ -1163,8 +1094,10 @@ bool AutocompleteMatch::ShouldShowTabMatchButtonInlineInResultView() const {
!OmniboxFieldTrial::IsSuggestionButtonRowEnabled();
}
// TODO(orinj): Dedicated tab switch suggestions are eliminated. Delete this
// method and clean up any remaining related logic.
bool AutocompleteMatch::IsTabSwitchSuggestion() const {
return (subrelevance & ~FAMILY_SIZE_MASK) == TAB_SWITCH_FAMILY_ID;
return false;
}
void AutocompleteMatch::UpgradeMatchWithPropertiesFrom(
......
components/omnibox/browser/autocomplete_match.h
View file @ 286b10c4
......@@ -119,15 +119,6 @@ struct AutocompleteMatch {
// and |description| strings.
static const base::char16 kInvalidChars[];
// All IDs should be less than the family size, and family size must be
// a power of 2 so that the counter wraps.
enum {
PEDAL_FAMILY_ID = 1,
TAB_SWITCH_FAMILY_ID = 2,
FAMILY_SIZE = 1 << 2,
};
static constexpr size_t FAMILY_SIZE_MASK = ~(FAMILY_SIZE - 1);
// Document subtype, for AutocompleteMatchType::DOCUMENT.
// Update kDocumentTypeStrings when updating DocumentType.
enum class DocumentType {
......@@ -250,10 +241,6 @@ struct AutocompleteMatch {
// clipboard or query tile.
static bool ShouldBeSkippedForGroupBySearchVsUrl(Type type);
// If this match is a submatch, returns the parent's type, otherwise this
// match's type.
Type GetDemotionType() const;
// A static version GetTemplateURL() that takes the match's keyword and
// match's hostname as parameters. In short, returns the TemplateURL
// associated with |keyword| if it exists; otherwise returns the TemplateURL
......@@ -315,17 +302,6 @@ struct AutocompleteMatch {
static void LogSearchEngineUsed(const AutocompleteMatch& match,
TemplateURLService* template_url_service);
// There are some suggestions that we would like to follow each other
// e.g. pedals, tab switches, possibly keyword provider suggestions.
// These functions provide and compare integer groups so that when we
// generate these suggestions, they can be given integers in the same
// family, which will cause them to be sorted together.
static size_t GetNextFamilyID();
static bool IsSameFamily(size_t lhs, size_t rhs);
// Preferred method to set both fields simultaneously.
void SetSubMatch(size_t subrelevance, AutocompleteMatch::Type parent_type);
bool IsSubMatch() const;
// Computes the stripped destination URL (via GURLToStrippedGURL()) and
// stores the result in |stripped_destination_url|. |input| is used for the
// same purpose as in GURLToStrippedGURL().
......@@ -370,11 +346,6 @@ struct AutocompleteMatch {
// there isn't an image URL, returns an empty GURL (test with is_empty()).
GURL ImageUrl() const;
// Returns a new Pedal match suggestion instance derived from this match,
// which is considered to be the triggering suggestion. The new match
// will be set to use the given |pedal|.
AutocompleteMatch DerivePedalSuggestion(OmniboxPedal* pedal);
// Adds optional information to the |additional_info| dictionary.
void RecordAdditionalInfo(const std::string& property,
const std::string& value);
......@@ -500,11 +471,6 @@ struct AutocompleteMatch {
// rather than being a fairly fixed value defined by the table above.
int relevance = 0;
// This represents the numeric family that the match is part of. It is only
// set for certain paired suggestions. Suggestions within the same group will
// have similar subrelevances, and they will sort together.
size_t subrelevance = 0;
// How many times this result was typed in / selected from the omnibox.
// Only set for some providers and result_types. If it is not set,
// its value is -1. At the time of writing this comment, it is only
......@@ -609,9 +575,6 @@ struct AutocompleteMatch {
// Type of this match.
Type type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
// If a submatch, the type of the parent.
Type parent_type;
// True if we saw a tab that matched this suggestion.
bool has_tab_match = false;
......@@ -690,11 +653,6 @@ struct AutocompleteMatch {
// So users of AutocompleteMatch can use the same ellipsis that it uses.
static const char kEllipsis[];
// A numeric quantity that only increases by the amount FAMILY_SIZE.
// It helps guarantee that a match family will have similar, but unique,
// numeric values.
static size_t next_family_id_;
#if DCHECK_IS_ON()
// Does a data integrity check on this match.
void Validate() const;
......
components/omnibox/browser/autocomplete_result.cc
View file @ 286b10c4
......@@ -42,20 +42,13 @@ typedef AutocompleteMatchType ACMatchType;
namespace {
// Rotates |it| and its associated submatches to be in the front of |matches|.
// Rotates |it| to be in the front of |matches|.
// |it| must be a valid iterator of |matches| or equal to |matches->end()|.
void RotateMatchToFront(ACMatches::iterator it, ACMatches* matches) {
if (it == matches->end())
return;
const size_t cookie = it->subrelevance;
auto next = std::next(it);
if (cookie != 0) {
// If default match followed by sub-match(es), move them too.
while (next != matches->end() &&
AutocompleteMatch::IsSameFamily(cookie, next->subrelevance))
next = std::next(next);
}
std::rotate(matches->begin(), it, next);
}
......@@ -285,15 +278,11 @@ void AutocompleteResult::SortAndCull(
#endif
// Skip over default match.
auto next = std::next(matches_.begin());
// If it has submatches, skip them too.
if (matches_.front().subrelevance != 0 ||
AutocompleteMatch::ShouldBeSkippedForGroupBySearchVsUrl(
if (AutocompleteMatch::ShouldBeSkippedForGroupBySearchVsUrl(
matches_.front().type)) {
while (next != matches_.end() &&
(AutocompleteMatch::IsSameFamily(matches_.front().subrelevance,
next->subrelevance) ||
AutocompleteMatch::ShouldBeSkippedForGroupBySearchVsUrl(
matches_.front().type))) {
(AutocompleteMatch::ShouldBeSkippedForGroupBySearchVsUrl(
matches_.front().type))) {
next = std::next(next);
}
}
......@@ -400,26 +389,6 @@ void AutocompleteResult::DemoteOnDeviceSearchSuggestions() {
}
}
void AutocompleteResult::AppendDedicatedPedalMatches(
AutocompleteProviderClient* client,
const AutocompleteInput& input) {
const OmniboxPedalProvider* provider = client->GetPedalProvider();
ACMatches pedal_suggestions;
for (auto& match : matches_) {
// We do not want to deal with pedals of pedals, or pedals among
// exclusive tail suggestions.
if (match.pedal || match.type == ACMatchType::SEARCH_SUGGEST_TAIL)
continue;
OmniboxPedal* const pedal = provider->FindPedalMatch(match.contents);
if (pedal)
pedal_suggestions.push_back(match.DerivePedalSuggestion(pedal));
}
if (!pedal_suggestions.empty()) {
AppendMatches(input, pedal_suggestions);
}
}
void AutocompleteResult::ConvertInSuggestionPedalMatches(
AutocompleteProviderClient* client) {
const OmniboxPedalProvider* provider = client->GetPedalProvider();
......@@ -452,32 +421,16 @@ void AutocompleteResult::ConvertInSuggestionPedalMatches(
void AutocompleteResult::ConvertOpenTabMatches(
AutocompleteProviderClient* client,
const AutocompleteInput* input) {
ACMatches matches_to_add;
for (auto& match : matches_) {
// If already converted this match, don't re-search through open tabs and
// possibly re-change the description. Also skip submatches.
if (match.has_tab_match || match.IsSubMatch())
// possibly re-change the description.
if (match.has_tab_match)
continue;
// If URL is in a tab, remember that.
if (client->IsTabOpenWithURL(match.destination_url, input)) {
match.has_tab_match = true;
// If will have dedicated row, add a match for it.
if (OmniboxFieldTrial::IsTabSwitchSuggestionsDedicatedRowEnabled()) {
if (match.subrelevance == 0)
match.subrelevance = AutocompleteMatch::GetNextFamilyID();
AutocompleteMatch tab_switch_match = match;
tab_switch_match.SetSubMatch(
match.subrelevance + AutocompleteMatch::TAB_SWITCH_FAMILY_ID,
match.type);
tab_switch_match.contents =
l10n_util::GetStringUTF16(IDS_OMNIBOX_TAB_SUGGEST_HINT);
tab_switch_match.contents_class = {{0, ACMatchClassification::NONE}};
matches_to_add.push_back(tab_switch_match);
}
}
}
std::copy(matches_to_add.begin(), matches_to_add.end(),
std::back_inserter(matches_));
}
bool AutocompleteResult::HasCopiedMatches() const {
......@@ -571,7 +524,7 @@ ACMatches::iterator AutocompleteResult::FindTopMatch(
input.type() == metrics::OmniboxInputType::URL) {
auto best = matches->end();
for (auto it = matches->begin(); it != matches->end(); ++it) {
if (it->allowed_to_be_default_match && !it->IsSubMatch() &&
if (it->allowed_to_be_default_match &&
(best == matches->end() ||
AutocompleteMatch::MoreRelevant(*it, *best))) {
best = it;
......@@ -580,7 +533,7 @@ ACMatches::iterator AutocompleteResult::FindTopMatch(
return best;
} else {
return std::find_if(matches->begin(), matches->end(), [](const auto& m) {
return m.allowed_to_be_default_match && !m.IsSubMatch();
return m.allowed_to_be_default_match;
});
}
}
......@@ -972,9 +925,7 @@ void AutocompleteResult::MergeMatchesByProvider(ACMatches* old_matches,
std::pair<GURL, bool> AutocompleteResult::GetMatchComparisonFields(
const AutocompleteMatch& match) {
return std::make_pair(match.stripped_destination_url,
match.type == ACMatchType::CALCULATOR ||
// Separate sub-matches from their origins.
match.IsSubMatch());
match.type == ACMatchType::CALCULATOR);
}
void AutocompleteResult::LimitNumberOfURLsShown(
......@@ -983,7 +934,7 @@ void AutocompleteResult::LimitNumberOfURLsShown(
const CompareWithDemoteByType<AutocompleteMatch>& comparing_object) {
size_t search_count = std::count_if(
matches_.begin(), matches_.end(), [&](const AutocompleteMatch& m) {
return !m.IsSubMatch() && AutocompleteMatch::IsSearchType(m.type) &&
return AutocompleteMatch::IsSearchType(m.type) &&
// Don't count if would be removed.
comparing_object.GetDemotedRelevance(m) > 0;
});
......@@ -997,16 +948,9 @@ void AutocompleteResult::LimitNumberOfURLsShown(
matches_.erase(
std::remove_if(matches_.begin(), matches_.end(),
[&url_count, max_url_count](const AutocompleteMatch& m) {
if (!m.IsSubMatch() &&
!AutocompleteMatch::IsSearchType(m.type) &&
if (!AutocompleteMatch::IsSearchType(m.type) &&
++url_count > max_url_count)
return true;
// Do not count submatches towards URL total, but
// drop them if parent was dropped.
if (m.IsSubMatch() &&
!AutocompleteMatch::IsSearchType(m.parent_type) &&
url_count > max_url_count)
return true;
return false;
}),
matches_.end());
......@@ -1016,6 +960,6 @@ void AutocompleteResult::LimitNumberOfURLsShown(
void AutocompleteResult::GroupSuggestionsBySearchVsURL(iterator begin,
iterator end) {
std::stable_partition(begin, end, [](const AutocompleteMatch& match) {
return match.IsSearchType(match.GetDemotionType());
return match.IsSearchType(match.type);
});
}
components/omnibox/browser/autocomplete_result.h
View file @ 286b10c4
......@@ -70,12 +70,6 @@ class AutocompleteResult {
TemplateURLService* template_url_service,
const AutocompleteMatch* preserve_default_match = nullptr);
// Creates and adds any dedicated Pedal matches triggered by existing matches.
// This should be the only place where new Pedal suggestions are introduced
// because it doesn't dedupe; it just carefully avoids adding duplicates.
void AppendDedicatedPedalMatches(AutocompleteProviderClient* client,
const AutocompleteInput& input);
// Sets |pedal| in matches that have Pedal-triggering text.
void ConvertInSuggestionPedalMatches(AutocompleteProviderClient* client);
......
components/omnibox/browser/autocomplete_result_unittest.cc
View file @ 286b10c4
......@@ -1754,103 +1754,6 @@ TEST_F(AutocompleteResultTest, DocumentSuggestionsCanMergeButNotToDefault) {
EXPECT_FALSE(result.match_at(2)->allowed_to_be_default_match);
}
TEST_F(AutocompleteResultTest, PedalSuggestionsCantBeDefaultMatch) {
TestData data[] = {
{1, 1, 500, true},
{0, 1, 1100, true},
};
ACMatches matches;
PopulateAutocompleteMatches(data, base::size(data), &matches);
matches[0].contents = base::UTF8ToUTF16("clear chrome history");
matches[1].contents = base::UTF8ToUTF16("open incognito tab");
AutocompleteInput input(base::ASCIIToUTF16("a"),
metrics::OmniboxEventProto::OTHER,
TestSchemeClassifier());
AutocompleteResult result;
result.AppendMatches(input, matches);
FakeAutocompleteProviderClient client;
result.AppendDedicatedPedalMatches(&client, input);
// Two distinct Pedals should be appended.
EXPECT_EQ(result.size(), 4u);
EXPECT_NE(result.match_at(2)->pedal, nullptr);
EXPECT_NE(result.match_at(3)->pedal, nullptr);
// Neither should be the default match, even though they were both
// derived from suggestions where the field is set true.
EXPECT_TRUE(result.match_at(0)->allowed_to_be_default_match);
EXPECT_EQ(result.match_at(0)->pedal, nullptr);
EXPECT_TRUE(result.match_at(1)->allowed_to_be_default_match);
EXPECT_EQ(result.match_at(1)->pedal, nullptr);
EXPECT_TRUE(result.match_at(2)->allowed_to_be_default_match);
EXPECT_NE(result.match_at(2)->pedal, nullptr);
EXPECT_TRUE(result.match_at(3)->allowed_to_be_default_match);
EXPECT_NE(result.match_at(3)->pedal, nullptr);
}
TEST_F(AutocompleteResultTest, PedalSuggestionsRemainUnique) {
TestData data[] = {
{1, 1, 500, false},
{0, 1, 1100, false},
{2, 1, 1000, true},
{0, 1, 1200, false},
};
ACMatches matches;
PopulateAutocompleteMatches(data, base::size(data), &matches);
matches[0].contents = base::UTF8ToUTF16("clear chrome history");
matches[1].contents = base::UTF8ToUTF16("open incognito tab");
matches[2].contents = base::UTF8ToUTF16("clear chrome history");
AutocompleteInput input(base::ASCIIToUTF16("a"),
metrics::OmniboxEventProto::OTHER,
TestSchemeClassifier());
AutocompleteResult result;
result.AppendMatches(input, matches);
FakeAutocompleteProviderClient client;
result.AppendDedicatedPedalMatches(&client, input);
result.DeduplicateMatches(&result.matches_);
// Exactly 2 (not 3) unique Pedals should be added with relevance close to max
// of the triggering suggestions.
EXPECT_EQ(result.size(), 6u);
EXPECT_NE(result.match_at(4)->pedal, nullptr);
EXPECT_NE(result.match_at(5)->pedal, nullptr);
EXPECT_NE(result.match_at(4)->pedal, result.match_at(5)->pedal);
EXPECT_EQ(result.match_at(4)->relevance, 1100);
EXPECT_EQ(result.match_at(5)->relevance, 1000);
// Now artificially modify existing suggestions and run again to ensure that
// no duplicates are added, but the existing Pedal suggestion is updated.
result.match_at(3)->contents = base::UTF8ToUTF16("open incognito tab");
result.AppendDedicatedPedalMatches(&client, input);
result.DeduplicateMatches(&result.matches_);
EXPECT_EQ(result.size(), 6u);
EXPECT_NE(result.match_at(4)->pedal, nullptr);
EXPECT_NE(result.match_at(5)->pedal, nullptr);
EXPECT_NE(result.match_at(4)->pedal, result.match_at(5)->pedal);
EXPECT_EQ(result.match_at(5)->relevance, 1200);
// Finally run a real final sort to make sure default match and its
// pedal get promoted.
result.SortAndCull(input, template_url_service_.get());
EXPECT_EQ(result.size(), 5u);
EXPECT_EQ(result.match_at(0)->relevance, 1000);
EXPECT_EQ(result.match_at(0)->pedal, nullptr);
EXPECT_EQ(result.match_at(1)->relevance, 1000);
EXPECT_NE(result.match_at(1)->pedal, nullptr);
EXPECT_EQ(result.match_at(2)->relevance, 1200);
EXPECT_EQ(result.match_at(2)->pedal, nullptr);
EXPECT_EQ(result.match_at(3)->relevance, 1200);
EXPECT_NE(result.match_at(3)->pedal, nullptr);
EXPECT_EQ(result.match_at(4)->relevance, 500);
EXPECT_EQ(result.match_at(4)->pedal, nullptr);
}
TEST_F(AutocompleteResultTest, CalculateNumMatchesPerUrlCountTest) {
CompareWithDemoteByType<AutocompleteMatch> comparison_object(
metrics::OmniboxEventProto::OTHER);
......
components/omnibox/browser/clipboard_provider_unittest.cc
View file @ 286b10c4
......@@ -127,7 +127,7 @@ TEST_F(ClipboardProviderTest, MatchesUrl) {
ASSERT_GE(provider_->matches().size(), 1U);
EXPECT_EQ(GURL(kClipboardURL), provider_->matches().back().destination_url);
EXPECT_EQ(AutocompleteMatchType::CLIPBOARD_URL,
provider_->matches().back().GetDemotionType());
provider_->matches().back().type);
}
TEST_F(ClipboardProviderTest, MatchesText) {
......@@ -142,7 +142,7 @@ TEST_F(ClipboardProviderTest, MatchesText) {
EXPECT_EQ(base::UTF8ToUTF16(kClipboardText),
provider_->matches().back().fill_into_edit);
EXPECT_EQ(AutocompleteMatchType::CLIPBOARD_TEXT,
provider_->matches().back().GetDemotionType());
provider_->matches().back().type);
}
TEST_F(ClipboardProviderTest, MatchesImage) {
......@@ -163,7 +163,7 @@ TEST_F(ClipboardProviderTest, MatchesImage) {
image_bytes);
ASSERT_GE(provider_->matches().size(), 1U);
EXPECT_EQ(AutocompleteMatchType::CLIPBOARD_IMAGE,
provider_->matches().back().GetDemotionType());
provider_->matches().back().type);
}
TEST_F(ClipboardProviderTest, DeleteMatch) {
......
components/omnibox/browser/match_compare.h
View file @ 286b10c4
......@@ -22,7 +22,7 @@ class CompareWithDemoteByType {
// Returns the relevance score of |match| demoted appropriately by
// |demotions_by_type_|.
int GetDemotedRelevance(const Match& match) const {
auto demotion_it = demotions_.find(match.GetDemotionType());
auto demotion_it = demotions_.find(match.type);
return (demotion_it == demotions_.end())
? match.relevance
: (match.relevance * demotion_it->second);
......@@ -37,11 +37,6 @@ class CompareWithDemoteByType {
// Greater relevance should come first.
return demoted_relevance1 > demoted_relevance2;
}
// "Paired" suggestions should follow each other, lower first.
// Even if subrelevances don't match, we must compare them to maintain
// ordering.
if (elem1.subrelevance != elem2.subrelevance)
return elem1.subrelevance < elem2.subrelevance;
// For equal-relevance matches, we sort alphabetically, so that providers
// who return multiple elements at the same priority get a "stable" sort
// across multiple updates.
......
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