Don't demote top match for certain match types.

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 0c01b3923344778c50e930e693230cfa507aa19c..8f75cdde417640dc7ae0f3b67d7fa6fd8af5477c 100644
--- a/chrome/browser/autocomplete/autocomplete_result.cc
+++ b/chrome/browser/autocomplete/autocomplete_result.cc
@@ -153,11 +153,26 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
   matches_.erase(std::unique(matches_.begin(), matches_.end(),
                              &AutocompleteMatch::DestinationsEqual),
                  matches_.end());
+  size_t max_num_matches = std::min(kMaxMatches, matches_.size());
+
+  // Don't demote the top match if applicable.
+  OmniboxFieldTrial::UndemotableTopMatchTypes undemotable_top_types;
+  OmniboxFieldTrial::GetUndemotableTopTypes(input.current_page_classification(),
+                                            &undemotable_top_types);
+  const bool have_top_undemotable_match = !matches_.empty() &&
+      undemotable_top_types.find(matches_.begin()->type) !=
+      undemotable_top_types.end();

[Mark P, 2013/11/04 20:30:12]

nit: parens around != please

[H Fung, 2013/11/04 21:39:57]

Done.

+  AutocompleteMatch top_undemotable_match;
+  if (have_top_undemotable_match) {
+    top_undemotable_match = *matches_.begin();
+    matches_.erase(matches_.begin());
+  }
 
   // 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::sort(matches_.begin(), matches_.end(), comparing_object);
+  if (have_top_undemotable_match)
+    matches_.insert(matches_.begin(), top_undemotable_match);
   if (!matches_.empty() && !matches_.begin()->allowed_to_be_default_match &&
       OmniboxFieldTrial::ReorderForLegalDefaultMatch(
           input.current_page_classification())) {
@@ -316,6 +331,10 @@ void AutocompleteResult::AddMatch(
   DCHECK_EQ(AutocompleteMatch::SanitizeString(match.contents), match.contents);
   DCHECK_EQ(AutocompleteMatch::SanitizeString(match.description),
             match.description);
+  // There is logic in SortAndCull() to possibly not demote the top match (so
+  // that CompareWithDemoteByType is not called on the top match).  That logic
+  // is not replicated here since we depend on SortAndCull() on |matches_| after
+  // this function is called.

[Mark P, 2013/11/04 20:30:12]

This comment made me realize the comment in autocomplet_result.h by the
declaration of AddMatch() is wrong:
  // Adds a single match. The match is inserted at the appropriate position
  // based on relevancy and display order. This is ONLY for use after
  // SortAndCull() has been invoked, and preserves default_match_.
Can you fix it and replace the last sentence of that comment with this sentence:
 // It preserves default_match_.
(after all, it's never used after SortAndCull, only before)

As for the comment here, how about replacing your comment with this?
(or combining your comment with this suggestion:)
// GetUndemotableTopTypes() is not used here because we're only worried about
preserving the top result when calculating the default match, which happens in
SortAndCull().  Because AddMatch() does not change the default match, there is
no need think about undemotable types here.

[H Fung, 2013/11/04 21:39:57]

Actually, SortAndCull() is called both before and after AddMatch().  
AutocompleteController::UpdateResult() calls SortAndCull() before
CopyOldMatches() (which calls MergeMatchesByProvider() which calls AddMatch). 
The only other caller to CopyOldMatches() is a test.
I'm not sure that's true.  What |default_match_| points to is not changed here
(even if the match is added to the top position), but |default_match_| can get
reset in SortAndCull().

   CompareWithDemoteByType comparing_object(page_classification);
   ACMatches::iterator insertion_point =
       std::upper_bound(begin(), end(), match, comparing_object);