Part 4 of search provider refactoring.

Moves AddMatchToMap method to the common base class. Adds method parameters
so that both SearchProvider and ZeroSuggestProvider can use this method.

For SearchProvider this is a pure refactoring. I added some helper classes
in SearchProvider to avoid duplicating code.

For ZeroSuggestProvider, this introduces some additional functionality on
top of what it used to have. Mainly the method may record additional metadata on
the AutocompleteMatch. This metadata is not currently used by zero suggest
provider and as far as I can tell will not in any way affect the operation
of zero suggest provider today. In the future, some of the metadata
recorded will actually be useful (e.g. I am planning to take advantage of
deletion_url).

BUG=338955
TBR=mpearson

Review URL: https://codereview.chromium.org/158053002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251236 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/autocomplete/base_search_provider.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/autocomplete/base_search_provider.h"
 
+#include "base/i18n/case_conversion.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
@@ -12,6 +13,9 @@
 #include "chrome/browser/autocomplete/url_prefix.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/instant_service.h"
+#include "chrome/browser/search/instant_service_factory.h"
+#include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ -33,6 +37,11 @@ BaseSearchProvider::BaseSearchProvider(AutocompleteProviderListener* listener,
       field_trial_triggered_(false),
       field_trial_triggered_in_session_(false) {}
 
+// static
+bool BaseSearchProvider::ShouldPrefetch(const AutocompleteMatch& match) {
+  return match.GetAdditionalInfo(kShouldPrefetchKey) == kTrue;
+}
+
 void BaseSearchProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
   provider_info->push_back(metrics::OmniboxEventProto_ProviderInfo());
   metrics::OmniboxEventProto_ProviderInfo& new_entry = provider_info->back();
@@ -50,6 +59,15 @@ void BaseSearchProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
   }
 }
 
+// static
+const char BaseSearchProvider::kRelevanceFromServerKey[] =
+    "relevance_from_server";
+const char BaseSearchProvider::kShouldPrefetchKey[] = "should_prefetch";
+const char BaseSearchProvider::kSuggestMetadataKey[] = "suggest_metadata";
+const char BaseSearchProvider::kDeletionUrlKey[] = "deletion_url";
+const char BaseSearchProvider::kTrue[] = "true";
+const char BaseSearchProvider::kFalse[] = "false";
+
 BaseSearchProvider::~BaseSearchProvider() {}
 
 // BaseSearchProvider::Result --------------------------------------------------
