Commit 32e2d81b authored by mathp@chromium.org's avatar mathp@chromium.org

[Search] Addresses several issues around Google search term extraction for instant extended api:

(1) Adds an additional template pattern with "webhp"
(2) ExtractSearchTermsFromURL now requires HTTPS

BUG=135106
TEST=Standard unit tests under TemplateURL.*


Review URL: https://chromiumcodereview.appspot.com/11094046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162199 0039d316-1c4b-4281-b951-d872f2087c98
parent 3ff52468
...@@ -659,10 +659,10 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) { ...@@ -659,10 +659,10 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
// Verifies that a default search is made using the provider configured via // Verifies that a default search is made using the provider configured via
// policy. Also checks that default search can be completely disabled. // policy. Also checks that default search can be completely disabled.
const string16 kKeyword(ASCIIToUTF16("testsearch")); const string16 kKeyword(ASCIIToUTF16("testsearch"));
const std::string kSearchURL("http://search.example/search?q={searchTerms}"); const std::string kSearchURL("https://search.example/search?q={searchTerms}");
const std::string kAlternateURL0( const std::string kAlternateURL0(
"http://search.example/search#q={searchTerms}"); "https://search.example/search#q={searchTerms}");
const std::string kAlternateURL1("http://search.example/#q={searchTerms}"); const std::string kAlternateURL1("https://search.example/#q={searchTerms}");
TemplateURLService* service = TemplateURLServiceFactory::GetForProfile( TemplateURLService* service = TemplateURLServiceFactory::GetForProfile(
browser()->profile()); browser()->profile());
...@@ -703,7 +703,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) { ...@@ -703,7 +703,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
chrome::FocusLocationBar(browser()); chrome::FocusLocationBar(browser());
LocationBar* location_bar = browser()->window()->GetLocationBar(); LocationBar* location_bar = browser()->window()->GetLocationBar();
ui_test_utils::SendToOmniboxAndSubmit(location_bar, ui_test_utils::SendToOmniboxAndSubmit(location_bar,
"http://search.example/#q=foobar"); "https://search.example/#q=foobar");
OmniboxEditModel* model = location_bar->GetLocationEntry()->model(); OmniboxEditModel* model = location_bar->GetLocationEntry()->model();
EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid()); EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid());
EXPECT_EQ(ASCIIToUTF16("foobar"), model->CurrentMatch().contents); EXPECT_EQ(ASCIIToUTF16("foobar"), model->CurrentMatch().contents);
...@@ -712,7 +712,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) { ...@@ -712,7 +712,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
// second URL pattern. // second URL pattern.
chrome::FocusLocationBar(browser()); chrome::FocusLocationBar(browser());
ui_test_utils::SendToOmniboxAndSubmit(location_bar, ui_test_utils::SendToOmniboxAndSubmit(location_bar,
"http://search.example/search#q=banana"); "https://search.example/search#q=banana");
model = location_bar->GetLocationEntry()->model(); model = location_bar->GetLocationEntry()->model();
EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid()); EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid());
EXPECT_EQ(ASCIIToUTF16("banana"), model->CurrentMatch().contents); EXPECT_EQ(ASCIIToUTF16("banana"), model->CurrentMatch().contents);
...@@ -721,7 +721,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) { ...@@ -721,7 +721,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
// standard search URL pattern. // standard search URL pattern.
chrome::FocusLocationBar(browser()); chrome::FocusLocationBar(browser());
ui_test_utils::SendToOmniboxAndSubmit(location_bar, ui_test_utils::SendToOmniboxAndSubmit(location_bar,
"http://search.example/search?q=tractor+parts"); "https://search.example/search?q=tractor+parts");
model = location_bar->GetLocationEntry()->model(); model = location_bar->GetLocationEntry()->model();
EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid()); EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid());
EXPECT_EQ(ASCIIToUTF16("tractor parts"), model->CurrentMatch().contents); EXPECT_EQ(ASCIIToUTF16("tractor parts"), model->CurrentMatch().contents);
...@@ -729,7 +729,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) { ...@@ -729,7 +729,7 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, ReplaceSearchTerms) {
// Verify that searching from the omnibox prioritizes hash over query. // Verify that searching from the omnibox prioritizes hash over query.
chrome::FocusLocationBar(browser()); chrome::FocusLocationBar(browser());
ui_test_utils::SendToOmniboxAndSubmit(location_bar, ui_test_utils::SendToOmniboxAndSubmit(location_bar,
"http://search.example/search?q=tractor+parts#q=foobar"); "https://search.example/search?q=tractor+parts#q=foobar");
model = location_bar->GetLocationEntry()->model(); model = location_bar->GetLocationEntry()->model();
EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid()); EXPECT_TRUE(model->CurrentMatch().destination_url.is_valid());
EXPECT_EQ(ASCIIToUTF16("foobar"), model->CurrentMatch().contents); EXPECT_EQ(ASCIIToUTF16("foobar"), model->CurrentMatch().contents);
......
...@@ -457,9 +457,8 @@ bool TemplateURLRef::ExtractSearchTermsFromURL(const GURL& url, ...@@ -457,9 +457,8 @@ bool TemplateURLRef::ExtractSearchTermsFromURL(const GURL& url,
// Fill-in the replacements. We don't care about search terms in the pattern, // Fill-in the replacements. We don't care about search terms in the pattern,
// so we use the empty string. // so we use the empty string.
GURL pattern(ReplaceSearchTerms(SearchTermsArgs(string16()))); GURL pattern(ReplaceSearchTerms(SearchTermsArgs(string16())));
// Scheme, host, path and port must match. // Host, path and port must match.
if (!url.SchemeIs(pattern.scheme().c_str()) || if (url.port() != pattern.port() ||
url.port() != pattern.port() ||
url.host() != host_ || url.host() != host_ ||
url.path() != path_) { url.path() != path_) {
return false; return false;
......
...@@ -1189,7 +1189,8 @@ const PrepopulatedEngine google = { ...@@ -1189,7 +1189,8 @@ const PrepopulatedEngine google = {
"{google:baseURL}webhp?sourceid=chrome-instant&{google:RLZ}" "{google:baseURL}webhp?sourceid=chrome-instant&{google:RLZ}"
"{google:instantEnabledParameter}ie={inputEncoding}", "{google:instantEnabledParameter}ie={inputEncoding}",
"[\"{google:baseURL}#q={searchTerms}\", " "[\"{google:baseURL}#q={searchTerms}\", "
"\"{google:baseURL}search#q={searchTerms}\"]", "\"{google:baseURL}search#q={searchTerms}\", "
"\"{google:baseURL}webhp#q={searchTerms}\"]",
SEARCH_ENGINE_GOOGLE, SEARCH_ENGINE_GOOGLE,
1, 1,
}; };
...@@ -3509,7 +3510,7 @@ void RegisterUserPrefs(PrefService* prefs) { ...@@ -3509,7 +3510,7 @@ void RegisterUserPrefs(PrefService* prefs) {
int GetDataVersion(PrefService* prefs) { int GetDataVersion(PrefService* prefs) {
// Increment this if you change the above data in ways that mean users with // Increment this if you change the above data in ways that mean users with
// existing data should get a new version. // existing data should get a new version.
const int kCurrentDataVersion = 44; const int kCurrentDataVersion = 45;
// Allow tests to override the local version. // Allow tests to override the local version.
return (prefs && prefs->HasPrefPath(prefs::kSearchProviderOverridesVersion)) ? return (prefs && prefs->HasPrefPath(prefs::kSearchProviderOverridesVersion)) ?
prefs->GetInteger(prefs::kSearchProviderOverridesVersion) : prefs->GetInteger(prefs::kSearchProviderOverridesVersion) :
......
...@@ -698,9 +698,9 @@ TEST_F(TemplateURLTest, ExtractSearchTermsFromURL) { ...@@ -698,9 +698,9 @@ TEST_F(TemplateURLTest, ExtractSearchTermsFromURL) {
GURL("http://google.com/foo/?q=foo"), &result)); GURL("http://google.com/foo/?q=foo"), &result));
EXPECT_EQ(string16(), result); EXPECT_EQ(string16(), result);
EXPECT_FALSE(url.ExtractSearchTermsFromURL( EXPECT_TRUE(url.ExtractSearchTermsFromURL(
GURL("https://google.com/?q=foo"), &result)); GURL("https://google.com/?q=foo"), &result));
EXPECT_EQ(string16(), result); EXPECT_EQ(ASCIIToUTF16("foo"), result);
EXPECT_FALSE(url.ExtractSearchTermsFromURL( EXPECT_FALSE(url.ExtractSearchTermsFromURL(
GURL("http://google.com:8080/?q=foo"), &result)); GURL("http://google.com:8080/?q=foo"), &result));
......
...@@ -212,8 +212,9 @@ string16 ToolbarModelImpl::TryToExtractSearchTermsFromURL( ...@@ -212,8 +212,9 @@ string16 ToolbarModelImpl::TryToExtractSearchTermsFromURL(
const GURL& url) const { const GURL& url) const {
Profile* profile = GetProfile(); Profile* profile = GetProfile();
// Ensure instant extended API is enabled. // Ensure instant extended API is enabled and query URL is HTTPS.
if (!profile || !chrome::search::IsInstantExtendedAPIEnabled(profile)) if (!profile || !chrome::search::IsInstantExtendedAPIEnabled(profile) ||
!url.SchemeIs(chrome::kHttpsScheme))
return string16(); return string16();
TemplateURLService* template_url_service = TemplateURLService* template_url_service =
......
...@@ -83,17 +83,17 @@ struct TestItem { ...@@ -83,17 +83,17 @@ struct TestItem {
true true
}, },
{ {
GURL("http://google.ca/search?q=tractor+supply"), GURL("https://google.ca/search?q=tractor+supply"),
ASCIIToUTF16("google.ca/search?q=tractor+supply"), ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
ASCIIToUTF16("google.ca/search?q=tractor+supply"), ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
ASCIIToUTF16("google.ca/search?q=tractor+supply"), ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
false, false,
true true
}, },
{ {
GURL("http://google.com/search?q=tractor+supply"), GURL("https://google.com/search?q=tractor+supply"),
ASCIIToUTF16("google.com/search?q=tractor+supply"), ASCIIToUTF16("https://google.com/search?q=tractor+supply"),
ASCIIToUTF16("google.com/search?q=tractor+supply"), ASCIIToUTF16("https://google.com/search?q=tractor+supply"),
ASCIIToUTF16("tractor supply"), ASCIIToUTF16("tractor supply"),
true, true,
true true
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment