Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3655)

Unified Diff: chrome/browser/autocomplete/zero_suggest_provider.cc

Issue 131433003: Refactor search and zero suggest providers to use common base class. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: More style + zero-suggest logic fixes Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/autocomplete/zero_suggest_provider.h ('k') | chrome/chrome_browser.gypi » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 62db13a0f99d8cc07c310794245ce2ee35a70796..a299b006806ed4fa50f3e5c3036b2505491a6dc6 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.cc
+++ b/chrome/browser/autocomplete/zero_suggest_provider.cc
@@ -4,41 +4,33 @@
#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/bind.h"
#include "base/metrics/histogram.h"
+#include "base/metrics/user_metrics_action.h"
#include "base/prefs/pref_service.h"
#include "base/strings/string16.h"
-#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
#include "chrome/browser/autocomplete/autocomplete_classifier.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/autocomplete/autocomplete_input.h"
#include "chrome/browser/autocomplete/autocomplete_match.h"
+#include "chrome/browser/autocomplete/autocomplete_provider.h"
#include "chrome/browser/autocomplete/autocomplete_provider_listener.h"
-#include "chrome/browser/autocomplete/history_url_provider.h"
-#include "chrome/browser/autocomplete/search_provider.h"
-#include "chrome/browser/autocomplete/url_prefix.h"
-#include "chrome/browser/history/history_types.h"
+#include "chrome/browser/autocomplete/base_search_provider.h"
#include "chrome/browser/history/top_sites.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/search.h"
+#include "chrome/browser/search_engines/template_url.h"
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/browser/search_engines/template_url_service_factory.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 "net/base/escape.h"
#include "net/base/load_flags.h"
#include "net/base/net_util.h"
#include "net/http/http_request_headers.h"
-#include "net/http/http_response_headers.h"
#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_status.h"
#include "url/gurl.h"
namespace {
@@ -80,41 +72,6 @@ ZeroSuggestProvider* ZeroSuggestProvider::Create(
return new ZeroSuggestProvider(listener, profile);
}
-void ZeroSuggestProvider::Start(const AutocompleteInput& input,
- bool /*minimal_changes*/) {
-}
-
-void ZeroSuggestProvider::Stop(bool clear_cached_results) {
- if (have_pending_request_)
- LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_INVALIDATED);
- have_pending_request_ = false;
- fetcher_.reset();
- done_ = true;
- if (clear_cached_results) {
- query_matches_map_.clear();
- navigation_results_.clear();
- current_query_.clear();
- matches_.clear();
- }
-}
-
-void ZeroSuggestProvider::AddProviderInfo(ProvidersInfo* provider_info) const {
- provider_info->push_back(metrics::OmniboxEventProto_ProviderInfo());
- metrics::OmniboxEventProto_ProviderInfo& new_entry = provider_info->back();
- new_entry.set_provider(AsOmniboxEventProviderType());
- new_entry.set_provider_done(done_);
- std::vector<uint32> field_trial_hashes;
- OmniboxFieldTrial::GetActiveSuggestFieldTrialHashes(&field_trial_hashes);
- for (size_t i = 0; i < field_trial_hashes.size(); ++i) {
- if (field_trial_triggered_)
- new_entry.mutable_field_trial_triggered()->Add(field_trial_hashes[i]);
- if (field_trial_triggered_in_session_) {
- new_entry.mutable_field_trial_triggered_in_session()->Add(
- field_trial_hashes[i]);
- }
- }
-}
-
void ZeroSuggestProvider::ResetSession() {
// The user has started editing in the omnibox, so leave
// |field_trial_triggered_in_session_| unchanged and set
@@ -123,26 +80,14 @@ void ZeroSuggestProvider::ResetSession() {
Stop(true);
}
-void ZeroSuggestProvider::OnURLFetchComplete(const net::URLFetcher* source) {
- have_pending_request_ = false;
+void ZeroSuggestProvider::LogFetchComplete(const net::URLFetcher* source) {
LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REPLY_RECEIVED);
+}
- std::string json_data;
- source->GetResponseAsString(&json_data);
- const bool request_succeeded =
- source->GetStatus().is_success() && source->GetResponseCode() == 200;
-
- if (request_succeeded) {
- scoped_ptr<base::Value> data(
- SearchProvider::DeserializeJsonData(json_data));
- if (data.get())
- ParseSuggestResults(*data.get());
- }
- done_ = true;
-
- ConvertResultsToAutocompleteMatches();
- if (!matches_.empty())
- listener_->OnProviderUpdate(true);
+bool ZeroSuggestProvider::ShouldSendProviderUpdate(bool results_updated) {
+ // If we have matches, the results must have successfully parsed.
+ DCHECK(!matches_.empty() ? results_updated : true);
+ return !matches_.empty();
}
void ZeroSuggestProvider::StartZeroSuggest(
@@ -166,13 +111,15 @@ void ZeroSuggestProvider::StartZeroSuggest(
search_term_args.current_page_url = current_query_;
GURL suggest_url(default_provider->suggestions_url_ref().
ReplaceSearchTerms(search_term_args));
- if (!SearchProvider::CanSendURL(
- current_page_url, suggest_url,
- template_url_service_->GetDefaultSearchProvider(),
- page_classification, profile_) ||
+ if (!BaseSearchProvider::CanSendURL(
+ current_page_url,
+ suggest_url,
+ template_url_service_->GetDefaultSearchProvider(),
+ page_classification,
+ profile_) ||
!OmniboxFieldTrial::InZeroSuggestFieldTrial())
return;
- verbatim_relevance_ = kDefaultVerbatimZeroSuggestRelevance;
+ results_.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
@@ -180,155 +127,83 @@ void ZeroSuggestProvider::StartZeroSuggest(
Run(suggest_url);
}
-ZeroSuggestProvider::ZeroSuggestProvider(
- AutocompleteProviderListener* listener,
- Profile* profile)
- : AutocompleteProvider(listener, profile,
- AutocompleteProvider::TYPE_ZERO_SUGGEST),
- template_url_service_(TemplateURLServiceFactory::GetForProfile(profile)),
- have_pending_request_(false),
- verbatim_relevance_(kDefaultVerbatimZeroSuggestRelevance),
- field_trial_triggered_(false),
- field_trial_triggered_in_session_(false),
- weak_ptr_factory_(this) {
+void ZeroSuggestProvider::StopSuggest() {
+ if (suggest_results_pending_ > 0)
+ LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_INVALIDATED);
+ suggest_results_pending_ = 0;
+ fetcher_.reset();
}
-ZeroSuggestProvider::~ZeroSuggestProvider() {
+void ZeroSuggestProvider::ClearAllResults() {
+ results_.Clear();
+ current_query_.clear();
+ matches_.clear();
}
-void ZeroSuggestProvider::FillResults(
- const base::Value& root_val,
- int* verbatim_relevance,
- SearchProvider::SuggestResults* suggest_results,
- SearchProvider::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.
+ZeroSuggestProvider::ZeroSuggestProvider(AutocompleteProviderListener* listener,
+ Profile* profile)
+ : BaseSearchProvider(listener,
+ profile,
+ AutocompleteProvider::TYPE_ZERO_SUGGEST),
+ template_url_service_(TemplateURLServiceFactory::GetForProfile(profile)),
+ weak_ptr_factory_(this) {}
- // Reset suggested relevance information from the provider.
- *verbatim_relevance = kDefaultVerbatimZeroSuggestRelevance;
+ZeroSuggestProvider::~ZeroSuggestProvider() {}
- // 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);
+bool ZeroSuggestProvider::IsValidQuery(const base::string16 query,
+ const net::URLFetcher* source) {
+ return query.empty();
+}
- // 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);
+const base::string16 ZeroSuggestProvider::GetInputText(
+ const net::URLFetcher* source) {
+ return base::ASCIIToUTF16(current_query_);
+}
- // 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;
- }
+int ZeroSuggestProvider::GetDefaultRelevance() {
+ return kDefaultZeroSuggestRelevance;
+}
- // Clear the previous results now that new results are available.
- suggest_results->clear();
- navigation_results->clear();
+void ZeroSuggestProvider::UpdateMatches() {
+ done_ = true;
+ ConvertResultsToAutocompleteMatches();
+}
- 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(SearchProvider::NavigationResult(
- *this, url, title, false, relevance, relevances != NULL,
- current_query_string16, languages));
- }
- } else {
- suggest_results->push_back(SearchProvider::SuggestResult(
- result, AutocompleteMatchType::SEARCH_SUGGEST, result,
- base::string16(), std::string(), std::string(), false, relevance,
- relevances != NULL, false, current_query_string16));
- }
- }
+BaseSearchProvider::Results* ZeroSuggestProvider::GetResultsObjectToFill(
+ const net::URLFetcher* source) {
+ return &results_;
}
void ZeroSuggestProvider::AddSuggestResultsToMap(
- const SearchProvider::SuggestResults& results,
+ const SuggestResults& results,
const TemplateURL* template_url,
- SearchProvider::MatchMap* map) {
+ 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());
+ 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,
+ AutocompleteInput(),
+ query_string,
+ template_url,
+ std::string(),
+ i,
+ true,
+ map);
}
}
-void ZeroSuggestProvider::AddMatchToMap(int relevance,
- AutocompleteMatch::Type type,
- const TemplateURL* template_url,
- const base::string16& query_string,
- int accepted_suggestion,
- SearchProvider::MatchMap* map) {
- // Pass in query_string as the input_text to avoid bolding.
- SearchProvider::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 = SearchProvider::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.
- SearchProvider::MatchKey match_key(
- std::make_pair(base::i18n::ToLower(query_string), std::string()));
- const std::pair<SearchProvider::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 SearchProvider::NavigationResult& navigation) {
+ const NavigationResult& navigation) {
AutocompleteMatch match(this, navigation.relevance(), false,
AutocompleteMatchType::NAVSUGGEST);
match.destination_url = navigation.url();
@@ -355,7 +230,7 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
}
void ZeroSuggestProvider::Run(const GURL& suggest_url) {
- have_pending_request_ = false;
+ suggest_results_pending_ = 0;
const int kFetcherID = 1;
fetcher_.reset(
net::URLFetcher::Create(kFetcherID,
@@ -380,19 +255,18 @@ void ZeroSuggestProvider::Run(const GURL& suggest_url) {
weak_ptr_factory_.GetWeakPtr()), false);
}
}
- have_pending_request_ = true;
+ suggest_results_pending_++;
LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_SENT);
}
-void ZeroSuggestProvider::ParseSuggestResults(const base::Value& root_val) {
- SearchProvider::SuggestResults suggest_results;
- FillResults(root_val, &verbatim_relevance_,
- &suggest_results, &navigation_results_);
-
- query_matches_map_.clear();
- AddSuggestResultsToMap(suggest_results,
- template_url_service_->GetDefaultSearchProvider(),
- &query_matches_map_);
+void ZeroSuggestProvider::RecordDeletionResult(bool success) {
+ if (success) {
+ content::RecordAction(
+ base::UserMetricsAction("Omnibox.ServerZeroSuggestDelete.Success"));
+ } else {
+ content::RecordAction(
+ base::UserMetricsAction("Omnibox.ServerZeroSuggestDelete.Failure"));
+ }
}
void ZeroSuggestProvider::OnMostVisitedUrlsAvailable(
@@ -409,8 +283,8 @@ 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();
+ const int num_query_results = results_.suggest_results.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);
@@ -433,9 +307,14 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
for (size_t i = 0; i < most_visited_urls_.size(); i++) {
const history::MostVisitedURL& url = most_visited_urls_[i];
- SearchProvider::NavigationResult nav(
- *this, url.url, url.title, false, relevance, true,
- current_query_string16, languages);
+ NavigationResult nav(*this,
+ url.url,
+ url.title,
+ false,
+ relevance,
+ true,
+ current_query_string16,
+ languages);
matches_.push_back(NavigationToMatch(nav));
--relevance;
}
@@ -449,12 +328,20 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
// current typing in the omnibox.
matches_.push_back(current_url_match_);
- for (SearchProvider::MatchMap::const_iterator it(query_matches_map_.begin());
- it != query_matches_map_.end(); ++it)
+ MatchMap query_matches_map;
+ AddSuggestResultsToMap(results_.suggest_results,
+ template_url_service_->GetDefaultSearchProvider(),
+ &query_matches_map);
+
+ for (MatchMap::const_iterator it(query_matches_map.begin());
+ it != query_matches_map.end();
+ ++it)
matches_.push_back(it->second);
- for (SearchProvider::NavigationResults::const_iterator it(
- navigation_results_.begin()); it != navigation_results_.end(); ++it)
+ for (NavigationResults::const_iterator it(
+ results_.navigation_results.begin());
+ it != results_.navigation_results.end();
+ ++it)
matches_.push_back(NavigationToMatch(*it));
}
@@ -472,7 +359,7 @@ 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 = results_.verbatim_relevance;
return match;
}
« no previous file with comments | « chrome/browser/autocomplete/zero_suggest_provider.h ('k') | chrome/chrome_browser.gypi » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698