@@ -258,7 +276,6 @@ bool BaseSearchProvider::Results::HasServerProvidedScores() const {
 AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
     AutocompleteProvider* autocomplete_provider,
     const AutocompleteInput& input,
-    const base::string16& input_text,
     const SuggestResult& suggestion,
     const TemplateURL* template_url,
     int accepted_suggestion,
@@ -277,7 +294,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
     match.description = suggestion.annotation();
 
   match.allowed_to_be_default_match =
-      (input_text == suggestion.match_contents());
+      (input.text() == suggestion.match_contents());
 
   // When the user forced a query, we need to make sure all the fill_into_edit
   // values preserve that property.  Otherwise, if the user starts editing a
@@ -287,9 +304,9 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
   if (suggestion.from_keyword_provider())
     match.fill_into_edit.append(match.keyword + base::char16(' '));
   if (!input.prevent_inline_autocomplete() &&
-      StartsWith(suggestion.suggestion(), input_text, false)) {
+      StartsWith(suggestion.suggestion(), input.text(), false)) {
     match.inline_autocompletion =
-        suggestion.suggestion().substr(input_text.length());
+        suggestion.suggestion().substr(input.text().length());
     match.allowed_to_be_default_match = true;
   }
   match.fill_into_edit.append(suggestion.suggestion());
@@ -298,7 +315,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
   DCHECK(search_url.SupportsReplacement());
   match.search_terms_args.reset(
       new TemplateURLRef::SearchTermsArgs(suggestion.suggestion()));
-  match.search_terms_args->original_query = input_text;
+  match.search_terms_args->original_query = input.text();
   match.search_terms_args->accepted_suggestion = accepted_suggestion;
   match.search_terms_args->omnibox_start_margin = omnibox_start_margin;
   match.search_terms_args->suggest_query_params =
@@ -403,3 +420,80 @@ bool BaseSearchProvider::CanSendURL(
   return true;
 }
 
+void BaseSearchProvider::AddMatchToMap(const SuggestResult& result,
+                                       const std::string& metadata,
+                                       int accepted_suggestion,
+                                       MatchMap* map) {
+  InstantService* instant_service =
+      InstantServiceFactory::GetForProfile(profile_);
+  // Android and iOS have no InstantService.
+  const int omnibox_start_margin = instant_service ?
+      instant_service->omnibox_start_margin() : chrome::kDisableStartMargin;
+
+  AutocompleteMatch match = CreateSearchSuggestion(
+      this, GetInput(result), result, GetTemplateURL(result),
+      accepted_suggestion, omnibox_start_margin,
+      ShouldAppendExtraParams(result));
+  if (!match.destination_url.is_valid())
+    return;
+  match.search_terms_args->bookmark_bar_pinned =
+      profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
+  match.RecordAdditionalInfo(kRelevanceFromServerKey,
+                             result.relevance_from_server() ? kTrue : kFalse);
+  match.RecordAdditionalInfo(kShouldPrefetchKey,
+                             result.should_prefetch() ? kTrue : kFalse);
+
+  if (!result.deletion_url().empty()) {
+    GURL url(match.destination_url.GetOrigin().Resolve(result.deletion_url()));
+    if (url.is_valid()) {
+      match.RecordAdditionalInfo(kDeletionUrlKey, url.spec());
+      match.deletable = true;
+    }
+  }
+
+  // Metadata is needed only for prefetching queries.
+  if (result.should_prefetch())
+    match.RecordAdditionalInfo(kSuggestMetadataKey, metadata);
+
+  // Try to add |match| to |map|.  If a match for this suggestion 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(result.suggestion()),
+                     match.search_terms_args->suggest_query_params));
+  const std::pair<MatchMap::iterator, bool> i(
+       map->insert(std::make_pair(match_key, match)));
+
+  bool should_prefetch = result.should_prefetch();
+  if (!i.second) {
+     // 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 (match.relevance > i.first->second.relevance) {
+       i.first->second = match;
+     } else if (match.keyword == i.first->second.keyword) {
+       // Old and new matches are from the same search provider. It is okay to
+       // record one match's prefetch data onto a different match (for the same
+       // query string) for the following reasons:
+       // 1. Because the suggest server only sends down a query string from
+       // which we construct a URL, rather than sending a full URL, and because
+       // we construct URLs from query strings in the same way every time, the
+       // URLs for the two matches will be the same. Therefore, we won't end up
+       // prefetching something the server didn't intend.
+       // 2. Presumably the server sets the prefetch bit on a match it things is
+       // sufficiently relevant that the user is likely to choose it. Surely
+       // setting the prefetch bit on a match of even higher relevance won't
+       // violate this assumption.
+       should_prefetch |= ShouldPrefetch(i.first->second);
+       i.first->second.RecordAdditionalInfo(kShouldPrefetchKey,
+                                            should_prefetch ? kTrue : kFalse);
+       if (should_prefetch)
+         i.first->second.RecordAdditionalInfo(kSuggestMetadataKey, metadata);
+     }
+   }
+}

chrome/browser/autocomplete/base_search_provider.h

@@ -39,6 +39,9 @@ class BaseSearchProvider : public AutocompleteProvider,
                      Profile* profile,
                      AutocompleteProvider::Type type);
 
+  // Returns whether |match| is flagged as a query that should be prefetched.
+  static bool ShouldPrefetch(const AutocompleteMatch& match);
+
   // AutocompleteProvider:
   virtual void AddProviderInfo(ProvidersInfo* provider_info) const OVERRIDE;
 
