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 7238c1ea87144b6e314c5903edeff610f5d8f1f5..fe4086c53b6bd44a28cca81007c238e3fe42ba1c 100644
--- a/chrome/browser/autocomplete/autocomplete_result.cc
+++ b/chrome/browser/autocomplete/autocomplete_result.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
+#include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/common/autocomplete_match_type.h"
 
@@ -102,15 +103,28 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
                  matches_.end());
 
   // Sort and trim to the most relevant kMaxMatches matches.
-  const size_t num_matches = std::min(kMaxMatches, matches_.size());
-  std::partial_sort(matches_.begin(), matches_.begin() + num_matches,
-                    matches_.end(), &AutocompleteMatch::MoreRelevant);
-  matches_.resize(num_matches);
+  std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::MoreRelevant);
+  if (!empty() && !matches_.begin()->allowed_to_be_default_match &&

[H Fung, 2013/08/08 20:10:36]

!matches_.empty() also, to be consistent?

[Mark P, 2013/08/08 22:06:10]

Done.

[Peter Kasting, 2013/08/09 21:59:34]

Doesn't look changed?

Honestly, I would also be OK with switching everything in this function to using
the member accessors instead of writing "matches_.X" everywhere (it'd be
shorter), just as long as things are consistent.

[Mark P, 2013/08/09 22:24:11]

This change disappeared when rebasing.  I noticed it later and uploaded a new
patch, but you reviewed a slightly earlier patch.
I changed things to drop matches_. where applicable.  I found it looked funny to
me, making it harder to read  things like std::sort(begin(), end(), ...) to
infer what was actually being sorted.  I decided to leave the matches_ as is in
this function.

+      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;
+      }
+    }
+  }
+  matches_.resize(std::min(kMaxMatches, matches_.size()));
 
-  default_match_ = begin();
+  default_match_ = matches_.begin();
+  DCHECK((default_match_ == matches_.end()) ||
+      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_);
 }