Part 4 of search provider refactoring.

chrome/browser/autocomplete/search_provider.cc

 // Copyright 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/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/callback.h"
@@ skipping 11 matching lines @@
 #include "chrome/browser/autocomplete/autocomplete_result.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
 #include "chrome/browser/autocomplete/url_prefix.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/in_memory_database.h"
 #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"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/user_metrics.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_util.h"
 #include "net/http/http_request_headers.h"
@@ skipping 144 matching lines @@
 };
 
 
 // SearchProvider -------------------------------------------------------------
 
 // static
 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)
     : BaseSearchProvider(listener, profile, AutocompleteProvider::TYPE_SEARCH),
       providers_(TemplateURLServiceFactory::GetForProfile(profile)),
       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);
 }
 
 void SearchProvider::DeleteMatch(const AutocompleteMatch& match) {
   DCHECK(match.deletable);
 
   deletion_handlers_.push_back(new SuggestionDeletionHandler(
       match.GetAdditionalInfo(SearchProvider::kDeletionUrlKey),
       profile_,
@@ skipping 250 matching lines @@
 
     scoped_ptr<base::Value> data(DeserializeJsonData(json_data));
     results_updated = data.get() && ParseSuggestResults(data.get(), is_keyword);
   }
 
   UpdateMatches();
   if (done_ || results_updated)
     listener_->OnProviderUpdate(results_updated);
 }
 
+bool SearchProvider::ShouldAppendExtraParams(
+    const SuggestResult& result) const {
+  return !result.from_keyword_provider() ||
+      providers_.default_provider().empty();

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

does this not fit on one line?

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

83 chars :(

+}
+
 void SearchProvider::OnDeletionComplete(bool success,
                                         SuggestionDeletionHandler* handler) {
   RecordDeletionResult(success);
   SuggestionDeletionHandlers::iterator it = std::find(
       deletion_handlers_.begin(), deletion_handlers_.end(), handler);
   DCHECK(it != deletion_handlers_.end());
   deletion_handlers_.erase(it);
 }
 
 
@@ skipping 493 matching lines @@
   int verbatim_relevance = GetVerbatimRelevance(&relevance_from_server);
   int did_not_accept_default_suggestion =
       default_results_.suggest_results.empty() ?
       TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
   if (verbatim_relevance > 0) {
     SuggestResult verbatim(
         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, input_, GetTemplateURL(verbatim), std::string(),
+        did_not_accept_default_suggestion, &map);
   }
   if (!keyword_input_.text().empty()) {
     const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
     // We only create the verbatim search query match for a keyword
     // if it's not an extension keyword.  Extension keywords are handled
     // in KeywordProvider::Start().  (Extensions are complicated...)
     // Note: in this provider, SEARCH_OTHER_ENGINE must correspond
     // to the keyword verbatim search query.  Do not create other matches
     // of type SEARCH_OTHER_ENGINE.
     if (keyword_url &&
         (keyword_url->GetType() != TemplateURL::OMNIBOX_API_EXTENSION)) {
       bool keyword_relevance_from_server;
       const int keyword_verbatim_relevance =
           GetKeywordVerbatimRelevance(&keyword_relevance_from_server);
       if (keyword_verbatim_relevance > 0) {
         SuggestResult verbatim(
             keyword_input_.text(), AutocompleteMatchType::SEARCH_OTHER_ENGINE,
             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, keyword_input_, GetTemplateURL(verbatim), std::string(),
+            did_not_accept_keyword_suggestion, &map);
       }
     }
   }
   AddHistoryResultsToMap(keyword_history_results_, true,
                          did_not_accept_keyword_suggestion, &map);
   AddHistoryResultsToMap(default_history_results_, false,
                          did_not_accept_default_suggestion, &map);
 
   AddSuggestResultsToMap(keyword_results_.suggest_results,
                          keyword_results_.metadata, &map);
@@ skipping 277 matching lines @@
              AutocompleteResult::kLowestDefaultScore) ||
         !HasMultipleWords(scored_results.front().suggestion()))
       scored_results.clear();  // Didn't detect the case above, score normally.
   }
   if (scored_results.empty())
     scored_results = ScoreHistoryResults(results, prevent_inline_autocomplete,
                                          input_multiple_words, input_text,
                                          is_keyword);
   for (SuggestResults::const_iterator i(scored_results.begin());
        i != scored_results.end(); ++i) {
-    AddMatchToMap(*i, input_text, std::string(),
+    AddMatchToMap(*i, GetInput(*i), GetTemplateURL(*i), std::string(),
                   did_not_accept_suggestion, map);
   }
   UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.AddHistoryResultsTime",
                       base::TimeTicks::Now() - start_time);
 }
 
 SearchProvider::SuggestResults SearchProvider::ScoreHistoryResults(
     const HistoryResults& results,
     bool base_prevent_inline_autocomplete,
     bool input_multiple_words,
@@ skipping 57 matching lines @@
     last_relevance = i->relevance();
   }
 
   return scored_results;
 }
 
 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);
+    const SuggestResult& result = results[i];
+    AddMatchToMap(result, GetInput(result), GetTemplateURL(result),
+                  metadata, i, map);
   }
 }
 
+const AutocompleteInput SearchProvider::GetInput(

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

Could BaseSearchProvider also declare this (and ZeroSuggestProvider also
implement it, or have BaseSearchProvider supply a default impl with a default
AutocompleteInput), so that the corresponding AutocompleteInput arguments could
be removed from CreateSearchSuggestion and AddMatchToMap, like I hope you'll do
with GetTemplateURL?

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

Done.

+    const SuggestResult& result) const {
+  return result.from_keyword_provider() ? keyword_input_ : input_;
+}
+
+const TemplateURL* SearchProvider::GetTemplateURL(
+    const SuggestResult& result) const {
+  return result.from_keyword_provider() ? providers_.GetKeywordProviderURL()
+                                        : providers_.GetDefaultProviderURL();
+}
+
 int SearchProvider::GetVerbatimRelevance(bool* relevance_from_server) const {
   // Use the suggested verbatim relevance score if it is non-negative (valid),
   // if inline autocomplete isn't prevented (always show verbatim on backspace),
   // and if it won't suppress verbatim, leaving no default provider matches.
   // Otherwise, if the default provider returned no matches and was still able
   // to suppress verbatim, the user would have no search/nav matches and may be
   // left unable to search using their default provider from the omnibox.
   // Check for results on each verbatim calculation, as results from older
   // queries (on previous input) may be trimmed for failing to inline new input.
   bool use_server_relevance =
@@ skipping 88 matching lines @@
   // Don't let scores go below 0.  Negative relevance scores are meaningful in
   // a different way.
   int base_score;
   if (is_primary_provider)
     base_score = (input_.type() == AutocompleteInput::URL) ? 750 : 1050;
   else
     base_score = 200;
   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() ?
       keyword_input_.text() : input_.text();
   AutocompleteMatch match(this, navigation.relevance(), false,
                           AutocompleteMatchType::NAVSUGGEST);
   match.destination_url = navigation.url();
 
   // First look for the user's input inside the formatted url as it would be
   // without trimming the scheme, so we can find matches at the beginning of the
@@ skipping 88 matching lines @@
     it->set_relevance(max_query_relevance);
     it->set_relevance_from_server(relevance_from_server);
   }
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on Instant.
   done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0);
 }