diff options
author | bartn@google.com <bartn@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-24 07:53:04 +0000 |
---|---|---|
committer | bartn@google.com <bartn@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-24 07:53:04 +0000 |
commit | bca359bee5cbd5ca3c8452539f9c28c695310d4a (patch) | |
tree | 45a9e4a6ea0d17ffdde8608ccbc9963d83592967 /chrome | |
parent | 908ba14fbcba14d6da3162fcafdacc8b63ed6433 (diff) | |
download | chromium_src-bca359bee5cbd5ca3c8452539f9c28c695310d4a.zip chromium_src-bca359bee5cbd5ca3c8452539f9c28c695310d4a.tar.gz chromium_src-bca359bee5cbd5ca3c8452539f9c28c695310d4a.tar.bz2 |
A working implementation of AQS (Assisted Query Stats).
BUG=132667
TEST=AutocompleteProviderTest::AssistedQueryStats,AutocompleteProviderTest::UpdateAssistedQueryStats, TemplateURLTest::ReplaceAssistedQueryStats, manual testing.
Review URL: https://chromiumcodereview.appspot.com/10537154
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143827 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
21 files changed, 493 insertions, 114 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 4f87500..337241f 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -11,9 +11,11 @@ #include "base/basictypes.h" #include "base/command_line.h" #include "base/i18n/number_formatting.h" +#include "base/format_macros.h" #include "base/metrics/histogram.h" #include "base/string_number_conversions.h" #include "base/string_util.h" +#include "base/stringprintf.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_controller_delegate.h" #include "chrome/browser/autocomplete/autocomplete_match.h" @@ -53,6 +55,7 @@ using base::TimeDelta; + // AutocompleteInput ---------------------------------------------------------- AutocompleteInput::AutocompleteInput() @@ -782,10 +785,15 @@ AutocompleteResult::iterator AutocompleteResult::end() { // Returns the match at the given index. const AutocompleteMatch& AutocompleteResult::match_at(size_t index) const { - DCHECK(index < matches_.size()); + DCHECK_LT(index, matches_.size()); return matches_[index]; } +AutocompleteMatch* AutocompleteResult::match_at(size_t index) { + DCHECK_LT(index, matches_.size()); + return &matches_[index]; +} + void AutocompleteResult::Reset() { matches_.clear(); default_match_ = end(); @@ -852,6 +860,39 @@ void AutocompleteResult::MergeMatchesByProvider(const ACMatches& old_matches, // AutocompleteController ----------------------------------------------------- +namespace { + +// Converts the given type to an integer based on the AQS specification. +// For more details, See http://goto.google.com/binary-clients-logging . +int AutocompleteMatchToAssistedQueryType(const AutocompleteMatch::Type& type) { + switch (type) { + case AutocompleteMatch::SEARCH_SUGGEST: return 0; + case AutocompleteMatch::NAVSUGGEST: return 5; + case AutocompleteMatch::SEARCH_WHAT_YOU_TYPED: return 57; + case AutocompleteMatch::URL_WHAT_YOU_TYPED: return 58; + case AutocompleteMatch::SEARCH_HISTORY: return 59; + case AutocompleteMatch::HISTORY_URL: return 60; + case AutocompleteMatch::HISTORY_TITLE: return 61; + case AutocompleteMatch::HISTORY_BODY: return 62; + case AutocompleteMatch::HISTORY_KEYWORD: return 63; + default: return 64; + } +} + +// Appends available autocompletion of the given type and number to the existing +// available autocompletions string, encoding according to the spec. +void AppendAvailableAutocompletion(int type, + int count, + std::string* autocompletions) { + if (!autocompletions->empty()) + autocompletions->append("j"); + base::StringAppendF(autocompletions, "%d", type); + if (count > 1) + base::StringAppendF(autocompletions, "l%d", count); +} + +} // namespace + const int AutocompleteController::kNoItemSelected = -1; // Amount of time (in ms) between when the user stops typing and when we remove @@ -1044,6 +1085,7 @@ void AutocompleteController::UpdateResult(bool is_synchronous_pass) { UpdateKeywordDescriptions(&result_); UpdateAssociatedKeywords(&result_); + UpdateAssistedQueryStats(&result_); bool notify_default_match = is_synchronous_pass; if (!is_synchronous_pass) { @@ -1100,6 +1142,43 @@ void AutocompleteController::UpdateAssociatedKeywords( } } +void AutocompleteController::UpdateAssistedQueryStats( + AutocompleteResult* result) { + if (result->empty()) + return; + + // Build the impressions string (the AQS part after "."). + std::string autocompletions; + int count = 0; + int last_type = -1; + for (ACMatches::iterator match(result->begin()); match != result->end(); + ++match) { + int type = AutocompleteMatchToAssistedQueryType(match->type); + if (last_type != -1 && type != last_type) { + AppendAvailableAutocompletion(last_type, count, &autocompletions); + count = 1; + } else { + count++; + } + last_type = type; + } + AppendAvailableAutocompletion(last_type, count, &autocompletions); + + // Go over all matches and set AQS if the match supports it. + for (size_t index = 0; index < result->size(); ++index) { + AutocompleteMatch* match = result->match_at(index); + const TemplateURL* template_url = match->GetTemplateURL(profile_); + if (!template_url || !match->search_terms_args.get()) + continue; + match->search_terms_args->assisted_query_stats = + base::StringPrintf("chrome.%" PRIuS ".%s", + index, + autocompletions.c_str()); + match->destination_url = GURL(template_url->url_ref().ReplaceSearchTerms( + *match->search_terms_args)); + } +} + void AutocompleteController::UpdateKeywordDescriptions( AutocompleteResult* result) { string16 last_keyword; diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index c010975..37ca60d 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -562,6 +562,7 @@ class AutocompleteResult { // Returns the match at the given index. const AutocompleteMatch& match_at(size_t index) const; + AutocompleteMatch* match_at(size_t index); // Get the default match for the query (not necessarily the first). Returns // end() if there is no default match. @@ -732,6 +733,7 @@ class AutocompleteController : public ACProviderListener { friend class AutocompleteProviderTest; FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult); + FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, UpdateAssistedQueryStats); // Updates |result_| to reflect the current provider state. Resets timers and // fires notifications as necessary. |is_synchronous_pass| is true only when @@ -747,6 +749,11 @@ class AutocompleteController : public ACProviderListener { // provider name as a description on the first match in the group. void UpdateKeywordDescriptions(AutocompleteResult* result); + // For each AutocompleteMatch returned by SearchProvider, updates the + // destination_url iff the provider's TemplateURL supports assisted query + // stats. + void UpdateAssistedQueryStats(AutocompleteResult* result); + // Calls AutocompleteControllerDelegate::OnResultChanged() and if done sends // AUTOCOMPLETE_CONTROLLER_RESULT_READY. void NotifyChanged(bool notify_default_match); diff --git a/chrome/browser/autocomplete/autocomplete_match.cc b/chrome/browser/autocomplete/autocomplete_match.cc index e5e2cd3..c16405b 100644 --- a/chrome/browser/autocomplete/autocomplete_match.cc +++ b/chrome/browser/autocomplete/autocomplete_match.cc @@ -68,11 +68,14 @@ AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match) transition(match.transition), is_history_what_you_typed_match(match.is_history_what_you_typed_match), type(match.type), + associated_keyword(match.associated_keyword.get() ? + new AutocompleteMatch(*match.associated_keyword) : NULL), keyword(match.keyword), starred(match.starred), - from_previous(match.from_previous) { - if (match.associated_keyword.get()) - associated_keyword.reset(new AutocompleteMatch(*match.associated_keyword)); + from_previous(match.from_previous), + search_terms_args(match.search_terms_args.get() ? + new TemplateURLRef::SearchTermsArgs(*match.search_terms_args) : + NULL) { } AutocompleteMatch::~AutocompleteMatch() { @@ -103,7 +106,8 @@ AutocompleteMatch& AutocompleteMatch::operator=( keyword = match.keyword; starred = match.starred; from_previous = match.from_previous; - + search_terms_args.reset(match.search_terms_args.get() ? + new TemplateURLRef::SearchTermsArgs(*match.search_terms_args) : NULL); return *this; } diff --git a/chrome/browser/autocomplete/autocomplete_match.h b/chrome/browser/autocomplete/autocomplete_match.h index e26e347..55932e0 100644 --- a/chrome/browser/autocomplete/autocomplete_match.h +++ b/chrome/browser/autocomplete/autocomplete_match.h @@ -10,6 +10,7 @@ #include <string> #include "base/memory/scoped_ptr.h" +#include "chrome/browser/search_engines/template_url.h" #include "content/public/common/page_transition_types.h" #include "googleurl/src/gurl.h" @@ -274,6 +275,15 @@ struct AutocompleteMatch { // True if this match is from a previous result. bool from_previous; + // Optional search terms args. If present, + // AutocompleteController::UpdateAssistedQueryStats() will incorporate this + // data with additional data it calculates and pass the completed struct to + // TemplateURLRef::ReplaceSearchTerms() to reset the match's |destination_url| + // after the complete set of matches in the AutocompleteResult has been chosen + // and sorted. Most providers will leave this as NULL, which will cause the + // AutocompleteController to do no additional transformations. + scoped_ptr<TemplateURLRef::SearchTermsArgs> search_terms_args; + #ifndef NDEBUG // Does a data integrity check on this match. void Validate() const; diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc index 61d984f..18eeb44 100644 --- a/chrome/browser/autocomplete/autocomplete_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_unittest.cc @@ -31,16 +31,20 @@ static std::ostream& operator<<(std::ostream& os, namespace { const size_t kResultsPerProvider = 3; +const char kTestTemplateURLKeyword[] = "t"; } // Autocomplete provider that provides known results. Note that this is // refcounted so that it can also be a task on the message loop. class TestProvider : public AutocompleteProvider { public: - TestProvider(int relevance, const string16& prefix) - : AutocompleteProvider(NULL, NULL, ""), + TestProvider(int relevance, const string16& prefix, + Profile* profile, + const string16 match_keyword) + : AutocompleteProvider(NULL, profile, ""), relevance_(relevance), - prefix_(prefix) { + prefix_(prefix), + match_keyword_(match_keyword) { } virtual void Start(const AutocompleteInput& input, @@ -56,9 +60,15 @@ class TestProvider : public AutocompleteProvider { void Run(); void AddResults(int start_at, int num); + void AddResultsWithSearchTermsArgs( + int start_at, + int num, + AutocompleteMatch::Type type, + const TemplateURLRef::SearchTermsArgs& search_terms_args); int relevance_; const string16 prefix_; + const string16 match_keyword_; }; void TestProvider::Start(const AutocompleteInput& input, @@ -68,8 +78,17 @@ void TestProvider::Start(const AutocompleteInput& input, matches_.clear(); - // Generate one result synchronously, the rest later. + // Generate 4 results synchronously, the rest later. AddResults(0, 1); + AddResultsWithSearchTermsArgs( + 1, 1, AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("echo"))); + AddResultsWithSearchTermsArgs( + 2, 1, AutocompleteMatch::NAVSUGGEST, + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("nav"))); + AddResultsWithSearchTermsArgs( + 3, 1, AutocompleteMatch::SEARCH_SUGGEST, + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("query"))); if (input.matches_requested() == AutocompleteInput::ALL_MATCHES) { done_ = false; @@ -87,9 +106,19 @@ void TestProvider::Run() { } void TestProvider::AddResults(int start_at, int num) { + AddResultsWithSearchTermsArgs(start_at, + num, + AutocompleteMatch::URL_WHAT_YOU_TYPED, + TemplateURLRef::SearchTermsArgs(string16())); +} + +void TestProvider::AddResultsWithSearchTermsArgs( + int start_at, + int num, + AutocompleteMatch::Type type, + const TemplateURLRef::SearchTermsArgs& search_terms_args) { for (int i = start_at; i < num; i++) { - AutocompleteMatch match(this, relevance_ - i, false, - AutocompleteMatch::URL_WHAT_YOU_TYPED); + AutocompleteMatch match(this, relevance_ - i, false, type); match.fill_into_edit = prefix_ + UTF8ToUTF16(base::IntToString(i)); match.destination_url = GURL(UTF16ToUTF8(match.fill_into_edit)); @@ -100,6 +129,12 @@ void TestProvider::AddResults(int start_at, int num) { match.description = match.fill_into_edit; match.description_class.push_back( ACMatchClassification(0, ACMatchClassification::NONE)); + match.search_terms_args.reset( + new TemplateURLRef::SearchTermsArgs(search_terms_args)); + if (!match_keyword_.empty()) { + match.keyword = match_keyword_; + ASSERT_TRUE(match.GetTemplateURL(profile_) != NULL); + } matches_.push_back(match); } @@ -114,7 +149,16 @@ class AutocompleteProviderTest : public testing::Test, const bool expected_keyword_result; }; + struct AssistedQueryStatsTestData { + const AutocompleteMatch::Type match_type; + const std::string expected_aqs; + }; + protected: + // Registers a test TemplateURL under the given keyword. + void RegisterTemplateURL(const string16 keyword, + const std::string& template_url); + void ResetControllerWithTestProviders(bool same_destinations); // Runs a query on the input "a", and makes sure both providers' input is @@ -123,6 +167,10 @@ class AutocompleteProviderTest : public testing::Test, void RunRedundantKeywordTest(const KeywordTestData* match_data, size_t size); + void RunAssistedQueryStatsTest( + const AssistedQueryStatsTestData* aqs_test_data, + size_t size); + void RunQuery(const string16 query); void ResetControllerWithTestProvidersWithKeywordAndSearchProviders(); @@ -146,20 +194,56 @@ class AutocompleteProviderTest : public testing::Test, TestingProfile profile_; }; +void AutocompleteProviderTest:: RegisterTemplateURL( + const string16 keyword, + const std::string& template_url) { + TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse( + &profile_, &TemplateURLServiceFactory::BuildInstanceFor); + TemplateURLData data; + data.SetURL(template_url); + data.SetKeyword(keyword); + TemplateURL* default_t_url = new TemplateURL(&profile_, data); + TemplateURLService* turl_model = + TemplateURLServiceFactory::GetForProfile(&profile_); + turl_model->Add(default_t_url); + turl_model->SetDefaultSearchProvider(default_t_url); + TemplateURLID default_provider_id = default_t_url->id(); + ASSERT_NE(0, default_provider_id); +} + void AutocompleteProviderTest::ResetControllerWithTestProviders( bool same_destinations) { // Forget about any existing providers. The controller owns them and will // Release() them below, when we delete it during the call to reset(). providers_.clear(); + // TODO: Move it outside this method, after refactoring the existing + // unit tests. Specifically: + // (1) Make sure that AutocompleteMatch.keyword is set iff there is + // a corresponding call to RegisterTemplateURL; otherwise the + // controller flow will crash; this practically means that + // RunTests/ResetControllerXXX/RegisterTemplateURL should + // be coordinated with each other. + // (2) Inject test arguments rather than rely on the hardcoded values, e.g. + // don't rely on kResultsPerProvided and default relevance ordering + // (B > A). + RegisterTemplateURL(ASCIIToUTF16(kTestTemplateURLKeyword), + "http://aqs/{searchTerms}/{google:assistedQueryStats}"); + // Construct two new providers, with either the same or different prefixes. - TestProvider* providerA = new TestProvider(kResultsPerProvider, - ASCIIToUTF16("http://a")); + TestProvider* providerA = + new TestProvider(kResultsPerProvider, + ASCIIToUTF16("http://a"), + &profile_, + ASCIIToUTF16(kTestTemplateURLKeyword)); providerA->AddRef(); providers_.push_back(providerA); - TestProvider* providerB = new TestProvider(kResultsPerProvider * 2, - same_destinations ? ASCIIToUTF16("http://a") : ASCIIToUTF16("http://b")); + TestProvider* providerB = new TestProvider( + kResultsPerProvider * 2, + same_destinations ? ASCIIToUTF16("http://a") : ASCIIToUTF16("http://b"), + &profile_, + string16()); providerB->AddRef(); providers_.push_back(providerB); @@ -217,8 +301,7 @@ void AutocompleteProviderTest:: controller_.reset(new AutocompleteController(providers_, &profile_)); } -void AutocompleteProviderTest:: - ResetControllerWithKeywordProvider() { +void AutocompleteProviderTest::ResetControllerWithKeywordProvider() { TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse( &profile_, &TemplateURLServiceFactory::BuildInstanceFor); @@ -280,10 +363,36 @@ void AutocompleteProviderTest::RunRedundantKeywordTest( for (size_t j = 0; j < result.size(); ++j) { EXPECT_EQ(match_data[j].expected_keyword_result, - result.match_at(j).associated_keyword.get() != NULL); + result.match_at(j)->associated_keyword.get() != NULL); } } +void AutocompleteProviderTest::RunAssistedQueryStatsTest( + const AssistedQueryStatsTestData* aqs_test_data, + size_t size) { + // Prepare input. + const size_t kMaxRelevance = 1000; + ACMatches matches; + for (size_t i = 0; i < size; ++i) { + AutocompleteMatch match(NULL, kMaxRelevance - i, false, + aqs_test_data[i].match_type); + match.keyword = ASCIIToUTF16(kTestTemplateURLKeyword); + match.search_terms_args.reset( + new TemplateURLRef::SearchTermsArgs(string16())); + matches.push_back(match); + } + result_.Reset(); + result_.AppendMatches(matches); + + // Update AQS. + controller_->UpdateAssistedQueryStats(&result_); + + // Verify data. + for (size_t i = 0; i < size; ++i) { + EXPECT_EQ(aqs_test_data[i].expected_aqs, + result_.match_at(i)->search_terms_args->assisted_query_stats); + } +} void AutocompleteProviderTest::RunQuery(const string16 query) { result_.Reset(); @@ -335,6 +444,26 @@ TEST_F(AutocompleteProviderTest, Query) { EXPECT_EQ(providers_[1], result_.default_match()->provider); } +// Tests assisted query stats. +TEST_F(AutocompleteProviderTest, AssistedQueryStats) { + ResetControllerWithTestProviders(false); + RunTest(); + + EXPECT_EQ(kResultsPerProvider * 2, result_.size()); // two providers + + // Now, check the results from the second provider, as they should not have + // assisted query stats set. + for (size_t i = 0; i < kResultsPerProvider; ++i) { + EXPECT_TRUE( + result_.match_at(i)->search_terms_args->assisted_query_stats.empty()); + } + // The first provider has a test keyword, so AQS should be non-empty. + for (size_t i = kResultsPerProvider; i < kResultsPerProvider * 2; ++i) { + EXPECT_FALSE( + result_.match_at(i)->search_terms_args->assisted_query_stats.empty()); + } +} + TEST_F(AutocompleteProviderTest, RemoveDuplicates) { ResetControllerWithTestProviders(true); RunTest(); @@ -395,6 +524,43 @@ TEST_F(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult) { } } +TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) { + ResetControllerWithTestProviders(false); + + { + AssistedQueryStatsTestData test_data[] = { + // MSVC doesn't support zero-length arrays, so supply some dummy data. + { AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, "" } + }; + SCOPED_TRACE("No matches"); + // Note: We pass 0 here to ignore the dummy data above. + RunAssistedQueryStatsTest(test_data, 0); + } + + { + AssistedQueryStatsTestData test_data[] = { + { AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, "chrome.0.57" } + }; + SCOPED_TRACE("One match"); + RunAssistedQueryStatsTest(test_data, ARRAYSIZE_UNSAFE(test_data)); + } + + { + AssistedQueryStatsTestData test_data[] = { + { AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, "chrome.0.57j58j5l2j0l3j59" }, + { AutocompleteMatch::URL_WHAT_YOU_TYPED, "chrome.1.57j58j5l2j0l3j59" }, + { AutocompleteMatch::NAVSUGGEST, "chrome.2.57j58j5l2j0l3j59" }, + { AutocompleteMatch::NAVSUGGEST, "chrome.3.57j58j5l2j0l3j59" }, + { AutocompleteMatch::SEARCH_SUGGEST, "chrome.4.57j58j5l2j0l3j59" }, + { AutocompleteMatch::SEARCH_SUGGEST, "chrome.5.57j58j5l2j0l3j59" }, + { AutocompleteMatch::SEARCH_SUGGEST, "chrome.6.57j58j5l2j0l3j59" }, + { AutocompleteMatch::SEARCH_HISTORY, "chrome.7.57j58j5l2j0l3j59" }, + }; + SCOPED_TRACE("Multiple matches"); + RunAssistedQueryStatsTest(test_data, ARRAYSIZE_UNSAFE(test_data)); + } +} + typedef testing::Test AutocompleteTest; TEST_F(AutocompleteTest, InputType) { diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc index fdd5489..b50720f 100644 --- a/chrome/browser/autocomplete/keyword_provider.cc +++ b/chrome/browser/autocomplete/keyword_provider.cc @@ -396,7 +396,7 @@ void KeywordProvider::FillInURLAndContents( // fixup to make the URL valid if necessary. DCHECK(element_ref.SupportsReplacement()); match->destination_url = GURL(element_ref.ReplaceSearchTerms( - remaining_input, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + TemplateURLRef::SearchTermsArgs(remaining_input))); std::vector<size_t> content_param_offsets; match->contents.assign(l10n_util::GetStringFUTF16(message_id, element->short_name(), diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 6b25b1e..5c314df 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -593,8 +593,8 @@ net::URLFetcher* SearchProvider::CreateSuggestFetcher( const string16& text) { DCHECK(suggestions_url.SupportsReplacement()); net::URLFetcher* fetcher = net::URLFetcher::Create(id, - GURL(suggestions_url.ReplaceSearchTerms(text, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())), + GURL(suggestions_url.ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(text))), net::URLFetcher::GET, this); fetcher->SetRequestContext(profile_->GetRequestContext()); fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES); @@ -1091,8 +1091,15 @@ void SearchProvider::AddMatchToMap(const string16& query_string, const TemplateURLRef& search_url = provider_url->url_ref(); DCHECK(search_url.SupportsReplacement()); - match.destination_url = GURL(search_url.ReplaceSearchTerms(query_string, - accepted_suggestion, input_text)); + match.search_terms_args.reset( + new TemplateURLRef::SearchTermsArgs(query_string)); + match.search_terms_args->original_query = input_text; + match.search_terms_args->accepted_suggestion = accepted_suggestion; + // This is the destination URL sans assisted query stats. This must be set + // so the AutocompleteController can properly de-dupe; the controller will + // eventually overwrite it before it reaches the user. + match.destination_url = + GURL(search_url.ReplaceSearchTerms(*match.search_terms_args.get())); // Search results don't look like URLs. match.transition = is_keyword ? diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 5e020e7..79e6582 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -211,8 +211,9 @@ void SearchProviderTest::QueryForInputAndSetWYTMatch( return; ASSERT_GE(provider_->matches().size(), 1u); EXPECT_TRUE(FindMatchWithDestination(GURL( - default_t_url_->url_ref().ReplaceSearchTerms(text, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())), wyt_match)); + default_t_url_->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(text))), + wyt_match)); } void SearchProviderTest::TearDown() { @@ -228,8 +229,8 @@ GURL SearchProviderTest::AddSearchToHistory(TemplateURL* t_url, HistoryService* history = HistoryServiceFactory::GetForProfile(&profile_, Profile::EXPLICIT_ACCESS); - GURL search(t_url->url_ref().ReplaceSearchTerms(term, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL search(t_url->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(term))); static base::Time last_added_time; last_added_time = std::max(base::Time::Now(), last_added_time + base::TimeDelta::FromMicroseconds(1)); @@ -288,7 +289,7 @@ TEST_F(SearchProviderTest, QueryDefaultProvider) { // And the URL matches what we expected. GURL expected_url(default_t_url_->suggestions_url_ref().ReplaceSearchTerms( - term, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + TemplateURLRef::SearchTermsArgs(term))); ASSERT_TRUE(fetcher->GetOriginalURL() == expected_url); // Tell the SearchProvider the suggest query is done. @@ -308,8 +309,8 @@ TEST_F(SearchProviderTest, QueryDefaultProvider) { AutocompleteMatch wyt_match; EXPECT_TRUE(FindMatchWithDestination( - GURL(default_t_url_->url_ref().ReplaceSearchTerms(term, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())), &wyt_match)); + GURL(default_t_url_->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(term))), &wyt_match)); EXPECT_TRUE(wyt_match.description.empty()); // The match for term1 should be more relevant than the what you typed result. @@ -349,7 +350,7 @@ TEST_F(SearchProviderTest, QueryKeywordProvider) { // And the URL matches what we expected. GURL expected_url(keyword_t_url_->suggestions_url_ref().ReplaceSearchTerms( - term, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + TemplateURLRef::SearchTermsArgs(term))); ASSERT_TRUE(keyword_fetcher->GetOriginalURL() == expected_url); // Tell the SearchProvider the keyword suggest query is done. @@ -418,8 +419,7 @@ TEST_F(SearchProviderTest, FinalizeInstantQuery) { // 'foobar'. EXPECT_EQ(2u, provider_->matches().size()); GURL instant_url(default_t_url_->url_ref().ReplaceSearchTerms( - ASCIIToUTF16("foobar"), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, - string16())); + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foobar")))); AutocompleteMatch instant_match; EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); @@ -429,8 +429,9 @@ TEST_F(SearchProviderTest, FinalizeInstantQuery) { // Make sure the what you typed match has no description. AutocompleteMatch wyt_match; EXPECT_TRUE(FindMatchWithDestination( - GURL(default_t_url_->url_ref().ReplaceSearchTerms(ASCIIToUTF16("foo"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())), &wyt_match)); + GURL(default_t_url_->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foo")))), + &wyt_match)); EXPECT_TRUE(wyt_match.description.empty()); // The instant search should be more relevant. @@ -452,8 +453,7 @@ TEST_F(SearchProviderTest, RememberInstantQuery) { // 'foobar'. EXPECT_EQ(2u, provider_->matches().size()); GURL instant_url(default_t_url_->url_ref().ReplaceSearchTerms( - ASCIIToUTF16("foobar"), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, - string16())); + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foobar")))); AutocompleteMatch instant_match; EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); diff --git a/chrome/browser/importer/profile_writer.cc b/chrome/browser/importer/profile_writer.cc index 18a5201..33f43745 100644 --- a/chrome/browser/importer/profile_writer.cc +++ b/chrome/browser/importer/profile_writer.cc @@ -263,8 +263,8 @@ static std::string BuildHostPathKey(const TemplateURL* t_url, if (t_url->url_ref().SupportsReplacement()) { return HostPathKeyForURL(GURL( - t_url->url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16()))); + t_url->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x"))))); } return std::string(); } diff --git a/chrome/browser/instant/instant_browsertest.cc b/chrome/browser/instant/instant_browsertest.cc index 1ed088b..be26992 100644 --- a/chrome/browser/instant/instant_browsertest.cc +++ b/chrome/browser/instant/instant_browsertest.cc @@ -289,8 +289,8 @@ IN_PROC_BROWSER_TEST_F(InstantTest, MAYBE(OnChangeEvent)) { TemplateURLServiceFactory::GetForProfile(browser()->profile())-> GetDefaultSearchProvider(); EXPECT_TRUE(default_turl); - EXPECT_EQ(default_turl->url_ref().ReplaceSearchTerms(ASCIIToUTF16("defghi"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16()), + EXPECT_EQ(default_turl->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("defghi"))), loader()->url().spec()); // Check that the value is reflected and onchange is called. diff --git a/chrome/browser/instant/instant_loader.cc b/chrome/browser/instant/instant_loader.cc index 460f343..402e8d2 100644 --- a/chrome/browser/instant/instant_loader.cc +++ b/chrome/browser/instant/instant_loader.cc @@ -1159,7 +1159,7 @@ void InstantLoader::LoadInstantURL(const TemplateURL* template_url, // correctly. // TODO(sky): having to use a replaceable url is a bit of a hack here. GURL instant_url(template_url->instant_url_ref().ReplaceSearchTerms( - string16(), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + TemplateURLRef::SearchTermsArgs(string16()))); CommandLine* cl = CommandLine::ForCurrentProcess(); if (cl->HasSwitch(switches::kInstantURL)) instant_url = GURL(cl->GetSwitchValueASCII(switches::kInstantURL)); diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc index 1d438e1..ad96e0e 100644 --- a/chrome/browser/search_engines/template_url.cc +++ b/chrome/browser/search_engines/template_url.cc @@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/metrics/field_trial.h" #include "base/string_number_conversions.h" +#include "base/string_util.h" #include "base/stringprintf.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_field_trial.h" @@ -42,6 +43,7 @@ const char kInputEncodingParameter[] = "inputEncoding"; const char kOutputEncodingParameter[] = "outputEncoding"; const char kGoogleAcceptedSuggestionParameter[] = "google:acceptedSuggestion"; +const char kGoogleAssistedQueryStatsParameter[] = "google:assistedQueryStats"; // Host/Domain Google searches are relative to. const char kGoogleBaseURLParameter[] = "google:baseURL"; const char kGoogleBaseURLParameterFull[] = "{google:baseURL}"; @@ -106,6 +108,14 @@ bool TryEncoding(const string16& terms, } // namespace +// TemplateURLRef::SearchTermsArgs -------------------------------------------- + +TemplateURLRef::SearchTermsArgs::SearchTermsArgs(const string16& search_terms) + : search_terms(search_terms), + accepted_suggestion(NO_SUGGESTIONS_AVAILABLE) { +} + + // TemplateURLRef ------------------------------------------------------------- TemplateURLRef::TemplateURLRef(TemplateURL* owner, Type type) @@ -142,18 +152,13 @@ bool TemplateURLRef::SupportsReplacementUsingTermsData( } std::string TemplateURLRef::ReplaceSearchTerms( - const string16& terms, - int accepted_suggestion, - const string16& original_query_for_suggestion) const { + const SearchTermsArgs& search_terms_args) const { UIThreadSearchTermsData search_terms_data(owner_->profile()); - return ReplaceSearchTermsUsingTermsData(terms, accepted_suggestion, - original_query_for_suggestion, search_terms_data); + return ReplaceSearchTermsUsingTermsData(search_terms_args, search_terms_data); } std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( - const string16& terms, - int accepted_suggestion, - const string16& original_query_for_suggestion, + const SearchTermsArgs& search_terms_args, const SearchTermsData& search_terms_data) const { ParseIfNecessaryUsingTermsData(search_terms_data); if (!valid_) @@ -182,7 +187,8 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( for (std::vector<std::string>::const_iterator i( owner_->input_encodings().begin()); i != owner_->input_encodings().end(); ++i) { - if (TryEncoding(terms, original_query_for_suggestion, i->c_str(), + if (TryEncoding(search_terms_args.search_terms, + search_terms_args.original_query, i->c_str(), is_in_query, &encoded_terms, &encoded_original_query)) { input_encoding = *i; break; @@ -190,7 +196,8 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( } if (input_encoding.empty()) { input_encoding = "UTF-8"; - if (!TryEncoding(terms, original_query_for_suggestion, + if (!TryEncoding(search_terms_args.search_terms, + search_terms_args.original_query, input_encoding.c_str(), is_in_query, &encoded_terms, &encoded_original_query)) NOTREACHED(); @@ -207,12 +214,32 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( url.insert(i->index, input_encoding); break; + case GOOGLE_ASSISTED_QUERY_STATS: + if (!search_terms_args.assisted_query_stats.empty()) { + // Get the base URL without substituting AQS to avoid infinite + // recursion. We need the URL to find out if it meets all + // AQS requirements (e.g. HTTPS protocol check). + // See TemplateURLRef::SearchTermsArgs for more details. + SearchTermsArgs search_terms_args_without_aqs(search_terms_args); + search_terms_args_without_aqs.assisted_query_stats.clear(); + GURL base_url(ReplaceSearchTermsUsingTermsData( + search_terms_args_without_aqs, search_terms_data)); + if (base_url.SchemeIs(chrome::kHttpsScheme)) { + url.insert(i->index, + "aqs=" + search_terms_args.assisted_query_stats + "&"); + } + } + break; + case GOOGLE_ACCEPTED_SUGGESTION: - if (accepted_suggestion == NO_SUGGESTION_CHOSEN) + if (search_terms_args.accepted_suggestion == NO_SUGGESTION_CHOSEN) { url.insert(i->index, "aq=f&"); - else if (accepted_suggestion != NO_SUGGESTIONS_AVAILABLE) + } else if (search_terms_args.accepted_suggestion != + NO_SUGGESTIONS_AVAILABLE) { url.insert(i->index, - base::StringPrintf("aq=%d&", accepted_suggestion)); + base::StringPrintf("aq=%d&", + search_terms_args.accepted_suggestion)); + } break; case GOOGLE_BASE_URL: @@ -228,9 +255,11 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( break; case GOOGLE_ORIGINAL_QUERY_FOR_SUGGESTION: - if (accepted_suggestion >= 0) + if (search_terms_args.accepted_suggestion >= 0 || + !search_terms_args.assisted_query_stats.empty()) { url.insert(i->index, "oq=" + UTF16ToUTF8(encoded_original_query) + "&"); + } break; case GOOGLE_RLZ: { @@ -257,7 +286,8 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( case GOOGLE_UNESCAPED_SEARCH_TERMS: { std::string unescaped_terms; - base::UTF16ToCodepage(terms, input_encoding.c_str(), + base::UTF16ToCodepage(search_terms_args.search_terms, + input_encoding.c_str(), base::OnStringConversionError::SKIP, &unescaped_terms); url.insert(i->index, std::string(unescaped_terms.begin(), @@ -417,6 +447,8 @@ bool TemplateURLRef::ParseParameter(size_t start, url->insert(start, kOutputEncodingType); } else if (parameter == kGoogleAcceptedSuggestionParameter) { replacements->push_back(Replacement(GOOGLE_ACCEPTED_SUGGESTION, start)); + } else if (parameter == kGoogleAssistedQueryStatsParameter) { + replacements->push_back(Replacement(GOOGLE_ASSISTED_QUERY_STATS, start)); } else if (parameter == kGoogleBaseURLParameter) { replacements->push_back(Replacement(GOOGLE_BASE_URL, start)); } else if (parameter == kGoogleBaseSuggestURLParameter) { diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h index 3264220..9468fcc 100644 --- a/chrome/browser/search_engines/template_url.h +++ b/chrome/browser/search_engines/template_url.h @@ -48,6 +48,28 @@ class TemplateURLRef { INSTANT, }; + // This struct encapsulates arguments passed to + // TemplateURLRef::ReplaceSearchTerms methods. By default, only search_terms + // is required and is passed in the constructor. + struct SearchTermsArgs { + explicit SearchTermsArgs(const string16& search_terms); + + // The search terms (query). + const string16 search_terms; + // The original (input) query. + string16 original_query; + // The optional assisted query stats, aka AQS, used for logging purposes. + // This string contains impressions of all autocomplete matches shown + // at the query submission time. For privacy reasons, we require the + // search provider to support HTTPS protocol in order to receive the AQS + // param. + // For more details, see http://goto.google.com/binary-clients-logging . + std::string assisted_query_stats; + + // TODO: Remove along with "aq" CGI param. + int accepted_suggestion; + }; + TemplateURLRef(TemplateURL* owner, Type type); ~TemplateURLRef(); @@ -62,22 +84,18 @@ class TemplateURLRef { const SearchTermsData& search_terms_data) const; // Returns a string that is the result of replacing the search terms in - // the url with the specified value. We use our owner's input encoding. + // the url with the specified arguments. We use our owner's input encoding. // // If this TemplateURLRef does not support replacement (SupportsReplacement // returns false), an empty string is returned. std::string ReplaceSearchTerms( - const string16& terms, - int accepted_suggestion, - const string16& original_query_for_suggestion) const; + const SearchTermsArgs& search_terms_args) const; // Just like ReplaceSearchTerms except that it takes SearchTermsData to supply // the data for some search terms. Most of the time ReplaceSearchTerms should // be called. std::string ReplaceSearchTermsUsingTermsData( - const string16& terms, - int accepted_suggestion, - const string16& original_query_for_suggestion, + const SearchTermsArgs& search_terms_args, const SearchTermsData& search_terms_data) const; // Returns true if the TemplateURLRef is valid. An invalid TemplateURLRef is @@ -126,6 +144,7 @@ class TemplateURLRef { enum ReplacementType { ENCODING, GOOGLE_ACCEPTED_SUGGESTION, + GOOGLE_ASSISTED_QUERY_STATS, GOOGLE_BASE_URL, GOOGLE_BASE_SUGGEST_URL, GOOGLE_INSTANT_ENABLED, diff --git a/chrome/browser/search_engines/template_url_prepopulate_data.cc b/chrome/browser/search_engines/template_url_prepopulate_data.cc index 7eccfdb..77c4fcd 100644 --- a/chrome/browser/search_engines/template_url_prepopulate_data.cc +++ b/chrome/browser/search_engines/template_url_prepopulate_data.cc @@ -1091,7 +1091,8 @@ const PrepopulatedEngine google = { "http://www.google.com/favicon.ico", "{google:baseURL}search?q={searchTerms}&{google:RLZ}" "{google:acceptedSuggestion}{google:originalQueryForSuggestion}" - "{google:searchFieldtrialParameter}sourceid=chrome&ie={inputEncoding}", + "{google:assistedQueryStats}{google:searchFieldtrialParameter}" + "sourceid=chrome&ie={inputEncoding}", "UTF-8", "{google:baseSuggestURL}search?{google:searchFieldtrialParameter}" "client=chrome&hl={language}&q={searchTerms}", @@ -3418,8 +3419,8 @@ SearchEngineType GetEngineType(const std::string& url) { TemplateURLData data; data.SetURL(url); TemplateURL turl(NULL, data); - GURL as_gurl(turl.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL as_gurl(turl.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")))); if (!as_gurl.is_valid()) return SEARCH_ENGINE_OTHER; diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc index 0f5e5a2..dc8b522 100644 --- a/chrome/browser/search_engines/template_url_service.cc +++ b/chrome/browser/search_engines/template_url_service.cc @@ -318,8 +318,8 @@ GURL TemplateURLService::GenerateSearchURLUsingTermsData( return GURL(t_url->url()); return GURL(search_ref.ReplaceSearchTermsUsingTermsData( - ASCIIToUTF16(kReplacementTerm), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, - string16(), search_terms_data)); + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16(kReplacementTerm)), + search_terms_data)); } bool TemplateURLService::CanReplaceKeyword( diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc index 92bdb37..316d7a1 100644 --- a/chrome/browser/search_engines/template_url_service_unittest.cc +++ b/chrome/browser/search_engines/template_url_service_unittest.cc @@ -1014,7 +1014,7 @@ TEST_F(TemplateURLServiceTest, ChangeGoogleBaseValue) { EXPECT_EQ("google.co.uk", t_url->url_ref().GetHost()); EXPECT_EQ(ASCIIToUTF16("google.co.uk"), t_url->keyword()); EXPECT_EQ("http://google.co.uk/?q=x", t_url->url_ref().ReplaceSearchTerms( - ASCIIToUTF16("x"), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")))); // Now add a manual entry and then change the Google base URL such that the // autogenerated Google search keyword would conflict. @@ -1062,8 +1062,8 @@ TEST_F(TemplateURLServiceTest, GenerateVisitOnKeyword) { HistoryServiceFactory::GetForProfile(test_util_.profile(), Profile::EXPLICIT_ACCESS); history->AddPage( - GURL(t_url->url_ref().ReplaceSearchTerms(ASCIIToUTF16("blah"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())), + GURL(t_url->url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("blah")))), NULL, 0, GURL(), content::PAGE_TRANSITION_KEYWORD, history::RedirectList(), history::SOURCE_BROWSED, false); diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc index 42aa172..089c1a1 100644 --- a/chrome/browser/search_engines/template_url_unittest.cc +++ b/chrome/browser/search_engines/template_url_unittest.cc @@ -93,8 +93,8 @@ TEST_F(TemplateURLTest, URLRefTestSearchTerms) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(value.terms, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(value.terms))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ(value.output, result.spec()); } @@ -106,8 +106,8 @@ TEST_F(TemplateURLTest, URLRefTestCount) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://foox/", result.spec()); } @@ -118,8 +118,8 @@ TEST_F(TemplateURLTest, URLRefTestCount2) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://foox10/", result.spec()); } @@ -130,8 +130,8 @@ TEST_F(TemplateURLTest, URLRefTestIndices) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://fooxxy/", result.spec()); } @@ -142,8 +142,8 @@ TEST_F(TemplateURLTest, URLRefTestIndices2) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://fooxx1y1/", result.spec()); } @@ -154,8 +154,8 @@ TEST_F(TemplateURLTest, URLRefTestEncoding) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://fooxxutf-8ya/", result.spec()); } @@ -187,8 +187,8 @@ TEST_F(TemplateURLTest, InputEncodingBeforeSearchTerm) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://fooxutf-8axyb/", result.spec()); } @@ -199,8 +199,8 @@ TEST_F(TemplateURLTest, URLRefTestEncoding2) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ("http://fooxxutf-8yutf-8a/", result.spec()); } @@ -225,9 +225,8 @@ TEST_F(TemplateURLTest, URLRefTestSearchTermsUsingTermsData) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTermsUsingTermsData(value.terms, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16(), - search_terms_data)); + GURL result(url.url_ref().ReplaceSearchTermsUsingTermsData( + TemplateURLRef::SearchTermsArgs(value.terms), search_terms_data)); ASSERT_TRUE(result.is_valid()); EXPECT_EQ(value.output, result.spec()); } @@ -331,8 +330,8 @@ TEST_F(TemplateURLTest, ReplaceSearchTerms) { std::string expected_result = test_data[i].expected_result; ReplaceSubstringsAfterOffset(&expected_result, 0, "{language}", g_browser_process->GetApplicationLocale()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("X")))); ASSERT_TRUE(result.is_valid()); EXPECT_EQ(expected_result, result.spec()); } @@ -369,8 +368,64 @@ TEST_F(TemplateURLTest, ReplaceArbitrarySearchTerms) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(test_data[i].search_term, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(test_data[i].search_term))); + ASSERT_TRUE(result.is_valid()); + EXPECT_EQ(test_data[i].expected_result, result.spec()); + } +} + +// Tests replacing assisted query stats (AQS) in various scenarios. +TEST_F(TemplateURLTest, ReplaceAssistedQueryStats) { + struct TestData { + const string16 search_term; + const std::string aqs; + const std::string base_url; + const std::string url; + const std::string expected_result; + } test_data[] = { + // No HTTPS, no AQS. + { ASCIIToUTF16("foo"), + "chrome.0.0l6", + "http://foo/", + "{google:baseURL}?{searchTerms}{google:assistedQueryStats}", + "http://foo/?foo" }, + // HTTPS available, AQS should be replaced. + { ASCIIToUTF16("foo"), + "chrome.0.0l6", + "https://foo/", + "{google:baseURL}?{searchTerms}{google:assistedQueryStats}", + "https://foo/?fooaqs=chrome.0.0l6&" }, + // HTTPS available, however AQS is empty. + { ASCIIToUTF16("foo"), + "", + "https://foo/", + "{google:baseURL}?{searchTerms}{google:assistedQueryStats}", + "https://foo/?foo" }, + // No {google:baseURL} and protocol is HTTP, we must not substitute AQS. + { ASCIIToUTF16("foo"), + "chrome.0.0l6", + "", + "http://foo?{searchTerms}{google:assistedQueryStats}", + "http://foo/?foo" }, + // A non-Google search provider with HTTPS should allow AQS. + { ASCIIToUTF16("foo"), + "chrome.0.0l6", + "", + "https://foo?{searchTerms}{google:assistedQueryStats}", + "https://foo/?fooaqs=chrome.0.0l6&" }, + }; + TemplateURLData data; + data.input_encodings.push_back("UTF-8"); + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { + data.SetURL(test_data[i].url); + TemplateURL url(NULL, data); + EXPECT_TRUE(url.url_ref().IsValid()); + ASSERT_TRUE(url.url_ref().SupportsReplacement()); + TemplateURLRef::SearchTermsArgs search_terms_args(test_data[i].search_term); + search_terms_args.assisted_query_stats = test_data[i].aqs; + UIThreadSearchTermsData::SetGoogleBaseURL(test_data[i].base_url); + GURL result(url.url_ref().ReplaceSearchTerms(search_terms_args)); ASSERT_TRUE(result.is_valid()); EXPECT_EQ(test_data[i].expected_result, result.spec()); } @@ -401,9 +456,12 @@ TEST_F(TemplateURLTest, Suggestions) { EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("foobar"), - test_data[i].accepted_suggestion, - test_data[i].original_query_for_suggestion)); + TemplateURLRef::SearchTermsArgs search_terms_args( + ASCIIToUTF16("foobar")); + search_terms_args.accepted_suggestion = test_data[i].accepted_suggestion; + search_terms_args.original_query = + test_data[i].original_query_for_suggestion; + GURL result(url.url_ref().ReplaceSearchTerms(search_terms_args)); ASSERT_TRUE(result.is_valid()); EXPECT_EQ(test_data[i].expected_result, result.spec()); } @@ -425,8 +483,8 @@ TEST_F(TemplateURLTest, RLZ) { TemplateURL url(NULL, data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); - GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + GURL result(url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")))); ASSERT_TRUE(result.is_valid()); std::string expected_url = "http://bar/?"; if (!rlz_string.empty()) diff --git a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc index 58562a3..160f22e 100644 --- a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc +++ b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc @@ -57,8 +57,8 @@ bool EditSearchEngineController::IsURLValid( // Replace any search term with a placeholder string and make sure the // resulting URL is valid. - return GURL(template_ref.ReplaceSearchTerms(ASCIIToUTF16("x"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())).is_valid(); + return GURL(template_ref.ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")))).is_valid(); } bool EditSearchEngineController::IsKeywordValid( @@ -133,8 +133,8 @@ std::string EditSearchEngineController::GetFixedUpURL( TemplateURLData data; data.SetURL(url); TemplateURL t_url(profile_, data); - std::string expanded_url(t_url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + std::string expanded_url(t_url.url_ref().ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")))); url_parse::Parsed parts; std::string scheme(URLFixerUpper::SegmentURL(expanded_url, &parts)); if (!parts.scheme.is_valid()) @@ -142,4 +142,3 @@ std::string EditSearchEngineController::GetFixedUpURL( return url; } - diff --git a/chrome/browser/ui/startup/startup_browser_creator.cc b/chrome/browser/ui/startup/startup_browser_creator.cc index d25fc1a..ab20879 100644 --- a/chrome/browser/ui/startup/startup_browser_creator.cc +++ b/chrome/browser/ui/startup/startup_browser_creator.cc @@ -276,8 +276,8 @@ std::vector<GURL> StartupBrowserCreator::GetURLsFromCommandLine( const TemplateURLRef& search_url = default_provider->url_ref(); DCHECK(search_url.SupportsReplacement()); string16 search_term = param.LossyDisplayName().substr(2); - urls.push_back(GURL(search_url.ReplaceSearchTerms(search_term, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16()))); + urls.push_back(GURL(search_url.ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(search_term)))); continue; } } @@ -562,4 +562,3 @@ bool HasPendingUncleanExit(Profile* profile) { return !profile->DidLastSessionExitCleanly() && !profile_launch_observer.Get().HasBeenLaunched(profile); } - diff --git a/chrome/browser/ui/startup/startup_browser_creator_win.cc b/chrome/browser/ui/startup/startup_browser_creator_win.cc index 9642c6e..974f492 100644 --- a/chrome/browser/ui/startup/startup_browser_creator_win.cc +++ b/chrome/browser/ui/startup/startup_browser_creator_win.cc @@ -53,12 +53,11 @@ GURL GetURLToOpen(Profile* profile) { if (default_provider) { const TemplateURLRef& search_url = default_provider->url_ref(); DCHECK(search_url.SupportsReplacement()); - return GURL(search_url.ReplaceSearchTerms(search_string, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); + return GURL(search_url.ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(search_string))); } } return GURL(); } } // namespace browser - diff --git a/chrome/browser/ui/views/frame/browser_frame_win.cc b/chrome/browser/ui/views/frame/browser_frame_win.cc index 9798256..301bdae 100644 --- a/chrome/browser/ui/views/frame/browser_frame_win.cc +++ b/chrome/browser/ui/views/frame/browser_frame_win.cc @@ -496,9 +496,8 @@ void BrowserFrameWin::HandleMetroNavSearchRequest(WPARAM w_param, if (default_provider) { const TemplateURLRef& search_url = default_provider->url_ref(); DCHECK(search_url.SupportsReplacement()); - request_url = GURL(search_url.ReplaceSearchTerms(search_string, - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, - string16())); + request_url = GURL(search_url.ReplaceSearchTerms( + TemplateURLRef::SearchTermsArgs(search_string))); } } if (request_url.is_valid()) { |