@@ -47,6 +50,26 @@ class BaseSearchProvider : public AutocompleteProvider,
   }
 
  protected:
+  // The following keys are used to record additional information on matches.
+
+  // We annotate our AutocompleteMatches with whether their relevance scores
+  // were server-provided using this key in the |additional_info| field.
+  static const char kRelevanceFromServerKey[];
+
+  // Indicates whether the server said a match should be prefetched.
+  static const char kShouldPrefetchKey[];
+
+  // Used to store metadata from the server response, which is needed for
+  // prefetching.
+  static const char kSuggestMetadataKey[];
+
+  // Used to store a deletion request url for server-provided suggestions.
+  static const char kDeletionUrlKey[];
+
+  // These are the values for the above keys.
+  static const char kTrue[];
+  static const char kFalse[];
+
   virtual ~BaseSearchProvider();
 
   // The Result classes are intermediate representations of AutocompleteMatches,
@@ -258,21 +281,20 @@ class BaseSearchProvider : public AutocompleteProvider,
   // for the search |suggestion|, which represents a search via |template_url|.
   // If |template_url| is NULL, returns a match with an invalid destination URL.
   //
-  // |input_text| is the original user input.  This is used to highlight
+  // |input| is the original user input. Text in the input is used to highlight
   // portions of the match contents to distinguish locally-typed text from
   // suggested text.
   //
-  // |input| is necessary for various other details, like whether we should
+  // |input| is also necessary for various other details, like whether we should
   // allow inline autocompletion and what the transition type should be.
-  // |accepted_suggestion| and |omnibox_start_margin| are used along with
-  // |input_text| to generate Assisted Query Stats.
+  // |accepted_suggestion| and |omnibox_start_margin| are used to generate
+  // Assisted Query Stats.
   // |append_extra_query_params| should be set if |template_url| is the default
   // search engine, so the destination URL will contain any
   // command-line-specified query params.
   static AutocompleteMatch CreateSearchSuggestion(
       AutocompleteProvider* autocomplete_provider,
       const AutocompleteInput& input,
-      const base::string16& input_text,
       const SuggestResult& suggestion,
       const TemplateURL* template_url,
       int accepted_suggestion,
@@ -313,6 +335,29 @@ class BaseSearchProvider : public AutocompleteProvider,
       AutocompleteInput::PageClassification page_classification,
       Profile* profile);
 
+  // Creates an AutocompleteMatch from |result| to search for the query in
+  // |result|. Adds the created match to |map|; if such a match
+  // already exists, whichever one has lower relevance is eliminated.
+  // |metadata| and |accepted_suggestion| are used for generating an
+  // AutocompleteMatch.
+  void AddMatchToMap(const SuggestResult& result,
+                     const std::string& metadata,
+                     int accepted_suggestion,
+                     MatchMap* map);
+
+  // Returns the TemplateURL for the given |result|.
+  virtual const TemplateURL* GetTemplateURL(
+      const SuggestResult& result) const = 0;
+
+  // Returns the AutocompleteInput based on whether the |result| is from the
+  // default provider or from the keyword provider.
+  virtual const AutocompleteInput GetInput(
+      const SuggestResult& result) const = 0;
+
+  // Returns whether the destination URL corresponding to the given |result|
+  // should contain command-line-specified query params.
+  virtual bool ShouldAppendExtraParams(const SuggestResult& result) const = 0;
+
   // Whether a field trial, if any, has triggered in the most recent
   // autocomplete query. This field is set to true only if the suggestion
   // provider has completed and the response contained

chrome/browser/autocomplete/search_provider.cc

@@ -29,15 +29,10 @@
 #include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/instant_service.h"
-#include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/search/instant_controller.h"
 #include "chrome/common/net/url_fixer_upper.h"
 #include "chrome/common/pref_names.h"
