diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 3b786d6e1204ae..167dbb0c1b752c 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -4114,7 +4114,9 @@ TEST_F(SearchProviderTest, DuplicateCardAnswer) { AutocompleteMatch match1, match2, match3; match1.contents = u"match 1"; match1.type = AutocompleteMatchType::SEARCH_SUGGEST; + match1.allowed_to_be_default_match = true; match1.answer_template = omnibox::RichAnswerTemplate(); + match1.destination_url = GURL("http://www.google.com/google.com/search?"); matches.push_back(match1); matches.push_back(match2); @@ -4124,7 +4126,10 @@ TEST_F(SearchProviderTest, DuplicateCardAnswer) { EXPECT_EQ(4u, matches.size()); EXPECT_TRUE(matches[0].answer_template); + EXPECT_FALSE(matches[0].allowed_to_be_default_match); EXPECT_FALSE(matches[3].answer_template); + EXPECT_TRUE(matches[3].allowed_to_be_default_match); + EXPECT_EQ(matches[3].suggestion_group_id, omnibox::GROUP_SEARCH); EXPECT_EQ(matches[0].contents, matches[3].contents); EXPECT_EQ(matches[0].type, matches[3].type); } diff --git a/components/omnibox/browser/autocomplete_match.cc b/components/omnibox/browser/autocomplete_match.cc index eaa82c6d4bb5bb..9d8910a3ce62ab 100644 --- a/components/omnibox/browser/autocomplete_match.cc +++ b/components/omnibox/browser/autocomplete_match.cc @@ -183,17 +183,22 @@ inline void hash_combine(std::size_t& seed, const T& value) { } // namespace -template -size_t ACMatchKeyHash::operator()(const ACMatchKey& key) const { +template +size_t ACMatchKeyHash::operator()( + const ACMatchKey& key) const { size_t seed = 0; - hash_combine(seed, key.first); - hash_combine(seed, key.second); + // Compute a hash by applying hash_combine to each element of the "key" tuple. + std::apply([&seed](auto&&... args) { ((hash_combine(seed, args)), ...); }, + key); return seed; } // This trick allows implementing ACMatchKeyHash in the implementation file. -template struct ACMatchKeyHash; -template struct ACMatchKeyHash; +// Every unique specialization of ACMatchKey should have a corresponding +// declaration here. +template struct ACMatchKeyHash; // base_search_provider +template struct ACMatchKeyHash; // autocomplete_result // RichAutocompletionParams --------------------------------------------------- @@ -1448,6 +1453,10 @@ int AutocompleteMatch::GetSortingOrder() const { return 3; } #endif // !BUILDFLAG(IS_IOS) + if (answer_template && actions.size() > 0 && + OmniboxFieldTrial::kAnswerActionsShowAboveKeyboard.Get()) { + return 4; + } if (IsSearchType(type)) { return 3; } diff --git a/components/omnibox/browser/autocomplete_match.h b/components/omnibox/browser/autocomplete_match.h index e960c73f27aa22..a7e6c6230c67b9 100644 --- a/components/omnibox/browser/autocomplete_match.h +++ b/components/omnibox/browser/autocomplete_match.h @@ -979,14 +979,14 @@ typedef std::vector ACMatchClassifications; typedef std::vector ACMatches; // Can be used as the key for grouping AutocompleteMatches in a map based on a -// std::pair of fields. This can be generalized to a std::tuple if ever needed. +// std::tuple of fields. // The accompanying hash function makes the key usable in an std::unordered_map. -template -using ACMatchKey = std::pair; +template +using ACMatchKey = std::tuple; -template +template struct ACMatchKeyHash { - size_t operator()(const ACMatchKey& key) const; + size_t operator()(const ACMatchKey& key) const; }; #endif // COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_MATCH_H_ diff --git a/components/omnibox/browser/autocomplete_result.cc b/components/omnibox/browser/autocomplete_result.cc index 888a76d244c821..20ba520406ee57 100644 --- a/components/omnibox/browser/autocomplete_result.cc +++ b/components/omnibox/browser/autocomplete_result.cc @@ -1154,7 +1154,7 @@ void AutocompleteResult::DeduplicateMatches( // Group matches by stripped URL and whether it's a calculator suggestion. std::unordered_map, - ACMatchKeyHash> + ACMatchKeyHash> url_to_matches; for (auto i = matches->begin(); i != matches->end(); ++i) { url_to_matches[GetMatchComparisonFields(*i)].push_back(i); @@ -1166,8 +1166,9 @@ void AutocompleteResult::DeduplicateMatches( // The vector of matches whose URL are equivalent. std::vector& duplicate_matches = group.second; - if (key.first.empty() || duplicate_matches.size() == 1) + if (std::get<0>(key).empty() || duplicate_matches.size() == 1) { continue; + } // Sort the matches best to worst, according to the deduplication criteria. std::sort(duplicate_matches.begin(), duplicate_matches.end(), @@ -1460,8 +1461,11 @@ void AutocompleteResult::MergeMatchesByProvider(ACMatches* old_matches, AutocompleteResult::MatchDedupComparator AutocompleteResult::GetMatchComparisonFields(const AutocompleteMatch& match) { - return std::make_pair(match.stripped_destination_url.spec(), - match.type == ACMatchType::CALCULATOR); + return std::make_tuple( + match.stripped_destination_url.spec(), + match.type == ACMatchType::CALCULATOR, + match.answer_template.has_value() && + OmniboxFieldTrial::kAnswerActionsShowAboveKeyboard.Get()); } void AutocompleteResult::LimitNumberOfURLsShown( diff --git a/components/omnibox/browser/autocomplete_result.h b/components/omnibox/browser/autocomplete_result.h index 4c319c3c34703f..10bce93416b39f 100644 --- a/components/omnibox/browser/autocomplete_result.h +++ b/components/omnibox/browser/autocomplete_result.h @@ -39,7 +39,7 @@ class AutocompleteResult { public: typedef ACMatches::const_iterator const_iterator; typedef ACMatches::iterator iterator; - using MatchDedupComparator = ACMatchKey; + using MatchDedupComparator = ACMatchKey; // Max number of matches we'll show from the various providers. This limit // may be different for zero suggest and non zero suggest. Does not take into @@ -413,11 +413,8 @@ class AutocompleteResult { void MergeMatchesByProvider(ACMatches* old_matches, const ACMatches& new_matches); - // This pulls the relevant fields out of a match for comparison with other - // matches for the purpose of deduping. It uses the stripped URL, so that we - // collapse similar URLs if necessary, and whether the match is a calculator - // suggestion, because we don't want to dedupe them against URLs that simply - // happen to go to the same destination. + // Returns a tuple encompassing all attributes relevant to determining whether a match should + // be deduplicated with another match. static MatchDedupComparator GetMatchComparisonFields( const AutocompleteMatch& match); diff --git a/components/omnibox/browser/base_search_provider.cc b/components/omnibox/browser/base_search_provider.cc index 5fe7042eb1a416..77ee9f8a32e1f5 100644 --- a/components/omnibox/browser/base_search_provider.cc +++ b/components/omnibox/browser/base_search_provider.cc @@ -544,8 +544,8 @@ void BaseSearchProvider::AddMatchToMap( // Try to add `match` to `map`. // NOTE: Keep this ToLower() call in sync with url_database.cc. MatchKey match_key( - std::make_pair(base::i18n::ToLower(result.suggestion()), - match.search_terms_args->additional_query_params)); + std::make_tuple(base::i18n::ToLower(result.suggestion()), + match.search_terms_args->additional_query_params)); const std::pair i( map->insert(std::make_pair(match_key, match))); if (i.second) { @@ -562,12 +562,12 @@ void BaseSearchProvider::AddMatchToMap( // Note that the match previously added to the map will continue to get the // typical `stripped_destination_url` allowing it to be deduped with the // plain-text matches (i.e., with no additional query params) as expected. - const auto& added_match_query = match_key.first; - const auto& added_match_query_params = match_key.second; + const auto& added_match_query = std::get<0>(match_key); + const auto& added_match_query_params = std::get<1>(match_key); if (!added_match_query_params.empty()) { for (const auto& entry : *map) { - const auto& existing_match_query = entry.first.first; - const auto& existing_match_query_params = entry.first.second; + const auto& existing_match_query = std::get<0>(entry.first); + const auto& existing_match_query_params = std::get<1>(entry.first); if (existing_match_query == added_match_query && !existing_match_query_params.empty() && existing_match_query_params != added_match_query_params) { diff --git a/components/omnibox/browser/search_provider.cc b/components/omnibox/browser/search_provider.cc index 79dc6a31fba8db..610bd6d74c07b2 100644 --- a/components/omnibox/browser/search_provider.cc +++ b/components/omnibox/browser/search_provider.cc @@ -1139,7 +1139,14 @@ void SearchProvider::DuplicateCardAnswer(ACMatches* matches) { return; } - matches->emplace_back(*iter).answer_template.reset(); + bool orig_allowed_to_be_default_match = iter->allowed_to_be_default_match; + iter->allowed_to_be_default_match = false; + + auto& copy = matches->emplace_back(*iter); + copy.answer_template.reset(); + copy.actions.clear(); + copy.allowed_to_be_default_match = orig_allowed_to_be_default_match; + copy.suggestion_group_id = omnibox::GROUP_SEARCH; } bool SearchProvider::IsTopMatchSearchWithURLInput() const {