Part 6 of search provider refactoring.

chrome/browser/autocomplete/zero_suggest_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/zero_suggest_provider.h"
 
 #include "base/callback.h"
 #include "base/i18n/case_conversion.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/metrics/histogram.h"
@@ skipping 121 matching lines @@
   base::string16 prefix;
   TemplateURLRef::SearchTermsArgs search_term_args(prefix);
   search_term_args.current_page_url = current_query_;
   GURL suggest_url(default_provider->suggestions_url_ref().
                    ReplaceSearchTerms(search_term_args));
   if (!CanSendURL(current_page_url, suggest_url,
           template_url_service_->GetDefaultSearchProvider(),
           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
   // suggestions, if based on local browsing history.
   Run(suggest_url);
 }
 
 ZeroSuggestProvider::ZeroSuggestProvider(
   AutocompleteProviderListener* listener,
   Profile* profile)
     : BaseSearchProvider(listener, profile,
                          AutocompleteProvider::TYPE_ZERO_SUGGEST),
       template_url_service_(TemplateURLServiceFactory::GetForProfile(profile)),
       have_pending_request_(false),
-      verbatim_relevance_(kDefaultVerbatimZeroSuggestRelevance),
       weak_ptr_factory_(this) {
 }
 
 ZeroSuggestProvider::~ZeroSuggestProvider() {
 }
 
 const TemplateURL* ZeroSuggestProvider::GetTemplateURL(
     const SuggestResult& result) const {
   // Zero suggest provider should not receive keyword results.
   DCHECK(!result.from_keyword_provider());
@@ skipping 15 matching lines @@
 }
 
 void ZeroSuggestProvider::StopSuggest() {
   if (have_pending_request_)
     LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_INVALIDATED);
   have_pending_request_ = false;
   fetcher_.reset();
 }
 
 void ZeroSuggestProvider::ClearAllResults() {
-  query_matches_map_.clear();
-  navigation_results_.clear();
+  // We do not call Clear() on |results_| because we use |verbatim_relevance|
+  // value on the next query to set the relevance of the current URL match.

[H Fung, 2014/02/19 06:33:31]

I don't think I understand this comment.

[Maria, 2014/02/19 18:25:53]

Clear() call resets results_.relevance value, but we want to use that value in
StartZeroSuggest() call to create the current url match, hence we omit resetting
it here.

I'll re-word the comment.

[H Fung, 2014/02/19 21:03:16]

I think there might be a bug since the verbatim score should apply to the
current URL, but I'll look into it later.

+  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) {
   for (size_t i = 0; i < results.size(); ++i) {
     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,
@@ skipping 53 matching lines @@
       ts->GetMostVisitedURLs(
           base::Bind(&ZeroSuggestProvider::OnMostVisitedUrlsAvailable,
                      weak_ptr_factory_.GetWeakPtr()), false);
     }
   }
   have_pending_request_ = true;
   LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_SENT);
 }
 
 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;
 
-  query_matches_map_.clear();
-  AddSuggestResultsToMap(suggest_results, &query_matches_map_);
+  // 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;

[H Fung, 2014/02/19 06:33:31]

It seems like verbatim_relevance is only set in this function so that it's
uninitialized if the request failed.  There might not be any noticeable behavior
in that case since there's only the URL suggestion, but it might be better to
make sure it's initialized anyway?

[Maria, 2014/02/19 18:25:53]

It's also initialized in Results constructor to -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();
+
+  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(
     const history::MostVisitedURLList& urls) {
   most_visited_urls_ = urls;
 }
 
 void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   matches_.clear();
 
   const TemplateURL* default_provider =
       template_url_service_->GetDefaultSearchProvider();
   // Fail if we can't set the clickthrough URL for query suggestions.
   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);
   UMA_HISTOGRAM_COUNTS("ZeroSuggest.AllResults", num_results);
 
   // Show Most Visited results after ZeroSuggest response is received.
   if (OmniboxFieldTrial::InZeroSuggestMostVisitedFieldTrial()) {
     if (!current_url_match_.destination_url.is_valid())
       return;
     matches_.push_back(current_url_match_);
@@ skipping 17 matching lines @@
     return;
   }
 
   if (num_results == 0)
     return;
 
   // TODO(jered): Rip this out once the first match is decoupled from the
   // 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));
 }
 
 AutocompleteMatch ZeroSuggestProvider::MatchForCurrentURL() {
   AutocompleteInput input(permanent_text_, base::string16::npos, base::string16(),
                           GURL(current_query_), current_page_classification_,
                           false, false, true, AutocompleteInput::ALL_MATCHES);
 
   AutocompleteMatch match;
   AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(
       permanent_text_, false, true, current_page_classification_, &match, NULL);
   match.is_history_what_you_typed_match = false;
   match.allowed_to_be_default_match = true;
 
   // 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;
+}