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

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: one final pass through the code 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 2f304504aedc799592c70a674e19c82aaa784837..5dca8c30f0291e0753d3d12cd498edb7bca6abbc 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
@@ -328,6 +331,7 @@ AutocompleteMatch SearchProvider::CreateSearchSuggestion(
StartsWith(query_string, input_text, false)) {
match.inline_autocomplete_offset =
match.fill_into_edit.length() + input_text.length();
+ match.allowed_to_be_default_match = true;
}
match.fill_into_edit.append(query_string);
@@ -1072,15 +1076,17 @@ bool SearchProvider::IsTopMatchHighRankSearchForURL() const {
matches_.front().type == AutocompleteMatchType::SEARCH_OTHER_ENGINE);
}
-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_autocomplete_offset == string16::npos &&
- 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() {
@@ -1125,18 +1131,32 @@ 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_ &&
Peter Kasting 2013/07/17 18:29:48 Nit: Simpler: if (omnibox_will_reorder_for_le
Mark P 2013/07/17 20:39:18 Done.
+ IsTopMatchNotAllowedToBeDefaultMatch()) ||
+ (omnibox_will_reorder_for_legal_default_match_ &&
+ LacksDefaultMatch())) {
+ // Disregard suggested relevances if either:
+ // * the top match is not allowed to be the default match (i.e., it's not
+ // 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. We only care
+ // about this condition if the omnibox itself will not reorder
+ // matches to put a legal default match on top.
+ // * no matches are allowed to be the default match. SearchProvider is
+ // supposed to return a legal default match for any input. We need
+ // to make sure it does so. This constraint should be true always,
+ // but we only need to check it if
+ // |omnibox_will_reorder_for_legal_default_match_|. (If that were
+ // false, the first part of the test above will suffice.)
ApplyCalculatedRelevance();
ConvertResultsToAutocompleteMatches();
}
DCHECK(!IsTopMatchNavigationInKeywordMode());
DCHECK(!IsTopMatchScoreTooLow());
DCHECK(!IsTopMatchHighRankSearchForURL());
- DCHECK(!IsTopMatchNotInlinable());
+ DCHECK(omnibox_will_reorder_for_legal_default_match_ ||
+ !IsTopMatchNotAllowedToBeDefaultMatch());
+ DCHECK(!LacksDefaultMatch());
}
UpdateStarredStateOfMatches();

Powered by Google App Engine
This is Rietveld 408576698