Part 6 of search provider refactoring.

Makes changes to how the state is recorded in zero_suggest_provider
to facilitate moving out ParseSuggestResults into super class.

Replaces navigation_results_, query_matches_map_, and
verbatim_relevance_ objects with a Results results_ instance. The latter
keeps track of navigation_results_ and query_results_ and constructs
the query matches map on when it's needed.

verbatim_relevance_ in the old code is only used to see the current url
match (based on the server relevance returned in the previous request
or a static value). The new code keeps track of the server verbatim
relevance in results_ and uses an accessor to get either server results
or a constant value if no server results are available, matching the
old semantics.

Merged FillResults with ParseSuggestResults function and moved match
map creation code into ConvertResultsToAutocompleteMatches where it's
used.

BUG=338955
TBR=mpearson

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

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

2 files changed, 106 insertions, 120 deletions

diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc
index a81079d..f488c9e 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.cc
+++ b/chrome/browser/autocomplete/zero_suggest_provider.cc
@@ -139,7 +139,6 @@ void ZeroSuggestProvider::StartZeroSuggest(
           page_classification, profile_) ||
       !OmniboxFieldTrial::InZeroSuggestFieldTrial())
     return;
-  verbatim_relevance_ = kDefaultVerbatimZeroSuggestRelevance;
   done_ = false;
   // TODO(jered): Consider adding locally-sourced zero-suggestions here too.
   // These may be useful on the NTP or more relevant to the user than server
@@ -154,7 +153,6 @@ ZeroSuggestProvider::ZeroSuggestProvider(
                          AutocompleteProvider::TYPE_ZERO_SUGGEST),
       template_url_service_(TemplateURLServiceFactory::GetForProfile(profile)),
       have_pending_request_(false),
-      verbatim_relevance_(kDefaultVerbatimZeroSuggestRelevance),
       weak_ptr_factory_(this) {
 }
 
@@ -190,95 +188,16 @@ void ZeroSuggestProvider::StopSuggest() {
 }
 
 void ZeroSuggestProvider::ClearAllResults() {
-  query_matches_map_.clear();
-  navigation_results_.clear();
+  // We do not call Clear() on |results_| to retain |verbatim_relevance|
+  // value in the |results_| object. |verbatim_relevance| is used at the
+  // beginning of the next StartZeroSuggest() call to determine the current url
+  // match relevance.
+  results_.suggest_results.clear();
+  results_.navigation_results.clear();
   current_query_.clear();
   matches_.clear();
 }
 
