Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/autocomplete/autocomplete_result.cc

Index: chrome/browser/autocomplete/autocomplete_result.cc
diff --git a/chrome/browser/autocomplete/autocomplete_result.cc b/chrome/browser/autocomplete/autocomplete_result.cc
index 832be7325d22f41d776ed8319d0838d65f09b58d..95bee29ba262bada9291622c58f8f60214693482 100644
--- a/chrome/browser/autocomplete/autocomplete_result.cc
+++ b/chrome/browser/autocomplete/autocomplete_result.cc
@@ -156,8 +156,20 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
   // Sort and trim to the most relevant kMaxMatches matches.
   size_t max_num_matches = std::min(kMaxMatches, matches_.size());
   CompareWithDemoteByType comparing_object(input.current_page_classification());
-  std::partial_sort(matches_.begin(), matches_.begin() + max_num_matches,
-                    matches_.end(), comparing_object);
+  std::sort(matches_.begin(), matches_.end(), comparing_object);
+  if (!matches_.empty() && !matches_.begin()->allowed_to_be_default_match &&
+      OmniboxFieldTrial::ReorderForLegalDefaultMatch(
+          input.current_page_classification())) {
+    // Top match is not allowed to be the default match.  Find the most
+    // relevant legal match and shift it to the front.
+    for (AutocompleteResult::iterator it = matches_.begin() + 1;
+         it != matches_.end(); ++it) {
+      if (it->allowed_to_be_default_match) {
+        std::rotate(matches_.begin(), it, it + 1);
+        break;
+      }
+    }
+  }
   // In the process of trimming, drop all matches with a demoted relevance
   // score of 0.
   size_t num_matches;
@@ -166,10 +178,12 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
        ++num_matches) {}
   matches_.resize(num_matches);
 
-  default_match_ = begin();
+  default_match_ = matches_.begin();
+  DCHECK((default_match_ == matches_.end()) ||

[msw, 2013/08/10 17:32:18]

nit: Use matches_.empty(), not default_match_ == matches_.end(); line 186 too.

[Mark P, 2013/08/11 03:22:48]

I'd prefer to leave this as it is because it's meant to be an assertion about
the validity of default_match_.  (I can imagine that default_match_ could end up
being an invalid iterator, and I don't want to have this CHECK pass if it's
invalid, which could happen if I change the == end() to !empty().  Obviously it
can't happen with line 181 immediately before this check, but still...)

I can move the DCHECK to line 188 for it to be more obviously a post-condition
for this function.  It would also look less silly there.

+      default_match_->allowed_to_be_default_match);
 
   // Set the alternate nav URL.
-  alternate_nav_url_ = (default_match_ == end()) ?
+  alternate_nav_url_ = (default_match_ == matches_.end()) ?
       GURL() : ComputeAlternateNavUrl(input, *default_match_);
 }