@@ -202,12 +197,6 @@ const int SearchProvider::kDefaultProviderURLFetcherID = 1;
 const int SearchProvider::kKeywordProviderURLFetcherID = 2;
 const int SearchProvider::kDeletionURLFetcherID = 3;
 int SearchProvider::kMinimumTimeBetweenSuggestQueriesMs = 100;
-const char SearchProvider::kRelevanceFromServerKey[] = "relevance_from_server";
-const char SearchProvider::kShouldPrefetchKey[] = "should_prefetch";
-const char SearchProvider::kSuggestMetadataKey[] = "suggest_metadata";
-const char SearchProvider::kDeletionUrlKey[] = "deletion_url";
-const char SearchProvider::kTrue[] = "true";
-const char SearchProvider::kFalse[] = "false";
 
 SearchProvider::SearchProvider(AutocompleteProviderListener* listener,
                                Profile* profile)
@@ -216,11 +205,6 @@ SearchProvider::SearchProvider(AutocompleteProviderListener* listener,
       suggest_results_pending_(0) {
 }
 
-// static
-bool SearchProvider::ShouldPrefetch(const AutocompleteMatch& match) {
-  return match.GetAdditionalInfo(kShouldPrefetchKey) == kTrue;
-}
-
 // static
 std::string SearchProvider::GetSuggestMetadata(const AutocompleteMatch& match) {
   return match.GetAdditionalInfo(kSuggestMetadataKey);
@@ -492,6 +476,23 @@ void SearchProvider::OnURLFetchComplete(const net::URLFetcher* source) {
     listener_->OnProviderUpdate(results_updated);
 }
 
+const TemplateURL* SearchProvider::GetTemplateURL(
+    const SuggestResult& result) const {
+  return result.from_keyword_provider() ? providers_.GetKeywordProviderURL()
+                                        : providers_.GetDefaultProviderURL();
+}
+
+const AutocompleteInput SearchProvider::GetInput(
+    const SuggestResult& result) const {
+  return result.from_keyword_provider() ? keyword_input_ : input_;
+}
+
+bool SearchProvider::ShouldAppendExtraParams(
+    const SuggestResult& result) const {
+  return !result.from_keyword_provider() ||
+      providers_.default_provider().empty();
+}
+
 void SearchProvider::OnDeletionComplete(bool success,
                                         SuggestionDeletionHandler* handler) {
   RecordDeletionResult(success);
@@ -1005,8 +1006,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
         input_.text(), AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
         input_.text(), base::string16(), std::string(), std::string(), false,
         verbatim_relevance, relevance_from_server, false, input_.text());
-    AddMatchToMap(verbatim, input_.text(), std::string(),
-                  did_not_accept_default_suggestion, &map);
+    AddMatchToMap(
+        verbatim, std::string(), did_not_accept_default_suggestion, &map);
   }
   if (!keyword_input_.text().empty()) {
     const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
@@ -1027,8 +1028,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
             keyword_input_.text(), base::string16(), std::string(),
             std::string(), true, keyword_verbatim_relevance,
             keyword_relevance_from_server, false, keyword_input_.text());
-        AddMatchToMap(verbatim, keyword_input_.text(), std::string(),
-                      did_not_accept_keyword_suggestion, &map);
+        AddMatchToMap(
+            verbatim, std::string(), did_not_accept_keyword_suggestion, &map);
       }
     }
   }
@@ -1326,8 +1327,7 @@ void SearchProvider::AddHistoryResultsToMap(const HistoryResults& results,
                                          is_keyword);
   for (SuggestResults::const_iterator i(scored_results.begin());
        i != scored_results.end(); ++i) {
-    AddMatchToMap(*i, input_text, std::string(),
-                  did_not_accept_suggestion, map);
+    AddMatchToMap(*i, std::string(), did_not_accept_suggestion, map);
   }
   UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.AddHistoryResultsTime",
                       base::TimeTicks::Now() - start_time);
