Part 4 of search provider refactoring.

chrome/browser/autocomplete/zero_suggest_provider.cc

Index: chrome/browser/autocomplete/zero_suggest_provider.cc
diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc
index b6e072277b9d585d346e922937760a0437d16d6e..c3e9f9c9a6a0dcaecc9683f83a17c9cd834ee8c4 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.cc
+++ b/chrome/browser/autocomplete/zero_suggest_provider.cc
@@ -175,6 +175,11 @@ ZeroSuggestProvider::ZeroSuggestProvider(
 ZeroSuggestProvider::~ZeroSuggestProvider() {
 }
 
+bool ZeroSuggestProvider::ShouldAppendExtraParams(

[Mark P, 2014/02/12 23:35:49]

It would be nice to comment why this is true.  (Yes, I know the original code
did not.)

[Maria, 2014/02/13 21:07:36]

Done.

+      const SuggestResult& result) const {
+  return true;
+}
+
 void ZeroSuggestProvider::FillResults(const base::Value& root_val,
                                       int* verbatim_relevance,
                                       SuggestResults* suggest_results,
@@ -262,48 +267,19 @@ void ZeroSuggestProvider::AddSuggestResultsToMap(
     const SuggestResults& results,
     const TemplateURL* template_url,
     MatchMap* map) {
+  AutocompleteInput input;
   for (size_t i = 0; i < results.size(); ++i) {
-    AddMatchToMap(results[i].relevance(), AutocompleteMatchType::SEARCH_SUGGEST,
-                  template_url, results[i].suggestion(), i, map);
+    const base::string16& query_string(results[i].suggestion());
+    const SuggestResult suggestion(

[Mark P, 2014/02/12 23:35:49]

Can you make a TODO in a later changelist to clean this up?  It's icky to create
a list of suggest results, then pull out some elements of each one, and use
those elements to create new suggest results.  We should simply create the right
list in the first place.

[H Fung, 2014/02/13 01:43:33]

Thanks for noticing, Mark.  What's the reason for needing to modify the original
SuggestResult?  Does it have to do with the query string?

[Mark P, 2014/02/13 01:45:30]

I don't recall.  It may simply have to do with the way we copied code into
ZeroSuggest in the first place (i.e., there might not be a reason at all).

[Maria, 2014/02/13 21:07:36]

Done.

+        query_string, AutocompleteMatchType::SEARCH_SUGGEST, query_string,
+        base::string16(), std::string(), std::string(), false,
+        results[i].relevance(), true, false, query_string);
+    // Set input's text to be query_string to avoid bolding

[msw, 2014/02/13 02:43:50]

nit: |input|'s and |query_string|; add a period at the end.

[Maria, 2014/02/13 21:07:36]

Done.

+    input.UpdateText(query_string, base::string16::npos, input.parts());
+    AddMatchToMap(suggestion, input, template_url, std::string(), i, map);
   }
 }
 
-void ZeroSuggestProvider::AddMatchToMap(int relevance,
-                                        AutocompleteMatch::Type type,
-                                        const TemplateURL* template_url,
-                                        const base::string16& query_string,
-                                        int accepted_suggestion,
-                                        MatchMap* map) {
-  // Pass in query_string as the input_text to avoid bolding.
-  SuggestResult suggestion(
-      query_string, type, query_string, base::string16(), std::string(),
-      std::string(), false, relevance, true, false, query_string);
-  // TODO(samarth|melevin): use the actual omnibox margin here as well instead
-  // of passing in -1.
-  AutocompleteMatch match = CreateSearchSuggestion(this, AutocompleteInput(),
-      query_string, suggestion, template_url, accepted_suggestion, -1, true);
-  if (!match.destination_url.is_valid())
-    return;
-
-  // Try to add |match| to |map|.  If a match for |query_string| is already in
-  // |map|, replace it if |match| is more relevant.
-  // NOTE: Keep this ToLower() call in sync with url_database.cc.
-  MatchKey match_key(
-      std::make_pair(base::i18n::ToLower(query_string), std::string()));
-  const std::pair<MatchMap::iterator, bool> i(map->insert(
-      std::make_pair(match_key, match)));
-  // NOTE: We purposefully do a direct relevance comparison here instead of
-  // using AutocompleteMatch::MoreRelevant(), so that we'll prefer "items added
-  // first" rather than "items alphabetically first" when the scores are equal.
-  // The only case this matters is when a user has results with the same score
-  // that differ only by capitalization; because the history system returns
-  // results sorted by recency, this means we'll pick the most recent such
-  // result even if the precision of our relevance score is too low to
-  // distinguish the two.
-  if (!i.second && (match.relevance > i.first->second.relevance))
-    i.first->second = match;
-}
-
 AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
     const NavigationResult& navigation) {
   AutocompleteMatch match(this, navigation.relevance(), false,