diff options
Diffstat (limited to 'chrome/browser')
18 files changed, 393 insertions, 240 deletions
diff --git a/chrome/browser/android/provider/chrome_browser_provider.cc b/chrome/browser/android/provider/chrome_browser_provider.cc index 656db59..71764ef 100644 --- a/chrome/browser/android/provider/chrome_browser_provider.cc +++ b/chrome/browser/android/provider/chrome_browser_provider.cc @@ -894,8 +894,9 @@ class SearchTermTask : public HistoryProviderTask { template_service->GetDefaultSearchProvider(); if (search_engine) { const TemplateURLRef* search_url = &search_engine->url_ref(); - std::string url = search_url->ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(row->search_term())); + TemplateURLRef::SearchTermsArgs search_terms_args(row->search_term()); + search_terms_args.append_extra_query_params = true; + std::string url = search_url->ReplaceSearchTerms(search_terms_args); if (!url.empty()) { row->set_url(GURL(url)); row->set_template_url_id(search_engine->id()); diff --git a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc index f95d1f6..d7342f5 100644 --- a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc @@ -5,6 +5,7 @@ #include "chrome/browser/autocomplete/autocomplete_provider.h" #include "base/bind.h" +#include "base/command_line.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/strings/string16.h" @@ -21,6 +22,7 @@ #include "chrome/browser/search_engines/template_url_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/common/chrome_notification_types.h" +#include "chrome/common/chrome_switches.h" #include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_profile.h" #include "content/public/browser/notification_observer.h" @@ -185,6 +187,8 @@ class AutocompleteProviderTest : public testing::Test, void ResetControllerWithKeywordProvider(); void RunExactKeymatchTest(bool allow_exact_keyword_match); + void CopyResults(); + AutocompleteResult result_; scoped_ptr<AutocompleteController> controller_; @@ -416,12 +420,16 @@ void AutocompleteProviderTest::RunExactKeymatchTest( controller_->result().default_match()->type); } +void AutocompleteProviderTest::CopyResults() { + result_.CopyFrom(controller_->result()); +} + void AutocompleteProviderTest::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { if (controller_->done()) { - result_.CopyFrom(controller_->result()); + CopyResults(); base::MessageLoop::current()->Quit(); } } @@ -480,6 +488,21 @@ TEST_F(AutocompleteProviderTest, AllowExactKeywordMatch) { RunExactKeymatchTest(false); } +// Ensures matches from (only) the default search provider respect any extra +// query params set on the command line. +TEST_F(AutocompleteProviderTest, ExtraQueryParams) { + ResetControllerWithKeywordAndSearchProviders(); + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kExtraSearchQueryParams, "a=b"); + RunExactKeymatchTest(true); + CopyResults(); + ASSERT_EQ(2U, result_.size()); + EXPECT_EQ("http://keyword/test", + result_.match_at(0)->destination_url.possibly_invalid_spec()); + EXPECT_EQ("http://defaultturl/k%20test?a=b", + result_.match_at(1)->destination_url.possibly_invalid_spec()); +} + // Test that redundant associated keywords are removed. TEST_F(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult) { ResetControllerWithKeywordProvider(); diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc index f6487a6..c2759dd 100644 --- a/chrome/browser/autocomplete/keyword_provider.cc +++ b/chrome/browser/autocomplete/keyword_provider.cc @@ -415,55 +415,6 @@ int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type, 1450 : 1100; } -// static -void KeywordProvider::FillInURLAndContents(const string16& remaining_input, - const TemplateURL* element, - AutocompleteMatch* match) { - DCHECK(!element->short_name().empty()); - const TemplateURLRef& element_ref = element->url_ref(); - DCHECK(element_ref.IsValid()); - int message_id = element->IsExtensionKeyword() ? - IDS_EXTENSION_KEYWORD_COMMAND : IDS_KEYWORD_SEARCH; - if (remaining_input.empty()) { - // Allow extension keyword providers to accept empty string input. This is - // useful to allow extensions to do something in the case where no input is - // entered. - if (element_ref.SupportsReplacement() && !element->IsExtensionKeyword()) { - // No query input; return a generic, no-destination placeholder. - match->contents.assign( - l10n_util::GetStringFUTF16(message_id, - element->AdjustedShortNameForLocaleDirection(), - l10n_util::GetStringUTF16(IDS_EMPTY_KEYWORD_VALUE))); - match->contents_class.push_back( - ACMatchClassification(0, ACMatchClassification::DIM)); - } else { - // Keyword that has no replacement text (aka a shorthand for a URL). - match->destination_url = GURL(element->url()); - match->contents.assign(element->short_name()); - AutocompleteMatch::ClassifyLocationInString(0, match->contents.length(), - match->contents.length(), ACMatchClassification::NONE, - &match->contents_class); - } - } else { - // Create destination URL by escaping user input and substituting into - // keyword template URL. The escaping here handles whitespace in user - // input, but we rely on later canonicalization functions to do more - // fixup to make the URL valid if necessary. - DCHECK(element_ref.SupportsReplacement()); - match->destination_url = GURL(element_ref.ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(remaining_input))); - std::vector<size_t> content_param_offsets; - match->contents.assign(l10n_util::GetStringFUTF16(message_id, - element->short_name(), - remaining_input, - &content_param_offsets)); - DCHECK_EQ(2U, content_param_offsets.size()); - AutocompleteMatch::ClassifyLocationInString(content_param_offsets[1], - remaining_input.length(), match->contents.length(), - ACMatchClassification::NONE, &match->contents_class); - } -} - AutocompleteMatch KeywordProvider::CreateAutocompleteMatch( const TemplateURL* template_url, const AutocompleteInput& input, @@ -512,6 +463,57 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch( return match; } +void KeywordProvider::FillInURLAndContents(const string16& remaining_input, + const TemplateURL* element, + AutocompleteMatch* match) const { + DCHECK(!element->short_name().empty()); + const TemplateURLRef& element_ref = element->url_ref(); + DCHECK(element_ref.IsValid()); + int message_id = element->IsExtensionKeyword() ? + IDS_EXTENSION_KEYWORD_COMMAND : IDS_KEYWORD_SEARCH; + if (remaining_input.empty()) { + // Allow extension keyword providers to accept empty string input. This is + // useful to allow extensions to do something in the case where no input is + // entered. + if (element_ref.SupportsReplacement() && !element->IsExtensionKeyword()) { + // No query input; return a generic, no-destination placeholder. + match->contents.assign( + l10n_util::GetStringFUTF16(message_id, + element->AdjustedShortNameForLocaleDirection(), + l10n_util::GetStringUTF16(IDS_EMPTY_KEYWORD_VALUE))); + match->contents_class.push_back( + ACMatchClassification(0, ACMatchClassification::DIM)); + } else { + // Keyword that has no replacement text (aka a shorthand for a URL). + match->destination_url = GURL(element->url()); + match->contents.assign(element->short_name()); + AutocompleteMatch::ClassifyLocationInString(0, match->contents.length(), + match->contents.length(), ACMatchClassification::NONE, + &match->contents_class); + } + } else { + // Create destination URL by escaping user input and substituting into + // keyword template URL. The escaping here handles whitespace in user + // input, but we rely on later canonicalization functions to do more + // fixup to make the URL valid if necessary. + DCHECK(element_ref.SupportsReplacement()); + TemplateURLRef::SearchTermsArgs search_terms_args(remaining_input); + search_terms_args.append_extra_query_params = + element == GetTemplateURLService()->GetDefaultSearchProvider(); + match->destination_url = + GURL(element_ref.ReplaceSearchTerms(search_terms_args)); + std::vector<size_t> content_param_offsets; + match->contents.assign(l10n_util::GetStringFUTF16(message_id, + element->short_name(), + remaining_input, + &content_param_offsets)); + DCHECK_EQ(2U, content_param_offsets.size()); + AutocompleteMatch::ClassifyLocationInString(content_param_offsets[1], + remaining_input.length(), match->contents.length(), + ACMatchClassification::NONE, &match->contents_class); + } +} + void KeywordProvider::Observe(int type, const content::NotificationSource& source, const content::NotificationDetails& details) { diff --git a/chrome/browser/autocomplete/keyword_provider.h b/chrome/browser/autocomplete/keyword_provider.h index 9d1ba52..7e582b4 100644 --- a/chrome/browser/autocomplete/keyword_provider.h +++ b/chrome/browser/autocomplete/keyword_provider.h @@ -121,12 +121,6 @@ class KeywordProvider : public AutocompleteProvider, bool prefer_keyword, bool allow_exact_keyword_match); - // Fills in the "destination_url" and "contents" fields of |match| with the - // provided user input and keyword data. - static void FillInURLAndContents(const string16& remaining_input, - const TemplateURL* element, - AutocompleteMatch* match); - // Creates a fully marked-up AutocompleteMatch from the user's input. // If |relevance| is negative, calculate a relevance based on heuristics. AutocompleteMatch CreateAutocompleteMatch(const TemplateURL* template_url, @@ -135,6 +129,12 @@ class KeywordProvider : public AutocompleteProvider, const string16& remaining_input, int relevance); + // Fills in the "destination_url" and "contents" fields of |match| with the + // provided user input and keyword data. + void FillInURLAndContents(const string16& remaining_input, + const TemplateURL* element, + AutocompleteMatch* match) const; + void EnterExtensionKeywordMode(const std::string& extension_id); void MaybeEndExtensionKeywordMode(); diff --git a/chrome/browser/autocomplete/keyword_provider_unittest.cc b/chrome/browser/autocomplete/keyword_provider_unittest.cc index b5f4e4c..45b5ac9 100644 --- a/chrome/browser/autocomplete/keyword_provider_unittest.cc +++ b/chrome/browser/autocomplete/keyword_provider_unittest.cc @@ -2,12 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/command_line.h" #include "base/message_loop.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_match.h" #include "chrome/browser/autocomplete/keyword_provider.h" #include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_service.h" +#include "chrome/common/chrome_switches.h" #include "chrome/test/base/testing_browser_process.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -33,23 +35,25 @@ class KeywordProviderTest : public testing::Test { ResultType AutocompleteMatch::* member); protected: + static const TemplateURLService::Initializer kTestData[]; + scoped_refptr<KeywordProvider> kw_provider_; scoped_ptr<TemplateURLService> model_; }; -void KeywordProviderTest::SetUp() { - static const TemplateURLService::Initializer kTestKeywordData[] = { - { "aa", "aa.com?foo=%s", "aa" }, - { "aaaa", "http://aaaa/?aaaa=1&b=%s&c", "aaaa" }, - { "aaaaa", "%s", "aaaaa" }, - { "ab", "bogus URL %s", "ab" }, - { "weasel", "weasel%sweasel", "weasel" }, - { "www", " +%2B?=%sfoo ", "www" }, - { "z", "%s=z", "z" }, - }; +// static +const TemplateURLService::Initializer KeywordProviderTest::kTestData[] = { + { "aa", "aa.com?foo=%s", "aa" }, + { "aaaa", "http://aaaa/?aaaa=1&b=%s&c", "aaaa" }, + { "aaaaa", "%s", "aaaaa" }, + { "ab", "bogus URL %s", "ab" }, + { "weasel", "weasel%sweasel", "weasel" }, + { "www", " +%2B?=%sfoo ", "www" }, + { "z", "%s=z", "z" }, +}; - model_.reset(new TemplateURLService(kTestKeywordData, - arraysize(kTestKeywordData))); +void KeywordProviderTest::SetUp() { + model_.reset(new TemplateURLService(kTestData, arraysize(kTestData))); kw_provider_ = new KeywordProvider(NULL, model_.get()); } @@ -189,7 +193,7 @@ TEST_F(KeywordProviderTest, Contents) { }; RunTest<string16>(contents_cases, arraysize(contents_cases), - &AutocompleteMatch::contents); + &AutocompleteMatch::contents); } TEST_F(KeywordProviderTest, AddKeyword) { @@ -274,3 +278,19 @@ TEST_F(KeywordProviderTest, GetSubstitutingTemplateURLForInput) { EXPECT_EQ(cases[i].updated_cursor_position, input.cursor_position()); } } + +// If extra query params are specified on the command line, they should be +// reflected (only) in the default search provider's destination URL. +TEST_F(KeywordProviderTest, ExtraQueryParams) { + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kExtraSearchQueryParams, "a=b"); + + test_data<GURL> url_cases[] = { + {ASCIIToUTF16("a 1 2 3"), 3, {GURL("aa.com?a=b&foo=1+2+3"), + GURL("bogus URL 1+2+3"), + GURL("http://aaaa/?aaaa=1&b=1+2+3&c")}}, + }; + + RunTest<GURL>(url_cases, arraysize(url_cases), + &AutocompleteMatch::destination_url); +} diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 474542c..c5861c4 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -265,7 +265,8 @@ AutocompleteMatch SearchProvider::CreateSearchSuggestion( const AutocompleteInput& input, bool is_keyword, int accepted_suggestion, - int omnibox_start_margin) { + int omnibox_start_margin, + bool append_extra_query_params) { AutocompleteMatch match(autocomplete_provider, relevance, false, type); if (!template_url) @@ -333,6 +334,8 @@ AutocompleteMatch SearchProvider::CreateSearchSuggestion( match.search_terms_args->original_query = input_text; match.search_terms_args->accepted_suggestion = accepted_suggestion; match.search_terms_args->omnibox_start_margin = omnibox_start_margin; + match.search_terms_args->append_extra_query_params = + append_extra_query_params; // 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. @@ -1374,7 +1377,8 @@ void SearchProvider::AddMatchToMap(const string16& query_string, providers_.GetKeywordProviderURL() : providers_.GetDefaultProviderURL(); AutocompleteMatch match = CreateSearchSuggestion(this, relevance, type, template_url, query_string, input_text, input_, is_keyword, - accepted_suggestion, omnibox_start_margin_); + accepted_suggestion, omnibox_start_margin_, + !is_keyword || providers_.default_provider().empty()); if (!match.destination_url.is_valid()) return; match.RecordAdditionalInfo(kRelevanceFromServerKey, diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h index 0706c1b..531825d 100644 --- a/chrome/browser/autocomplete/search_provider.h +++ b/chrome/browser/autocomplete/search_provider.h @@ -74,6 +74,9 @@ class SearchProvider : public AutocompleteProvider, // whether we should allow inline autocompletion and what the transition type // should be. |accepted_suggestion| and |omnibox_start_margin| are used along // with |input_text| to generate Assisted Query Stats. + // |append_extra_query_params| should be set if |template_url| is the default + // search engine, so the destination URL will contain any + // command-line-specified query params. static AutocompleteMatch CreateSearchSuggestion( AutocompleteProvider* autocomplete_provider, int relevance, @@ -84,7 +87,8 @@ class SearchProvider : public AutocompleteProvider, const AutocompleteInput& input, bool is_keyword, int accepted_suggestion, - int omnibox_start_margin); + int omnibox_start_margin, + bool append_extra_query_params); // AutocompleteProvider: virtual void AddProviderInfo(ProvidersInfo* provider_info) const OVERRIDE; diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index ac27b58..2551ca2 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -812,13 +812,15 @@ TEST_F(SearchProviderTest, CommandLineOverrides) { CommandLine::ForCurrentProcess()->AppendSwitchASCII(switches::kGoogleBaseURL, "http://www.bar.com/"); + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kExtraSearchQueryParams, "a=b"); TestData cases[] = { { ASCIIToUTF16("k a"), 2, { ResultInfo(GURL("http://keyword/a"), AutocompleteMatchType::SEARCH_OTHER_ENGINE, ASCIIToUTF16("k a")), - ResultInfo(GURL("http://www.bar.com/k%20a"), + ResultInfo(GURL("http://www.bar.com/k%20a?a=b"), AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, ASCIIToUTF16("k a")) } }, }; diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc index 6e585a8..77c0e1f 100644 --- a/chrome/browser/autocomplete/zero_suggest_provider.cc +++ b/chrome/browser/autocomplete/zero_suggest_provider.cc @@ -315,7 +315,7 @@ void ZeroSuggestProvider::AddMatchToMap(int relevance, // of passing in -1. AutocompleteMatch match = SearchProvider::CreateSearchSuggestion( this, relevance, type, template_url, query_string, query_string, - AutocompleteInput(), false, accepted_suggestion, -1); + AutocompleteInput(), false, accepted_suggestion, -1, true); if (!match.destination_url.is_valid()) return; diff --git a/chrome/browser/search/search.cc b/chrome/browser/search/search.cc index 9f2bbb3..9d31c26 100644 --- a/chrome/browser/search/search.cc +++ b/chrome/browser/search/search.cc @@ -88,10 +88,13 @@ TemplateURL* GetDefaultSearchProviderTemplateURL(Profile* profile) { return NULL; } -GURL TemplateURLRefToGURL(const TemplateURLRef& ref, int start_margin) { +GURL TemplateURLRefToGURL(const TemplateURLRef& ref, + int start_margin, + bool append_extra_query_params) { TemplateURLRef::SearchTermsArgs search_terms_args = TemplateURLRef::SearchTermsArgs(string16()); search_terms_args.omnibox_start_margin = start_margin; + search_terms_args.append_extra_query_params = append_extra_query_params; return GURL(ref.ReplaceSearchTerms(search_terms_args)); } @@ -105,14 +108,14 @@ bool MatchesOrigin(const GURL& my_url, const GURL& other_url) { bool MatchesAnySearchURL(const GURL& url, TemplateURL* template_url) { GURL search_url = - TemplateURLRefToGURL(template_url->url_ref(), kDisableStartMargin); + TemplateURLRefToGURL(template_url->url_ref(), kDisableStartMargin, false); if (search_url.is_valid() && MatchesOriginAndPath(url, search_url)) return true; // "URLCount() - 1" because we already tested url_ref above. for (size_t i = 0; i < template_url->URLCount() - 1; ++i) { TemplateURLRef ref(template_url, i); - search_url = TemplateURLRefToGURL(ref, kDisableStartMargin); + search_url = TemplateURLRefToGURL(ref, kDisableStartMargin, false); if (search_url.is_valid() && MatchesOriginAndPath(url, search_url)) return true; } @@ -181,7 +184,7 @@ bool IsInstantURL(const GURL& url, Profile* profile) { const TemplateURLRef& instant_url_ref = template_url->instant_url_ref(); const GURL instant_url = - TemplateURLRefToGURL(instant_url_ref, kDisableStartMargin); + TemplateURLRefToGURL(instant_url_ref, kDisableStartMargin, false); return instant_url.is_valid() && (MatchesOriginAndPath(url, instant_url) || (extended_api_enabled && MatchesAnySearchURL(url, template_url))); @@ -442,7 +445,7 @@ GURL GetInstantURL(Profile* profile, int start_margin) { TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile); GURL instant_url = - TemplateURLRefToGURL(template_url->instant_url_ref(), start_margin); + TemplateURLRefToGURL(template_url->instant_url_ref(), start_margin, true); // Extended mode requires HTTPS. Force it unless the base URL was overridden // on the command line, in which case we allow HTTP (see comments on @@ -669,7 +672,7 @@ bool DefaultSearchProviderSupportsInstant(Profile* profile) { return false; GURL instant_url = TemplateURLRefToGURL(template_url->instant_url_ref(), - kDisableStartMargin); + kDisableStartMargin, false); // Extended mode instant requires a search terms replacement key. return instant_url.is_valid() && (!IsInstantExtendedAPIEnabled() || diff --git a/chrome/browser/search/search_unittest.cc b/chrome/browser/search/search_unittest.cc index 1eb84bc..fb3acc0 100644 --- a/chrome/browser/search/search_unittest.cc +++ b/chrome/browser/search/search_unittest.cc @@ -583,6 +583,14 @@ TEST_F(SearchTest, CommandLineOverrides) { // URL doesn't contain "google". local_instant_url = GetLocalInstantURL(profile()); EXPECT_EQ(GURL(chrome::kChromeSearchLocalGoogleNtpUrl), local_instant_url); + + // If we specify extra search query params, they should be inserted into the + // query portion of the instant URL. + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kExtraSearchQueryParams, "a=b"); + instant_url = GetInstantURL(profile(), kDisableStartMargin); + ASSERT_TRUE(instant_url.is_valid()); + EXPECT_EQ("http://www.bar.com/webhp?a=b&strk", instant_url.spec()); } } // namespace chrome diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc index 535bb28..1ccf6dc 100644 --- a/chrome/browser/search_engines/template_url.cc +++ b/chrome/browser/search_engines/template_url.cc @@ -4,6 +4,7 @@ #include "chrome/browser/search_engines/template_url.h" +#include "base/command_line.h" #include "base/format_macros.h" #include "base/guid.h" #include "base/i18n/case_conversion.h" @@ -18,6 +19,7 @@ #include "chrome/browser/google/google_util.h" #include "chrome/browser/search_engines/search_terms_data.h" #include "chrome/browser/search_engines/template_url_service.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/url_constants.h" #include "extensions/common/constants.h" #include "google_apis/google_api_keys.h" @@ -155,7 +157,8 @@ TemplateURLRef::SearchTermsArgs::SearchTermsArgs(const string16& search_terms) : search_terms(search_terms), accepted_suggestion(NO_SUGGESTIONS_AVAILABLE), cursor_position(string16::npos), - omnibox_start_margin(-1) { + omnibox_start_margin(-1), + append_extra_query_params(false) { } TemplateURLRef::SearchTermsArgs::~SearchTermsArgs() { @@ -227,159 +230,21 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( if (!valid_) return std::string(); - if (replacements_.empty()) - return parsed_url_; - - // Determine if the search terms are in the query or before. We're escaping - // space as '+' in the former case and as '%20' in the latter case. - bool is_in_query = true; - for (Replacements::iterator i = replacements_.begin(); - i != replacements_.end(); ++i) { - if (i->type == SEARCH_TERMS) { - string16::size_type query_start = parsed_url_.find('?'); - is_in_query = query_start != string16::npos && - (static_cast<string16::size_type>(i->index) > query_start); - break; - } - } - - std::string input_encoding; - string16 encoded_terms; - string16 encoded_original_query; - owner_->EncodeSearchTerms(search_terms_args, is_in_query, &input_encoding, - &encoded_terms, &encoded_original_query); - - std::string url = parsed_url_; - - // replacements_ is ordered in ascending order, as such we need to iterate - // from the back. - for (Replacements::reverse_iterator i = replacements_.rbegin(); - i != replacements_.rend(); ++i) { - switch (i->type) { - case ENCODING: - 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_BASE_URL: - url.insert(i->index, search_terms_data.GoogleBaseURLValue()); - break; - - case GOOGLE_BASE_SUGGEST_URL: - url.insert(i->index, search_terms_data.GoogleBaseSuggestURLValue()); - break; - - case GOOGLE_CURSOR_POSITION: - if (search_terms_args.cursor_position != string16::npos) - url.insert(i->index, - base::StringPrintf("cp=%" PRIuS "&", - search_terms_args.cursor_position)); - break; - - case GOOGLE_INSTANT_ENABLED: - url.insert(i->index, search_terms_data.InstantEnabledParam()); - break; - - case GOOGLE_INSTANT_EXTENDED_ENABLED: - url.insert(i->index, search_terms_data.InstantExtendedEnabledParam()); - break; - - case GOOGLE_NTP_IS_THEMED: - url.insert(i->index, search_terms_data.NTPIsThemedParam()); - break; - - case GOOGLE_OMNIBOX_START_MARGIN: - if (search_terms_args.omnibox_start_margin >= 0) { - url.insert(i->index, "es_sm=" + - base::IntToString(search_terms_args.omnibox_start_margin) + "&"); - } - break; - - case GOOGLE_ORIGINAL_QUERY_FOR_SUGGESTION: - 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: { - // On platforms that don't have RLZ, we still want this branch - // to happen so that we replace the RLZ template with the - // empty string. (If we don't handle this case, we hit a - // NOTREACHED below.) - string16 rlz_string = search_terms_data.GetRlzParameterValue(); - if (!rlz_string.empty()) { - url.insert(i->index, "rlz=" + UTF16ToUTF8(rlz_string) + "&"); - } - break; - } - - case GOOGLE_SEARCH_CLIENT: { - std::string client = search_terms_data.GetSearchClient(); - if (!client.empty()) - url.insert(i->index, "client=" + client + "&"); - break; - } - - case GOOGLE_SEARCH_FIELDTRIAL_GROUP: - // We are not currently running any fieldtrials that modulate the search - // url. If we do, then we'd have some conditional insert such as: - // url.insert(i->index, used_www ? "gcx=w&" : "gcx=c&"); - break; - - case GOOGLE_SUGGEST_CLIENT: - url.insert(i->index, search_terms_data.GetSuggestClient()); - break; - - case GOOGLE_UNESCAPED_SEARCH_TERMS: { - std::string unescaped_terms; - 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(), - unescaped_terms.end())); - break; - } - - case GOOGLE_ZERO_PREFIX_URL: - if (!search_terms_args.zero_prefix_url.empty()) { - const std::string& escaped_zero_prefix_url = - net::EscapeQueryParamValue(search_terms_args.zero_prefix_url, - true); - url.insert(i->index, "url=" + escaped_zero_prefix_url + "&"); - } - - break; - - case LANGUAGE: - url.insert(i->index, search_terms_data.GetApplicationLocale()); - break; - - case SEARCH_TERMS: - url.insert(i->index, UTF16ToUTF8(encoded_terms)); - break; - - default: - NOTREACHED(); - break; + std::string url(HandleReplacements(search_terms_args, search_terms_data)); + + // If the user specified additional query params on the command line, add + // them. + if (search_terms_args.append_extra_query_params) { + std::string query_params(CommandLine::ForCurrentProcess()-> + GetSwitchValueASCII(switches::kExtraSearchQueryParams)); + GURL gurl(url); + if (!query_params.empty() && gurl.is_valid()) { + GURL::Replacements replacements; + const std::string existing_query_params(gurl.query()); + if (!existing_query_params.empty()) + query_params += "&" + existing_query_params; + replacements.SetQueryStr(query_params); + return gurl.ReplaceComponents(replacements).possibly_invalid_spec(); } } @@ -733,6 +598,168 @@ void TemplateURLRef::ParseHostAndSearchTermKey( path_ = url.path(); } +std::string TemplateURLRef::HandleReplacements( + const SearchTermsArgs& search_terms_args, + const SearchTermsData& search_terms_data) const { + if (replacements_.empty()) + return parsed_url_; + + // Determine if the search terms are in the query or before. We're escaping + // space as '+' in the former case and as '%20' in the latter case. + bool is_in_query = true; + for (Replacements::iterator i = replacements_.begin(); + i != replacements_.end(); ++i) { + if (i->type == SEARCH_TERMS) { + string16::size_type query_start = parsed_url_.find('?'); + is_in_query = query_start != string16::npos && + (static_cast<string16::size_type>(i->index) > query_start); + break; + } + } + + std::string input_encoding; + string16 encoded_terms; + string16 encoded_original_query; + owner_->EncodeSearchTerms(search_terms_args, is_in_query, &input_encoding, + &encoded_terms, &encoded_original_query); + + std::string url = parsed_url_; + + // replacements_ is ordered in ascending order, as such we need to iterate + // from the back. + for (Replacements::reverse_iterator i = replacements_.rbegin(); + i != replacements_.rend(); ++i) { + switch (i->type) { + case ENCODING: + 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_BASE_URL: + url.insert(i->index, search_terms_data.GoogleBaseURLValue()); + break; + + case GOOGLE_BASE_SUGGEST_URL: + url.insert(i->index, search_terms_data.GoogleBaseSuggestURLValue()); + break; + + case GOOGLE_CURSOR_POSITION: + if (search_terms_args.cursor_position != string16::npos) + url.insert(i->index, + base::StringPrintf("cp=%" PRIuS "&", + search_terms_args.cursor_position)); + break; + + case GOOGLE_INSTANT_ENABLED: + url.insert(i->index, search_terms_data.InstantEnabledParam()); + break; + + case GOOGLE_INSTANT_EXTENDED_ENABLED: + url.insert(i->index, search_terms_data.InstantExtendedEnabledParam()); + break; + + case GOOGLE_NTP_IS_THEMED: + url.insert(i->index, search_terms_data.NTPIsThemedParam()); + break; + + case GOOGLE_OMNIBOX_START_MARGIN: + if (search_terms_args.omnibox_start_margin >= 0) { + url.insert(i->index, "es_sm=" + + base::IntToString(search_terms_args.omnibox_start_margin) + "&"); + } + break; + + case GOOGLE_ORIGINAL_QUERY_FOR_SUGGESTION: + 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: { + // On platforms that don't have RLZ, we still want this branch + // to happen so that we replace the RLZ template with the + // empty string. (If we don't handle this case, we hit a + // NOTREACHED below.) + string16 rlz_string = search_terms_data.GetRlzParameterValue(); + if (!rlz_string.empty()) { + url.insert(i->index, "rlz=" + UTF16ToUTF8(rlz_string) + "&"); + } + break; + } + + case GOOGLE_SEARCH_CLIENT: { + std::string client = search_terms_data.GetSearchClient(); + if (!client.empty()) + url.insert(i->index, "client=" + client + "&"); + break; + } + + case GOOGLE_SEARCH_FIELDTRIAL_GROUP: + // We are not currently running any fieldtrials that modulate the search + // url. If we do, then we'd have some conditional insert such as: + // url.insert(i->index, used_www ? "gcx=w&" : "gcx=c&"); + break; + + case GOOGLE_SUGGEST_CLIENT: + url.insert(i->index, search_terms_data.GetSuggestClient()); + break; + + case GOOGLE_UNESCAPED_SEARCH_TERMS: { + std::string unescaped_terms; + 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(), + unescaped_terms.end())); + break; + } + + case GOOGLE_ZERO_PREFIX_URL: + if (!search_terms_args.zero_prefix_url.empty()) { + const std::string& escaped_zero_prefix_url = + net::EscapeQueryParamValue(search_terms_args.zero_prefix_url, + true); + url.insert(i->index, "url=" + escaped_zero_prefix_url + "&"); + } + + break; + + case LANGUAGE: + url.insert(i->index, search_terms_data.GetApplicationLocale()); + break; + + case SEARCH_TERMS: + url.insert(i->index, UTF16ToUTF8(encoded_terms)); + break; + + default: + NOTREACHED(); + break; + } + } + + return url; +} + // TemplateURLData ------------------------------------------------------------ diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h index f81876a..3eb9f4d 100644 --- a/chrome/browser/search_engines/template_url.h +++ b/chrome/browser/search_engines/template_url.h @@ -58,7 +58,7 @@ class TemplateURLRef { ~SearchTermsArgs(); // The search terms (query). - const string16 search_terms; + string16 search_terms; // The original (input) query. string16 original_query; @@ -85,6 +85,15 @@ class TemplateURLRef { // The URL of the current webpage to be used for experimental zero-prefix // suggestions. std::string zero_prefix_url; + + // If set, ReplaceSearchTerms() will automatically append any extra query + // params specified via the --extra-search-query-params command-line + // argument. Generally, this should be set when dealing with the search or + // instant TemplateURLRefs of the default search engine and the caller cares + // about the query portion of the URL. Since neither TemplateURLRef nor + // indeed TemplateURL know whether a TemplateURL is the default search + // engine, callers instead must set this manually. + bool append_extra_query_params; }; TemplateURLRef(TemplateURL* owner, Type type); @@ -246,6 +255,13 @@ class TemplateURLRef { void ParseHostAndSearchTermKey( const SearchTermsData& search_terms_data) const; + // Replaces all replacements in |parsed_url_| with their actual values and + // returns the result. This is the main functionality of + // ReplaceSearchTermsUsingTermsData(). + std::string HandleReplacements( + const SearchTermsArgs& search_terms_args, + const SearchTermsData& search_terms_data) const; + // The TemplateURL that contains us. This should outlive us. TemplateURL* const owner_; diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc index 813f488..bde41b0 100644 --- a/chrome/browser/search_engines/template_url_service.cc +++ b/chrome/browser/search_engines/template_url_service.cc @@ -1391,7 +1391,12 @@ void TemplateURLService::Init(const Initializer* initializers, data.short_name = UTF8ToUTF16(initializers[i].content); data.SetKeyword(UTF8ToUTF16(initializers[i].keyword)); data.SetURL(osd_url); - AddNoNotify(new TemplateURL(profile_, data), true); + TemplateURL* template_url = new TemplateURL(profile_, data); + AddNoNotify(template_url, true); + + // Set the first provided identifier to be the default. + if (i == 0) + SetDefaultSearchProviderNoNotify(template_url); } } diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc index cffbb59..2f8b6c4 100644 --- a/chrome/browser/search_engines/template_url_unittest.cc +++ b/chrome/browser/search_engines/template_url_unittest.cc @@ -3,12 +3,14 @@ // found in the LICENSE file. #include "base/base_paths.h" +#include "base/command_line.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/rlz/rlz.h" #include "chrome/browser/search_engines/search_terms_data.h" #include "chrome/browser/search_engines/template_url.h" +#include "chrome/common/chrome_switches.h" #include "testing/gtest/include/gtest/gtest.h" #if defined(ENABLE_RLZ) @@ -1032,3 +1034,38 @@ TEST_F(TemplateURLTest, ReplaceSearchTermsInURL) { GURL("http://google.com/alt/?q=#q=123"), search_terms, &result)); EXPECT_EQ(GURL("http://google.com/alt/?q=#q=Bob Morane"), result); } + +// Test the |append_extra_query_params| field of SearchTermsArgs. +TEST_F(TemplateURLTest, ExtraQueryParams) { + UIThreadSearchTermsData::SetGoogleBaseURL("http://www.google.com/"); + TemplateURLData data; + // Pick a URL with replacements before, during, and after the query, to ensure + // we don't goof up any of them. + data.SetURL("{google:baseURL}search?q={searchTerms}" + "#{google:originalQueryForSuggestion}x"); + TemplateURL url(NULL, data); + + // Baseline: no command-line args, no |append_extra_query_params| flag. + TemplateURLRef::SearchTermsArgs search_terms(ASCIIToUTF16("abc")); + search_terms.original_query = ASCIIToUTF16("def"); + search_terms.accepted_suggestion = 0; + EXPECT_EQ("http://www.google.com/search?q=abc#oq=def&x", + url.url_ref().ReplaceSearchTerms(search_terms)); + + // Set the flag. Since there are no command-line args, this should have no + // effect. + search_terms.append_extra_query_params = true; + EXPECT_EQ("http://www.google.com/search?q=abc#oq=def&x", + url.url_ref().ReplaceSearchTerms(search_terms)); + + // Now append the command-line arg. This should be inserted into the query. + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kExtraSearchQueryParams, "a=b"); + EXPECT_EQ("http://www.google.com/search?a=b&q=abc#oq=def&x", + url.url_ref().ReplaceSearchTerms(search_terms)); + + // Turn off the flag. Now the command-line arg should be ignored again. + search_terms.append_extra_query_params = false; + EXPECT_EQ("http://www.google.com/search?q=abc#oq=def&x", + url.url_ref().ReplaceSearchTerms(search_terms)); +} diff --git a/chrome/browser/search_engines/util.cc b/chrome/browser/search_engines/util.cc index fbef1cc..6aa478e 100644 --- a/chrome/browser/search_engines/util.cc +++ b/chrome/browser/search_engines/util.cc @@ -50,6 +50,7 @@ GURL GetDefaultSearchURLForSearchTerms(Profile* profile, const TemplateURLRef& search_url = default_provider->url_ref(); DCHECK(search_url.SupportsReplacement()); TemplateURLRef::SearchTermsArgs search_terms_args(terms); + search_terms_args.append_extra_query_params = true; return GURL(search_url.ReplaceSearchTerms(search_terms_args)); } diff --git a/chrome/browser/ui/omnibox/omnibox_controller.cc b/chrome/browser/ui/omnibox/omnibox_controller.cc index 2abb986..5024ab4 100644 --- a/chrome/browser/ui/omnibox/omnibox_controller.cc +++ b/chrome/browser/ui/omnibox/omnibox_controller.cc @@ -266,5 +266,5 @@ void OmniboxController::CreateAndSetInstantMatch( current_match_ = SearchProvider::CreateSearchSuggestion( NULL, 0, match_type, template_url, query_string, input_text, - AutocompleteInput(), false, 0, -1); + AutocompleteInput(), false, 0, -1, true); } diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc index 8d450ee..62404fc 100644 --- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc +++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc @@ -1242,7 +1242,7 @@ void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, default_provider, text, text, input, false, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, - controller_->GetOmniboxBounds().x()); + controller_->GetOmniboxBounds().x(), true); } else { // Can't create a new search match. Leave |match| as is, with an // invalid destination_url. This shouldn't ever happen. For example, |