@@ -1403,12 +1403,8 @@ SearchProvider::SuggestResults SearchProvider::ScoreHistoryResults(
 void SearchProvider::AddSuggestResultsToMap(const SuggestResults& results,
                                             const std::string& metadata,
                                             MatchMap* map) {
-  for (size_t i = 0; i < results.size(); ++i) {
-    const bool is_keyword = results[i].from_keyword_provider();
-    const base::string16& input = is_keyword ? keyword_input_.text()
-                                             : input_.text();
-    AddMatchToMap(results[i], input, metadata, i, map);
-  }
+  for (size_t i = 0; i < results.size(); ++i)
+    AddMatchToMap(results[i], metadata, i, map);
 }
 
 int SearchProvider::GetVerbatimRelevance(bool* relevance_from_server) const {
@@ -1519,87 +1515,6 @@ int SearchProvider::CalculateRelevanceForHistory(
   return std::max(0, base_score - score_discount);
 }
 
-void SearchProvider::AddMatchToMap(const SuggestResult& result,
-                                   const base::string16& input_text,
-                                   const std::string& metadata,
-                                   int accepted_suggestion,
-                                   MatchMap* map) {
-  InstantService* instant_service =
-      InstantServiceFactory::GetForProfile(profile_);
-  // Android and iOS have no InstantService.
-  const int omnibox_start_margin = instant_service ?
-      instant_service->omnibox_start_margin() : chrome::kDisableStartMargin;
-
-  const TemplateURL* template_url = result.from_keyword_provider() ?
-      providers_.GetKeywordProviderURL() : providers_.GetDefaultProviderURL();
-  AutocompleteMatch match = CreateSearchSuggestion(
-      this, input_, input_text, result, template_url, accepted_suggestion,
-      omnibox_start_margin,
-      !result.from_keyword_provider() || providers_.default_provider().empty());
-  if (!match.destination_url.is_valid())
-    return;
-  match.search_terms_args->bookmark_bar_pinned =
-      profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
-  match.RecordAdditionalInfo(kRelevanceFromServerKey,
-                             result.relevance_from_server() ? kTrue : kFalse);
-  match.RecordAdditionalInfo(kShouldPrefetchKey,
-                             result.should_prefetch() ? kTrue : kFalse);
-
-  if (!result.deletion_url().empty()) {
-    GURL url(match.destination_url.GetOrigin().Resolve(result.deletion_url()));
-    if (url.is_valid()) {
-      match.RecordAdditionalInfo(kDeletionUrlKey, url.spec());
-      match.deletable = true;
-    }
-  }
-
-  // Metadata is needed only for prefetching queries.
-  if (result.should_prefetch())
-    match.RecordAdditionalInfo(kSuggestMetadataKey, metadata);
-
-  // 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(result.suggestion()),
-                     match.search_terms_args->suggest_query_params));
-  const std::pair<MatchMap::iterator, bool> i(
-      map->insert(std::make_pair(match_key, match)));
-
-  bool should_prefetch = result.should_prefetch();
-  if (!i.second) {
-    // 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 (match.relevance > i.first->second.relevance) {
-      i.first->second = match;
-    } else if (match.keyword == i.first->second.keyword) {
-      // Old and new matches are from the same search provider. It is okay to
-      // record one match's prefetch data onto a different match (for the same
-      // query string) for the following reasons:
-      // 1. Because the suggest server only sends down a query string from which
-      // we construct a URL, rather than sending a full URL, and because we
-      // construct URLs from query strings in the same way every time, the URLs
-      // for the two matches will be the same. Therefore, we won't end up
-      // prefetching something the server didn't intend.
-      // 2. Presumably the server sets the prefetch bit on a match it things is
-      // sufficiently relevant that the user is likely to choose it. Surely
-      // setting the prefetch bit on a match of even higher relevance won't
-      // violate this assumption.
-      should_prefetch |= ShouldPrefetch(i.first->second);
-      i.first->second.RecordAdditionalInfo(kShouldPrefetchKey,
-                                           should_prefetch ? kTrue : kFalse);
-      if (should_prefetch)
-        i.first->second.RecordAdditionalInfo(kSuggestMetadataKey, metadata);
-    }
-  }
-}
-
 AutocompleteMatch SearchProvider::NavigationToMatch(
     const NavigationResult& navigation) {
   const base::string16& input = navigation.from_keyword_provider() ?

chrome/browser/autocomplete/search_provider.h

@@ -57,10 +57,6 @@ class SearchProvider : public BaseSearchProvider {
 
   SearchProvider(AutocompleteProviderListener* listener, Profile* profile);
 
-  // Returns whether the SearchProvider previously flagged |match| as a query
-  // that should be prefetched.
-  static bool ShouldPrefetch(const AutocompleteMatch& match);
-
   // Extracts the suggest response metadata which SearchProvider previously
   // stored for |match|.
   static std::string GetSuggestMetadata(const AutocompleteMatch& match);
@@ -167,6 +163,14 @@ class SearchProvider : public BaseSearchProvider {
   // net::URLFetcherDelegate:
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
+  // BaseSearchProvider:
+  virtual const TemplateURL* GetTemplateURL(
+      const SuggestResult& result) const OVERRIDE;
+  virtual const AutocompleteInput GetInput(
+      const SuggestResult& result) const OVERRIDE;
+  virtual bool ShouldAppendExtraParams(
+      const SuggestResult& result) const OVERRIDE;
+
   // This gets called when we have requested a suggestion deletion from the
   // server to handle the results of the deletion.
   void OnDeletionComplete(bool success,
@@ -312,15 +316,6 @@ class SearchProvider : public BaseSearchProvider {
                                    bool use_aggressive_method,
                                    bool prevent_search_history_inlining) const;
 
-  // Creates an AutocompleteMatch for "Search <engine> for |query_string|" with
-  // the supplied details.  Adds this match to |map|; if such a match already
-  // exists, whichever one has lower relevance is eliminated.
-  void AddMatchToMap(const SuggestResult& result,
-                     const base::string16& input_text,
-                     const std::string& metadata,
-                     int accepted_suggestion,
-                     MatchMap* map);
-
   // Returns an AutocompleteMatch for a navigational suggestion.
   AutocompleteMatch NavigationToMatch(const NavigationResult& navigation);
 
@@ -341,26 +336,6 @@ class SearchProvider : public BaseSearchProvider {
   // previous one.  Non-const because some unittests modify this value.
   static int kMinimumTimeBetweenSuggestQueriesMs;
 
-  // The following keys are used to record additional information on matches.
-
-  // We annotate our AutocompleteMatches with whether their relevance scores
-  // were server-provided using this key in the |additional_info| field.
-  static const char kRelevanceFromServerKey[];
-
-  // Indicates whether the server said a match should be prefetched.
-  static const char kShouldPrefetchKey[];
-
-  // Used to store metadata from the server response, which is needed for
-  // prefetching.
-  static const char kSuggestMetadataKey[];
-
-  // Used to store a deletion request url for server-provided suggestions.
-  static const char kDeletionUrlKey[];
-
-  // These are the values for the above keys.
-  static const char kTrue[];
-  static const char kFalse[];
-
   // Maintains the TemplateURLs used.
   Providers providers_;
 

chrome/browser/autocomplete/zero_suggest_provider.cc

@@ -175,6 +175,27 @@ ZeroSuggestProvider::ZeroSuggestProvider(
 ZeroSuggestProvider::~ZeroSuggestProvider() {
 }
 
+const TemplateURL* ZeroSuggestProvider::GetTemplateURL(
+    const SuggestResult& result) const {
+  // Zero suggest provider should not receive keyword results.
+  DCHECK(!result.from_keyword_provider());
+  return template_url_service_->GetDefaultSearchProvider();
+}
+
+const AutocompleteInput ZeroSuggestProvider::GetInput(
+    const SuggestResult& result) const {
+  AutocompleteInput input;
+  // Set |input|'s text to be |query_string| to avoid bolding.
+  input.UpdateText(result.suggestion(), base::string16::npos, input.parts());
+  return input;
+}
+
+bool ZeroSuggestProvider::ShouldAppendExtraParams(
+      const SuggestResult& result) const {
+  // We always use the default provider for search, so append the params.
+  return true;
+}
+
 void ZeroSuggestProvider::FillResults(const base::Value& root_val,
                                       int* verbatim_relevance,
                                       SuggestResults* suggest_results,
@@ -260,50 +281,19 @@ void ZeroSuggestProvider::FillResults(const base::Value& root_val,
 
 void ZeroSuggestProvider::AddSuggestResultsToMap(
     const SuggestResults& results,
-    const TemplateURL* template_url,
     MatchMap* map) {
   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());
+    // TODO(mariakhomenko): Do not reconstruct SuggestResult objects with
+    // a different query -- create correct objects to begin with.
+    const SuggestResult suggestion(
+        query_string, AutocompleteMatchType::SEARCH_SUGGEST, query_string,
+        base::string16(), std::string(), std::string(), false,
+        results[i].relevance(), true, false, query_string);
+    AddMatchToMap(suggestion, 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,
@@ -367,9 +357,7 @@ void ZeroSuggestProvider::ParseSuggestResults(const base::Value& root_val) {
               &suggest_results, &navigation_results_);
 
   query_matches_map_.clear();
-  AddSuggestResultsToMap(suggest_results,
-                         template_url_service_->GetDefaultSearchProvider(),
-                         &query_matches_map_);
+  AddSuggestResultsToMap(suggest_results, &query_matches_map_);
 }
 
 void ZeroSuggestProvider::OnMostVisitedUrlsAvailable(

chrome/browser/autocomplete/zero_suggest_provider.h

@@ -72,6 +72,14 @@ class ZeroSuggestProvider : public BaseSearchProvider {
 
   virtual ~ZeroSuggestProvider();
 
+  // BaseSearchProvider:
+  virtual const TemplateURL* GetTemplateURL(
+      const SuggestResult& result) const OVERRIDE;
+  virtual const AutocompleteInput GetInput(
+      const SuggestResult& result) const OVERRIDE;
+  virtual bool ShouldAppendExtraParams(
+      const SuggestResult& result) const OVERRIDE;
+
   // The 4 functions below (that take classes defined in SearchProvider as
   // arguments) were copied and trimmed from SearchProvider.
   // TODO(hfung): Refactor them into a new base class common to both
@@ -86,25 +94,11 @@ class ZeroSuggestProvider : public BaseSearchProvider {
                    SuggestResults* suggest_results,
                    NavigationResults* navigation_results);
 
-  // Creates AutocompleteMatches to search |template_url| for "<suggestion>" for
-  // all suggestions in |results|, and adds them to |map|.
+  // Adds AutocompleteMatches for each of the suggestions in |results| to
+  // |map|.
   void AddSuggestResultsToMap(const SuggestResults& results,
-                              const TemplateURL* template_url,
                               MatchMap* map);
 
-  // Creates an AutocompleteMatch with the provided |relevance| and |type| to
-  // search |template_url| for |query_string|.  |accepted_suggestion| will be
-  // used to generate Assisted Query Stats.
-  //
-  // Adds this match to |map|; if such a match already exists, whichever one
-  // has lower relevance is eliminated.
-  void AddMatchToMap(int relevance,
-                     AutocompleteMatch::Type type,
-                     const TemplateURL* template_url,
-                     const base::string16& query_string,
-                     int accepted_suggestion,
-                     MatchMap* map);
-
   // Returns an AutocompleteMatch for a navigational suggestion |navigation|.
   AutocompleteMatch NavigationToMatch(const NavigationResult& navigation);