Commit d566a9fa authored by Kevin Bailey's avatar Kevin Bailey Committed by Commit Bot

[omnibox] Remove calls to EnsureUWYTIsAllowedToBeDefault

The suggestion providers HistoryQuick, Search and Shortcuts call
AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault() to promote
suggestions to have |allowed_to_be_default_match| set if the
stripped URL matches the input. This was done to promote some
higher scoring suggestions to become the default match. This is
now done during sort-and-dedup by having an |allowed| suggestion
inherit the best score that it matches. This CL removes the
calls and therefore doesn't set |allowed| in these matches.

Bug: 879796
Change-Id: I9529e3bea015e6e04f44ed5b401344d73556665b
Reviewed-on: https://chromium-review.googlesource.com/1213281
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589935}
parent d85fd54c
...@@ -2381,12 +2381,6 @@ TEST_F(SearchProviderTest, DefaultProviderSuggestRelevanceScoringUrlInput) { ...@@ -2381,12 +2381,6 @@ TEST_F(SearchProviderTest, DefaultProviderSuggestRelevanceScoringUrlInput) {
{ { "a.com/a", AutocompleteMatchType::NAVSUGGEST, true }, { { "a.com/a", AutocompleteMatchType::NAVSUGGEST, true },
{ "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true }, { "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
kEmptyMatch, kEmptyMatch } }, kEmptyMatch, kEmptyMatch } },
{ "a.com", "[\"a.com\",[\"https://a.com\"],[],[],"
"{\"google:suggesttype\":[\"NAVIGATION\"],"
"\"google:suggestrelevance\":[9999]}]",
{ { "a.com", AutocompleteMatchType::NAVSUGGEST, true },
{ "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
kEmptyMatch, kEmptyMatch } },
// Ensure topmost inlineable SUGGEST matches are NOT allowed for URL // Ensure topmost inlineable SUGGEST matches are NOT allowed for URL
// input. SearchProvider disregards search and verbatim suggested // input. SearchProvider disregards search and verbatim suggested
...@@ -2633,28 +2627,15 @@ TEST_F(SearchProviderTest, NavigationInline) { ...@@ -2633,28 +2627,15 @@ TEST_F(SearchProviderTest, NavigationInline) {
// will be allowed to be default. // will be allowed to be default.
{ "abc.com", "http://www.abc.com", { "abc.com", "http://www.abc.com",
"www.abc.com", std::string(), true, true }, "www.abc.com", std::string(), true, true },
{ "abc.com/", "http://www.abc.com",
"www.abc.com", std::string(), true, true },
{ "http://www.abc.com", "http://www.abc.com", { "http://www.abc.com", "http://www.abc.com",
"http://www.abc.com", std::string(), true, true }, "http://www.abc.com", std::string(), true, true },
{ "http://www.abc.com/", "http://www.abc.com",
"http://www.abc.com", std::string(), true, true },
// Inputs with trailing whitespace should inline when possible. // Inputs with trailing whitespace should inline when possible.
{ "abc.com ", "http://www.abc.com", { "abc.com ", "http://www.abc.com",
"www.abc.com", std::string(), true, true }, "www.abc.com", std::string(), true, true },
{ "abc.com/ ", "http://www.abc.com",
"www.abc.com", std::string(), true, true },
{ "abc.com ", "http://www.abc.com/bar", { "abc.com ", "http://www.abc.com/bar",
"www.abc.com/bar", "/bar", false, false }, "www.abc.com/bar", "/bar", false, false },
// A suggestion that's equivalent to what the input gets fixed up to
// should be inlined.
{ "abc.com:", "http://abc.com/",
"abc.com", std::string(), true, true },
{ "abc.com:", "http://www.abc.com",
"www.abc.com", std::string(), true, true },
// Inline matches when the input is a leading substring of the scheme. // Inline matches when the input is a leading substring of the scheme.
{ "h", "http://www.abc.com", { "h", "http://www.abc.com",
"http://www.abc.com", "ttp://www.abc.com", true, false }, "http://www.abc.com", "ttp://www.abc.com", true, false },
......
...@@ -647,19 +647,6 @@ void AutocompleteMatch::ComputeStrippedDestinationURL( ...@@ -647,19 +647,6 @@ void AutocompleteMatch::ComputeStrippedDestinationURL(
destination_url, input, template_url_service, keyword); destination_url, input, template_url_service, keyword);
} }
void AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault(
const AutocompleteInput& input,
TemplateURLService* template_url_service) {
if (!allowed_to_be_default_match) {
const GURL& stripped_canonical_input_url =
GURLToStrippedGURL(input.canonicalized_url(), input,
template_url_service, base::string16());
ComputeStrippedDestinationURL(input, template_url_service);
allowed_to_be_default_match =
stripped_canonical_input_url == stripped_destination_url;
}
}
void AutocompleteMatch::GetKeywordUIState( void AutocompleteMatch::GetKeywordUIState(
TemplateURLService* template_url_service, TemplateURLService* template_url_service,
base::string16* keyword, base::string16* keyword,
......
...@@ -265,13 +265,6 @@ struct AutocompleteMatch { ...@@ -265,13 +265,6 @@ struct AutocompleteMatch {
void ComputeStrippedDestinationURL(const AutocompleteInput& input, void ComputeStrippedDestinationURL(const AutocompleteInput& input,
TemplateURLService* template_url_service); TemplateURLService* template_url_service);
// Sets |allowed_to_be_default_match| to true if this match is effectively
// the URL-what-you-typed match (i.e., would be dupped against the UWYT
// match when AutocompleteResult merges matches).
void EnsureUWYTIsAllowedToBeDefault(
const AutocompleteInput& input,
TemplateURLService* template_url_service);
// Gets data relevant to whether there should be any special keyword-related // Gets data relevant to whether there should be any special keyword-related
// UI shown for this match. If this match represents a selected keyword, i.e. // UI shown for this match. If this match represents a selected keyword, i.e.
// the UI should be "in keyword mode", |keyword| will be set to the keyword // the UI should be "in keyword mode", |keyword| will be set to the keyword
......
...@@ -236,8 +236,6 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch( ...@@ -236,8 +236,6 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
match.allowed_to_be_default_match = match.inline_autocompletion.empty() || match.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
!PreventInlineAutocomplete(autocomplete_input_); !PreventInlineAutocomplete(autocomplete_input_);
} }
match.EnsureUWYTIsAllowedToBeDefault(autocomplete_input_,
client()->GetTemplateURLService());
// The term match offsets should be adjusted based on the formatting // The term match offsets should be adjusted based on the formatting
// applied to the suggestion contents displayed in the dropdown. // applied to the suggestion contents displayed in the dropdown.
......
...@@ -1508,8 +1508,6 @@ AutocompleteMatch SearchProvider::NavigationToMatch( ...@@ -1508,8 +1508,6 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
!navigation.received_after_last_keystroke() && !navigation.received_after_last_keystroke() &&
(match.inline_autocompletion.empty() || (match.inline_autocompletion.empty() ||
(!input_.prevent_inline_autocomplete() && !trimmed_whitespace)); (!input_.prevent_inline_autocomplete() && !trimmed_whitespace));
match.EnsureUWYTIsAllowedToBeDefault(input_,
client()->GetTemplateURLService());
match.contents = navigation.match_contents(); match.contents = navigation.match_contents();
match.contents_class = navigation.match_contents_class(); match.contents_class = navigation.match_contents_class();
......
...@@ -424,8 +424,6 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( ...@@ -424,8 +424,6 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
} }
} }
match.EnsureUWYTIsAllowedToBeDefault(input, client_->GetTemplateURLService());
// Try to mark pieces of the contents and description as matches if they // Try to mark pieces of the contents and description as matches if they
// appear in |input.text()|. // appear in |input.text()|.
if (!terms_map.empty()) { if (!terms_map.empty()) {
......
...@@ -368,15 +368,6 @@ TEST_F(ShortcutsProviderTest, TrickySingleMatch) { ...@@ -368,15 +368,6 @@ TEST_F(ShortcutsProviderTest, TrickySingleMatch) {
RunShortcutsProviderTest(provider_, text, true, expected_urls, expected_url, RunShortcutsProviderTest(provider_, text, true, expected_urls, expected_url,
ASCIIToUTF16("ace/long-url-with-space.html")); ASCIIToUTF16("ace/long-url-with-space.html"));
// Test when the user input has a trailing slash but fill_into_edit does
// not. This should still be allowed to be default.
text = ASCIIToUTF16("notrailing.com/");
expected_url = "http://notrailing.com/";
expected_urls.clear();
expected_urls.push_back(ExpectedURLAndAllowedToBeDefault(expected_url, true));
RunShortcutsProviderTest(provider_, text, true, expected_urls, expected_url,
base::string16());
// Test when the user input has a typo that can be fixed up for matching // Test when the user input has a typo that can be fixed up for matching
// fill_into_edit. This should still be allowed to be default. // fill_into_edit. This should still be allowed to be default.
text = ASCIIToUTF16("http:///foo.com"); text = ASCIIToUTF16("http:///foo.com");
......
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