-void ZeroSuggestProvider::FillResults(const base::Value& root_val,
-                                      int* verbatim_relevance,
-                                      SuggestResults* suggest_results,
-                                      NavigationResults* navigation_results) {
-  base::string16 query;
-  const base::ListValue* root_list = NULL;
-  const base::ListValue* results = NULL;
-  const base::ListValue* relevances = NULL;
-  // The response includes the query, which should be empty for ZeroSuggest
-  // responses.
-  if (!root_val.GetAsList(&root_list) || !root_list->GetString(0, &query) ||
-      (!query.empty()) || !root_list->GetList(1, &results))
-    return;
-
-  // 3rd element: Description list.
-  const base::ListValue* descriptions = NULL;
-  root_list->GetList(2, &descriptions);
-
-  // 4th element: Disregard the query URL list for now.
-
-  // Reset suggested relevance information from the provider.
-  *verbatim_relevance = kDefaultVerbatimZeroSuggestRelevance;
-
-  // 5th element: Optional key-value pairs from the Suggest server.
-  const base::ListValue* types = NULL;
-  const base::DictionaryValue* extras = NULL;
-  if (root_list->GetDictionary(4, &extras)) {
-    extras->GetList("google:suggesttype", &types);
-
-    // Discard this list if its size does not match that of the suggestions.
-    if (extras->GetList("google:suggestrelevance", &relevances) &&
-        relevances->GetSize() != results->GetSize())
-      relevances = NULL;
-    extras->GetInteger("google:verbatimrelevance", verbatim_relevance);
-
-    // Check if the active suggest field trial (if any) has triggered.
-    bool triggered = false;
-    extras->GetBoolean("google:fieldtrialtriggered", &triggered);
-    field_trial_triggered_ |= triggered;
-    field_trial_triggered_in_session_ |= triggered;
-  }
-
-  // Clear the previous results now that new results are available.
-  suggest_results->clear();
-  navigation_results->clear();
-
-  base::string16 result, title;
-  std::string type;
-  const base::string16 current_query_string16 =
-      base::ASCIIToUTF16(current_query_);
-  const std::string languages(
-      profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
-  for (size_t index = 0; results->GetString(index, &result); ++index) {
-    // Google search may return empty suggestions for weird input characters,
-    // they make no sense at all and can cause problems in our code.
-    if (result.empty())
-      continue;
-
-    int relevance = kDefaultZeroSuggestRelevance;
-
-    // Apply valid suggested relevance scores; discard invalid lists.
-    if (relevances != NULL && !relevances->GetInteger(index, &relevance))
-      relevances = NULL;
-    if (types && types->GetString(index, &type) && (type == "NAVIGATION")) {
-      // Do not blindly trust the URL coming from the server to be valid.
-      GURL url(URLFixerUpper::FixupURL(
-          base::UTF16ToUTF8(result), std::string()));
-      if (url.is_valid()) {
-        if (descriptions != NULL)
-          descriptions->GetString(index, &title);
-        navigation_results->push_back(NavigationResult(
-            *this, url, title, false, relevance, relevances != NULL,
-            current_query_string16, languages));
-      }
-    } else {
-      suggest_results->push_back(SuggestResult(
-          result, AutocompleteMatchType::SEARCH_SUGGEST, result,
-          base::string16(), std::string(), std::string(), false, relevance,
-          relevances != NULL, false, current_query_string16));
-    }
-  }
-}
-
 void ZeroSuggestProvider::AddSuggestResultsToMap(
     const SuggestResults& results,
     MatchMap* map) {
@@ -352,12 +271,84 @@ void ZeroSuggestProvider::Run(const GURL& suggest_url) {
 }
 
 void ZeroSuggestProvider::ParseSuggestResults(const base::Value& root_val) {
-  SuggestResults suggest_results;
-  FillResults(root_val, &verbatim_relevance_,
-              &suggest_results, &navigation_results_);
+  base::string16 query;
+  const base::ListValue* root_list = NULL;
+  const base::ListValue* results = NULL;
+  const base::ListValue* relevances = NULL;
+  // The response includes the query, which should be empty for ZeroSuggest
+  // responses.
+  if (!root_val.GetAsList(&root_list) || !root_list->GetString(0, &query) ||
+      (!query.empty()) || !root_list->GetList(1, &results))
+    return;
+
+  // 3rd element: Description list.
+  const base::ListValue* descriptions = NULL;
+  root_list->GetList(2, &descriptions);
+
+  // 4th element: Disregard the query URL list for now.
+
+  // Reset suggested relevance information from the provider.
+  results_.verbatim_relevance = -1;
+
+  // 5th element: Optional key-value pairs from the Suggest server.
+  const base::ListValue* types = NULL;
+  const base::DictionaryValue* extras = NULL;
+  if (root_list->GetDictionary(4, &extras)) {
+    extras->GetList("google:suggesttype", &types);
+
+    // Discard this list if its size does not match that of the suggestions.
+    if (extras->GetList("google:suggestrelevance", &relevances) &&
+        relevances->GetSize() != results->GetSize())
+      relevances = NULL;
+    extras->GetInteger("google:verbatimrelevance",
+                       &results_.verbatim_relevance);
+
+    // Check if the active suggest field trial (if any) has triggered.
+    bool triggered = false;
+    extras->GetBoolean("google:fieldtrialtriggered", &triggered);
+    field_trial_triggered_ |= triggered;
+    field_trial_triggered_in_session_ |= triggered;
+  }
+
+  // Clear the previous results now that new results are available.
+  results_.suggest_results.clear();
+  results_.navigation_results.clear();
 
-  query_matches_map_.clear();
-  AddSuggestResultsToMap(suggest_results, &query_matches_map_);
+  base::string16 result, title;
+  std::string type;
+  const base::string16 current_query_string16 =
+      base::ASCIIToUTF16(current_query_);
+  const std::string languages(
+      profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
+  for (size_t index = 0; results->GetString(index, &result); ++index) {
+    // Google search may return empty suggestions for weird input characters,
+    // they make no sense at all and can cause problems in our code.
+    if (result.empty())
+      continue;
+
+    int relevance = kDefaultZeroSuggestRelevance;
+
+    // Apply valid suggested relevance scores; discard invalid lists.
+    if (relevances != NULL && !relevances->GetInteger(index, &relevance))
+      relevances = NULL;
+    if (types && types->GetString(index, &type) && (type == "NAVIGATION")) {
+      // Do not blindly trust the URL coming from the server to be valid.
+      GURL url(URLFixerUpper::FixupURL(
+          base::UTF16ToUTF8(result), std::string()));
+      if (url.is_valid()) {
+        if (descriptions != NULL)
+          descriptions->GetString(index, &title);
+        results_.navigation_results.push_back(NavigationResult(
+            *this, url, title, false, relevance, relevances != NULL,
+            current_query_string16, languages));
+      }
+    } else {
+      results_.suggest_results.push_back(SuggestResult(
+          result, AutocompleteMatchType::SEARCH_SUGGEST, result,
+          base::string16(), std::string(), std::string(), false, relevance,
+          relevances != NULL, false, current_query_string16));
+    }
+  }
 }
 
 void ZeroSuggestProvider::OnMostVisitedUrlsAvailable(
@@ -374,8 +365,11 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   if (default_provider == NULL || !default_provider->SupportsReplacement())
     return;
 
-  const int num_query_results = query_matches_map_.size();
-  const int num_nav_results = navigation_results_.size();
+  MatchMap map;
+  AddSuggestResultsToMap(results_.suggest_results, &map);
+
+  const int num_query_results = map.size();
+  const int num_nav_results = results_.navigation_results.size();
   const int num_results = num_query_results + num_nav_results;
   UMA_HISTOGRAM_COUNTS("ZeroSuggest.QueryResults", num_query_results);
   UMA_HISTOGRAM_COUNTS("ZeroSuggest.URLResults",  num_nav_results);
@@ -413,12 +407,12 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   // current typing in the omnibox.
   matches_.push_back(current_url_match_);
 
-  for (MatchMap::const_iterator it(query_matches_map_.begin());
-       it != query_matches_map_.end(); ++it)
+  for (MatchMap::const_iterator it(map.begin()); it != map.end(); ++it)
     matches_.push_back(it->second);
 
-  for (NavigationResults::const_iterator it(navigation_results_.begin());
-       it != navigation_results_.end(); ++it)
+  const NavigationResults& nav_results(results_.navigation_results);
+  for (NavigationResults::const_iterator it(nav_results.begin());
+       it != nav_results.end(); ++it)
     matches_.push_back(NavigationToMatch(*it));
 }
 
@@ -436,7 +430,12 @@ AutocompleteMatch ZeroSuggestProvider::MatchForCurrentURL() {
   // The placeholder suggestion for the current URL has high relevance so
   // that it is in the first suggestion slot and inline autocompleted. It
   // gets dropped as soon as the user types something.
-  match.relevance = verbatim_relevance_;
+  match.relevance = GetVerbatimRelevance();
 
   return match;
 }
+
+int ZeroSuggestProvider::GetVerbatimRelevance() const {
+  return results_.verbatim_relevance >= 0 ?
+      results_.verbatim_relevance : kDefaultVerbatimZeroSuggestRelevance;
+}
diff --git a/chrome/browser/autocomplete/zero_suggest_provider.h b/chrome/browser/autocomplete/zero_suggest_provider.h
index 01d561b..889a0ff 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.h
+++ b/chrome/browser/autocomplete/zero_suggest_provider.h
@@ -81,20 +81,6 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   virtual void StopSuggest() OVERRIDE;
   virtual void ClearAllResults() 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
-  // ZeroSuggestProvider and SearchProvider.
-
-  // From the OpenSearch formatted response |root_val|, populate query
-  // suggestions into |suggest_results|, navigation suggestions into
-  // |navigation_results|, and the verbatim relevance score into
-  // |verbatim_relevance|.
-  void FillResults(const base::Value& root_val,
-                   int* verbatim_relevance,
-                   SuggestResults* suggest_results,
-                   NavigationResults* navigation_results);
-
   // Adds AutocompleteMatches for each of the suggestions in |results| to
   // |map|.
   void AddSuggestResultsToMap(const SuggestResults& results,
@@ -106,7 +92,7 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   // Fetches zero-suggest suggestions by sending a request using |suggest_url|.
   void Run(const GURL& suggest_url);
 
-  // Parses results from the zero-suggest server and updates results.
+  // Parses results from the zero-suggest server and updates |results_|.
   void ParseSuggestResults(const base::Value& root_val);
 
   // Converts the parsed results to a set of AutocompleteMatches and adds them
@@ -124,6 +110,9 @@ class ZeroSuggestProvider : public BaseSearchProvider {
   // function to return those |urls|.
   void OnMostVisitedUrlsAvailable(const history::MostVisitedURLList& urls);
 
+  // Returns the relevance score for the verbatim result.
+  int GetVerbatimRelevance() const;
+
   // Used to build default search engine URLs for suggested queries.
   TemplateURLService* template_url_service_;
 
@@ -144,12 +133,10 @@ class ZeroSuggestProvider : public BaseSearchProvider {
 
   // Suggestion for the current URL.
   AutocompleteMatch current_url_match_;
-  // Navigation suggestions for the most recent ZeroSuggest input URL.
-  NavigationResults navigation_results_;
-  // Query suggestions for the most recent ZeroSuggest input URL.
-  MatchMap query_matches_map_;
-  // The relevance score for the URL of the current page.
-  int verbatim_relevance_;
+
+  // Contains suggest and navigation results as well as relevance parsed from
+  // the response for the most recent zero suggest input URL.
+  Results results_;
 
   history::MostVisitedURLList most_visited_urls_;