Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(60)

Unified Diff: chrome/browser/autocomplete/search_provider.cc

Issue 18878007: Omnibox: Make the Controller Reorder Matches for Inlining (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: rebase Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 8806491259be83fad8b339e091b2501f62b3143e..536e8902fc0d12ea130dc48e1fc927bd12935e29 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -255,7 +255,9 @@ SearchProvider::SearchProvider(AutocompleteProviderListener* listener,
prevent_search_history_inlining_(
OmniboxFieldTrial::SearchHistoryPreventInlining()),
disable_search_history_(
- OmniboxFieldTrial::SearchHistoryDisable()) {
+ OmniboxFieldTrial::SearchHistoryDisable()),
+ omnibox_will_reorder_for_legal_default_match_(
+ OmniboxFieldTrial::InReorderForLegalDefaultMatchGroup()) {
}
// static
@@ -315,6 +317,7 @@ AutocompleteMatch SearchProvider::CreateSearchSuggestion(
// completion.
match.contents_class.push_back(
ACMatchClassification(0, ACMatchClassification::NONE));
+ match.allowed_to_be_default_match = true;
}
// When the user forced a query, we need to make sure all the fill_into_edit
@@ -327,6 +330,7 @@ AutocompleteMatch SearchProvider::CreateSearchSuggestion(
if (!input.prevent_inline_autocomplete() &&
StartsWith(query_string, input_text, false)) {
match.inline_autocompletion = query_string.substr(input_text.length());
+ match.allowed_to_be_default_match = true;
}
match.fill_into_edit.append(query_string);
@@ -516,6 +520,7 @@ void SearchProvider::Start(const AutocompleteInput& input,
match.contents_class.push_back(
ACMatchClassification(0, ACMatchClassification::NONE));
match.keyword = providers_.default_provider();
+ match.allowed_to_be_default_match = true;
matches_.push_back(match);
}
Stop(false);
@@ -1068,15 +1073,17 @@ bool SearchProvider::IsTopMatchSearchWithURLInput() const {
matches_.front().type != AutocompleteMatchType::NAVSUGGEST;
}
-bool SearchProvider::IsTopMatchNotInlinable() const {
- // Note: this test assumes the SEARCH_OTHER_ENGINE match corresponds to
- // the verbatim search query on the keyword engine. SearchProvider should
- // not create any other match of type SEARCH_OTHER_ENGINE.
- return
- matches_.front().type != AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED &&
- matches_.front().type != AutocompleteMatchType::SEARCH_OTHER_ENGINE &&
- matches_.front().inline_autocompletion.empty() &&
- matches_.front().fill_into_edit != input_.text();
+bool SearchProvider::IsTopMatchNotAllowedToBeDefaultMatch() const {
+ return !matches_.front().allowed_to_be_default_match;
+}
+
+bool SearchProvider::LacksDefaultMatch() const {
+ for (ACMatches::const_iterator it = matches_.begin(); it != matches_.end();
+ ++it) {
+ if (it->allowed_to_be_default_match)
+ return false;
+ }
+ return true;
}
void SearchProvider::UpdateMatches() {
@@ -1101,11 +1108,15 @@ void SearchProvider::UpdateMatches() {
ConvertResultsToAutocompleteMatches();
DCHECK(!IsTopMatchNavigationInKeywordMode());
}
- if (IsTopMatchScoreTooLow()) {
+ if (!omnibox_will_reorder_for_legal_default_match_ &&
+ IsTopMatchScoreTooLow()) {
// Disregard the suggested verbatim relevance if the top score is below
// the usual verbatim value. For example, a BarProvider may rely on
- // SearchProvider's verbatim or inlineable matches for input "foo" to
- // always outrank its own lowly-ranked non-inlineable "bar" match.
+ // SearchProvider's verbatim or inlineable matches for input "foo" (all
+ // allowed to be default match) to always outrank its own lowly-ranked
+ // "bar" matches that shouldn't be the default match. This only needs
+ // to be enforced when the omnibox will not reorder results to make a
+ // legal default match first.
default_results_.verbatim_relevance = -1;
keyword_results_.verbatim_relevance = -1;
ConvertResultsToAutocompleteMatches();
@@ -1121,18 +1132,28 @@ void SearchProvider::UpdateMatches() {
keyword_results_.verbatim_relevance = -1;
ConvertResultsToAutocompleteMatches();
}
- if (IsTopMatchNotInlinable()) {
- // Disregard suggested relevances if the top match is not a verbatim match
- // or inlinable. For example, input "foo" should not invoke a search for
- // "bar", which would happen if the "bar" search match outranked all other
- // matches.
+ if (omnibox_will_reorder_for_legal_default_match_ ?
+ LacksDefaultMatch() : IsTopMatchNotAllowedToBeDefaultMatch()) {
+ // If the omnibox is not going to reorder results to put a legal default
+ // match at the top, then this provider needs to guarantee that its top
+ // scoring result is a legal default match (i.e., it's either a verbatim
+ // match or inlinable). For example, input "foo" should not invoke a
+ // search for "bar", which would happen if the "bar" search match
+ // outranked all other matches. On the other hand, if the omnibox will
+ // reorder matches as necessary to put a legal default match at the top,
+ // all we need to guarantee is that SearchProvider returns a legal
+ // default match. (The omnibox always needs at least one legal default
+ // match, and it relies on SearchProvider to always return one.)
ApplyCalculatedRelevance();
ConvertResultsToAutocompleteMatches();
}
DCHECK(!IsTopMatchNavigationInKeywordMode());
- DCHECK(!IsTopMatchScoreTooLow());
+ DCHECK(omnibox_will_reorder_for_legal_default_match_ ||
+ !IsTopMatchScoreTooLow());
DCHECK(!IsTopMatchSearchWithURLInput());
- DCHECK(!IsTopMatchNotInlinable());
+ DCHECK(omnibox_will_reorder_for_legal_default_match_ ||
+ !IsTopMatchNotAllowedToBeDefaultMatch());
+ DCHECK(!LacksDefaultMatch());
}
UpdateStarredStateOfMatches();
@@ -1448,6 +1469,8 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
if (!input_.prevent_inline_autocomplete() &&
(inline_autocomplete_offset != string16::npos)) {
DCHECK(inline_autocomplete_offset <= match.fill_into_edit.length());
+ match.allowed_to_be_default_match =
+ (inline_autocomplete_offset != string16::npos);
match.inline_autocompletion =
match.fill_into_edit.substr(inline_autocomplete_offset);
}

Powered by Google App Engine
This is Rietveld 408576698