Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/autocomplete/search_provider.cc

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..cd7ee7dd3660719d0117915ceb831bf17c1132a7 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);
 
@@ -517,6 +521,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);
@@ -1072,15 +1077,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() {
@@ -1105,11 +1112,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();
@@ -1125,18 +1136,31 @@ 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()) {
+      // Disregard suggested relevances if either:
+      // * no matches are allowed to be the default match.  SearchProvider is

[Peter Kasting, 2013/07/17 22:12:30]

Tiny nit: I'd capitalize the first word of each bullet

[Mark P, 2013/07/17 23:50:45]

Done.

+      //   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.)
+      // * 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.
       ApplyCalculatedRelevance();
       ConvertResultsToAutocompleteMatches();
     }
     DCHECK(!IsTopMatchNavigationInKeywordMode());
-    DCHECK(!IsTopMatchScoreTooLow());
+    DCHECK(omnibox_will_reorder_for_legal_default_match_ ||
+           !IsTopMatchScoreTooLow());
     DCHECK(!IsTopMatchHighRankSearchForURL());
-    DCHECK(!IsTopMatchNotInlinable());
+    DCHECK(omnibox_will_reorder_for_legal_default_match_ ||
+           !IsTopMatchNotAllowedToBeDefaultMatch());
+    DCHECK(!LacksDefaultMatch());
   }
 
   UpdateStarredStateOfMatches();
@@ -1449,8 +1473,11 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
           net::FormatUrl(navigation.url(), languages, format_types,
                          net::UnescapeRule::SPACES, NULL, NULL,
                          &inline_autocomplete_offset));
-  if (!input_.prevent_inline_autocomplete())
+  if (!input_.prevent_inline_autocomplete()) {
+    match.allowed_to_be_default_match =
+        (inline_autocomplete_offset != string16::npos);
     match.inline_autocomplete_offset = inline_autocomplete_offset;
+  }
   DCHECK((match.inline_autocomplete_offset == string16::npos) ||
          (match.inline_autocomplete_offset <= match.fill_into_edit.length()));