Commit dd3cf62f authored by Yi Su's avatar Yi Su Committed by Commit Bot

Support adding TemplateURL by searchable URL in SearchEngineTabHelper.

Enable SearchEngineTabHelper to add new TemplateURL to TemplateURLService
by following steps:
    1. When a searchable URL is generated from <form> submission, save
       it to SearchEngineTabHelper;
    2. If the navigation caused by <form> successfully finishes, create a
       new TemplateURL by recorded searchable URL and add it to
       TemplateURLService, then erase the recorded url. Failed navigation
       will be ignored because in that condition the generated searchable
       URL will probably lead user into an error page;

Bug: 433824
Change-Id: I1835f2e12e9bc3a1d613011b5b7dd38206c019ad
Reviewed-on: https://chromium-review.googlesource.com/c/1317909Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605997}
parent ce828beb
...@@ -180,6 +180,12 @@ https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/exported/we ...@@ -180,6 +180,12 @@ https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/exported/we
* searchableURL. * searchableURL.
*/ */
function generateSearchableUrl_(form) { function generateSearchableUrl_(form) {
// Only consider <form> that navigates in current frame, because new
// TemplateURL will only be created from navigation in the same page:
// https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc?l=177&gsn=last_index
if (form.target && form.target != '_self')
return;
// Only consider forms that GET data. // Only consider forms that GET data.
if (form.method && form.method.toLowerCase() != 'get') { if (form.method && form.method.toLowerCase() != 'get') {
return; return;
......
...@@ -42,10 +42,15 @@ class SearchEngineTabHelper ...@@ -42,10 +42,15 @@ class SearchEngineTabHelper
explicit SearchEngineTabHelper(web::WebState* web_state); explicit SearchEngineTabHelper(web::WebState* web_state);
// Creates a TemplateURL by downloading and parsing the OSDD // Adds a TemplateURL by downloading and parsing the OSDD.
void AddTemplateURLByOSDD(const GURL& page_url, const GURL& osdd_url); void AddTemplateURLByOSDD(const GURL& page_url, const GURL& osdd_url);
// Adds a TemplateURL by |searchable_url|.
void AddTemplateURLBySearchableURL(const GURL& searchable_url);
// WebStateObserver implementation. // WebStateObserver implementation.
void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override;
void WebStateDestroyed(web::WebState* web_state) override; void WebStateDestroyed(web::WebState* web_state) override;
// Handles messages from JavaScript. Messages can be: // Handles messages from JavaScript. Messages can be:
...@@ -60,6 +65,14 @@ class SearchEngineTabHelper ...@@ -60,6 +65,14 @@ class SearchEngineTabHelper
// WebState this tab helper is attached to. // WebState this tab helper is attached to.
web::WebState* web_state_ = nullptr; web::WebState* web_state_ = nullptr;
// The searchable URL generated from <form> submission. This ivar is an empty
// GURL by default. If a web page has a searchable <form>, a searchable URL is
// generated by JavaScript when the <form> is submitted, and stored in this
// ivar. When the navigation triggered by the <form> submission finishes
// successfully, this ivar will be used to add a new TemplateURL and then it
// will be set to empty GURL again.
GURL searchable_url_;
DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelper); DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelper);
}; };
......
...@@ -7,11 +7,14 @@ ...@@ -7,11 +7,14 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "components/search_engines/template_url.h" #include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_fetcher.h" #include "components/search_engines/template_url_fetcher.h"
#include "components/search_engines/template_url_service.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/search_engines/template_url_fetcher_factory.h" #include "ios/chrome/browser/search_engines/template_url_fetcher_factory.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#include "ios/web/public/favicon_status.h" #include "ios/web/public/favicon_status.h"
#import "ios/web/public/navigation_item.h" #import "ios/web/public/navigation_item.h"
#import "ios/web/public/navigation_manager.h" #import "ios/web/public/navigation_manager.h"
#import "ios/web/public/web_state/navigation_context.h"
#include "ui/base/page_transition_types.h" #include "ui/base/page_transition_types.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -27,6 +30,8 @@ const char kCommandPrefix[] = "searchEngine"; ...@@ -27,6 +30,8 @@ const char kCommandPrefix[] = "searchEngine";
const char kCommandOpenSearch[] = "searchEngine.openSearch"; const char kCommandOpenSearch[] = "searchEngine.openSearch";
const char kOpenSearchPageUrlKey[] = "pageUrl"; const char kOpenSearchPageUrlKey[] = "pageUrl";
const char kOpenSearchOsddUrlKey[] = "osddUrl"; const char kOpenSearchOsddUrlKey[] = "osddUrl";
const char kCommandSearchableUrl[] = "searchEngine.searchableUrl";
const char kSearchableUrlUrlKey[] = "url";
// Returns true if the |item|'s transition type is FORM_SUBMIT. // Returns true if the |item|'s transition type is FORM_SUBMIT.
bool IsFormSubmit(const web::NavigationItem* item) { bool IsFormSubmit(const web::NavigationItem* item) {
...@@ -35,7 +40,7 @@ bool IsFormSubmit(const web::NavigationItem* item) { ...@@ -35,7 +40,7 @@ bool IsFormSubmit(const web::NavigationItem* item) {
} }
// Generates a keyword from |item|. This code is based on: // Generates a keyword from |item|. This code is based on:
// https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc?rcl=4e19b43513aa9590ae89d5c68523bc764f40067f&l=41i // https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
base::string16 GenerateKeywordFromNavigationItem( base::string16 GenerateKeywordFromNavigationItem(
const web::NavigationItem* item) { const web::NavigationItem* item) {
// Don't autogenerate keywords for pages that are the result of form // Don't autogenerate keywords for pages that are the result of form
...@@ -85,6 +90,22 @@ void SearchEngineTabHelper::WebStateDestroyed(web::WebState* web_state) { ...@@ -85,6 +90,22 @@ void SearchEngineTabHelper::WebStateDestroyed(web::WebState* web_state) {
web_state_ = nullptr; web_state_ = nullptr;
} }
// When the page is loaded, checks if |searchable_url_| has a value generated
// from the <form> submission before the navigation. If true, and the navigation
// is successful, adds a TemplateURL by |searchable_url_|. |searchable_url_|
// will be set to empty in the end.
void SearchEngineTabHelper::DidFinishNavigation(
web::WebState* web_state,
web::NavigationContext* navigation_context) {
if (!searchable_url_.is_empty()) {
if (!navigation_context->GetError() &&
!navigation_context->IsSameDocument()) {
AddTemplateURLBySearchableURL(searchable_url_);
}
searchable_url_ = GURL();
}
}
bool SearchEngineTabHelper::OnJsMessage(const base::DictionaryValue& message, bool SearchEngineTabHelper::OnJsMessage(const base::DictionaryValue& message,
const GURL& page_url, const GURL& page_url,
bool has_user_gesture, bool has_user_gesture,
...@@ -104,13 +125,23 @@ bool SearchEngineTabHelper::OnJsMessage(const base::DictionaryValue& message, ...@@ -104,13 +125,23 @@ bool SearchEngineTabHelper::OnJsMessage(const base::DictionaryValue& message,
return false; return false;
AddTemplateURLByOSDD(GURL(document_url->GetString()), AddTemplateURLByOSDD(GURL(document_url->GetString()),
GURL(osdd_url->GetString())); GURL(osdd_url->GetString()));
} else if (cmd_str == kCommandSearchableUrl) {
const base::Value* url = message.FindKey(kSearchableUrlUrlKey);
if (!url || !url->is_string())
return false;
// Save |url| to |searchable_url_| when generated from <form> submission,
// and create the TemplateURL when the submission did lead to a successful
// navigation.
searchable_url_ = GURL(url->GetString());
} else {
return false;
} }
return true; return true;
} }
// Creates a new TemplateURL by OSDD. The TemplateURL will be added to // Creates a new TemplateURL by OSDD. The TemplateURL will be added to
// TemplateURLService by TemplateURLFecther. This code is based on: // TemplateURLService by TemplateURLFecther. This code is based on:
// https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc?rcl=50f50d521b18ac53d05c4e159c02bcb609454b8e&l=96 // https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
void SearchEngineTabHelper::AddTemplateURLByOSDD(const GURL& page_url, void SearchEngineTabHelper::AddTemplateURLByOSDD(const GURL& page_url,
const GURL& osdd_url) { const GURL& osdd_url) {
// Checks to see if we should generate a keyword based on the OSDD, and if // Checks to see if we should generate a keyword based on the OSDD, and if
...@@ -164,3 +195,73 @@ void SearchEngineTabHelper::AddTemplateURLByOSDD(const GURL& page_url, ...@@ -164,3 +195,73 @@ void SearchEngineTabHelper::AddTemplateURLByOSDD(const GURL& page_url,
browser_state->GetURLLoaderFactory(), MSG_ROUTING_NONE, browser_state->GetURLLoaderFactory(), MSG_ROUTING_NONE,
/* content::ResourceType::RESOURCE_TYPE_SUB_RESOURCE */ 6); /* content::ResourceType::RESOURCE_TYPE_SUB_RESOURCE */ 6);
} }
// Creates a TemplateURL by |searchable_url| and adds it to TemplateURLService.
// This code is based on:
// https://cs.chromium.org/chromium/src/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
void SearchEngineTabHelper::AddTemplateURLBySearchableURL(
const GURL& searchable_url) {
if (!searchable_url.is_valid()) {
return;
}
ios::ChromeBrowserState* browser_state =
ios::ChromeBrowserState::FromBrowserState(web_state_->GetBrowserState());
// Don't add TemplateURL under incognito mode.
if (browser_state->IsOffTheRecord())
return;
const web::NavigationManager* manager = web_state_->GetNavigationManager();
int last_index = manager->GetLastCommittedItemIndex();
// When there was no previous page, the last index will be 0. This is
// normally due to a form submit that opened in a new tab.
if (last_index <= 0)
return;
const web::NavigationItem* item = manager->GetItemAtIndex(last_index - 1);
base::string16 keyword(GenerateKeywordFromNavigationItem(item));
if (keyword.empty())
return;
TemplateURLService* url_service =
ios::TemplateURLServiceFactory::GetForBrowserState(browser_state);
if (!url_service)
return;
if (!url_service->loaded()) {
url_service->Load();
return;
}
const TemplateURL* current_url;
if (!url_service->CanAddAutogeneratedKeyword(keyword, searchable_url,
&current_url))
return;
if (current_url) {
if (current_url->originating_url().is_valid()) {
// The existing keyword was generated from an OpenSearch description
// document, don't regenerate.
return;
}
url_service->Remove(current_url);
}
TemplateURLData data;
data.SetShortName(keyword);
data.SetKeyword(keyword);
data.SetURL(searchable_url.spec());
const GURL& current_favicon =
manager->GetLastCommittedItem()->GetFavicon().url;
// If the favicon url isn't valid, it means there really isn't a favicon, or
// the favicon url wasn't obtained before the load started. This assumes the
// latter.
if (current_favicon.is_valid()) {
data.favicon_url = current_favicon;
} else if (item->GetReferrer().url.is_valid()) {
data.favicon_url = TemplateURL::GenerateFaviconURL(item->GetReferrer().url);
}
data.safe_for_autoreplace = true;
url_service->Add(std::make_unique<TemplateURL>(data));
}
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ios/chrome/browser/search_engines/template_url_service_factory.h" #include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#include "ios/chrome/browser/web/chrome_web_test.h" #include "ios/chrome/browser/web/chrome_web_test.h"
#import "ios/web/public/test/web_test_with_web_state.h" #import "ios/web/public/test/web_test_with_web_state.h"
#import "ios/web/public/test/web_view_interaction_test_util.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h" #include "net/test/embedded_test_server/http_response.h"
...@@ -23,10 +24,13 @@ ...@@ -23,10 +24,13 @@
using base::test::ios::WaitUntilConditionOrTimeout; using base::test::ios::WaitUntilConditionOrTimeout;
using base::test::ios::kWaitForJSCompletionTimeout; using base::test::ios::kWaitForJSCompletionTimeout;
using web::test::SubmitWebViewFormWithId;
namespace { namespace {
const char kOpenSearchXmlFilePath[] = const char kOpenSearchXmlFilePath[] =
"/ios/testing/data/http_server_files/opensearch.xml"; "/ios/testing/data/http_server_files/opensearch.xml";
const char kPonyHtmlFilePath[] =
"/ios/testing/data/http_server_files/pony.html";
// A BrowserStateKeyedServiceFactory::TestingFactory that creates a testing // A BrowserStateKeyedServiceFactory::TestingFactory that creates a testing
// TemplateURLService. The created TemplateURLService may contain some default // TemplateURLService. The created TemplateURLService may contain some default
...@@ -78,6 +82,15 @@ class SearchEngineTabHelperTest : public ChromeWebTest { ...@@ -78,6 +82,15 @@ class SearchEngineTabHelperTest : public ChromeWebTest {
return [result isEqual:@YES]; return [result isEqual:@YES];
} }
// Sends a message that |searchable_url| is generated from <form> submission.
bool SendMessageOfSearchableUrl(const GURL& searchable_url) {
id result = ExecuteJavaScript([NSString
stringWithFormat:@"__gCrWeb.message.invokeOnHost({'command': "
@"'searchEngine.searchableUrl', 'url' : '%s'}); true;",
searchable_url.spec().c_str()]);
return [result isEqual:@YES];
}
net::EmbeddedTestServer server_; net::EmbeddedTestServer server_;
DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelperTest); DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelperTest);
...@@ -86,7 +99,7 @@ class SearchEngineTabHelperTest : public ChromeWebTest { ...@@ -86,7 +99,7 @@ class SearchEngineTabHelperTest : public ChromeWebTest {
// Tests that SearchEngineTabHelper can add TemplateURL to TemplateURLService // Tests that SearchEngineTabHelper can add TemplateURL to TemplateURLService
// when a OSDD <link> is found in web page. // when a OSDD <link> is found in web page.
TEST_F(SearchEngineTabHelperTest, AddTemplateURLByOpenSearch) { TEST_F(SearchEngineTabHelperTest, AddTemplateURLByOpenSearch) {
GURL page_url("https://chrooome.com"); GURL page_url("https://chromium.test");
GURL osdd_url = server_.GetURL(kOpenSearchXmlFilePath); GURL osdd_url = server_.GetURL(kOpenSearchXmlFilePath);
// Record the original TemplateURLs in TemplateURLService. // Record the original TemplateURLs in TemplateURLService.
...@@ -115,13 +128,58 @@ TEST_F(SearchEngineTabHelperTest, AddTemplateURLByOpenSearch) { ...@@ -115,13 +128,58 @@ TEST_F(SearchEngineTabHelperTest, AddTemplateURLByOpenSearch) {
} }
} }
ASSERT_TRUE(new_url); ASSERT_TRUE(new_url);
EXPECT_EQ(base::UTF8ToUTF16("chrooome.com"), new_url->data().keyword()); EXPECT_EQ(base::UTF8ToUTF16("chromium.test"), new_url->data().keyword());
EXPECT_EQ(base::UTF8ToUTF16("Chrooome"), new_url->data().short_name()); EXPECT_EQ(base::UTF8ToUTF16("Chrooome"), new_url->data().short_name());
EXPECT_EQ( EXPECT_EQ(
"https://chrooome.com/index.php?title=chrooome&search={searchTerms}", "https://chromium.test/index.php?title=chrooome&search={searchTerms}",
new_url->data().url()); new_url->data().url());
} }
// Tests that SearchEngineTabHelper can add TemplateURL to TemplateURLService
// when a <form> submission generates a searchable URL and leads to a successful
// navigation.
TEST_F(SearchEngineTabHelperTest, AddTemplateURLBySearchableURL) {
GURL page_url("https://chromium.test");
GURL searchable_url(
"https://chromium.test/index.php?title=chrooome&search={searchTerms}");
// HTML of the search engine page, with a <form> navigates to pony.html as
// search result page.
NSString* html = [NSString
stringWithFormat:@"<html><form id='f' action='%s'></form></html>",
server_.GetURL(kPonyHtmlFilePath).spec().c_str()];
// Record the original TemplateURLs in TemplateURLService.
std::vector<TemplateURL*> old_urls =
template_url_service()->GetTemplateURLs();
// Load an empty page, and send a message of openSearchUrl from Js.
LoadHtml(html, page_url);
SendMessageOfSearchableUrl(searchable_url);
SubmitWebViewFormWithId(web_state(), "f");
// Wait for TemplateURL added to TemplateURLService.
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
base::RunLoop().RunUntilIdle();
return template_url_service()->GetTemplateURLs().size() ==
old_urls.size() + 1;
}));
// TemplateURLService doesn't guarantee that newly added TemplateURL is
// appended, so we should check in a general way that only one TemplateURL is
// added and others remain untouched.
TemplateURL* new_url = nullptr;
for (TemplateURL* url : template_url_service()->GetTemplateURLs()) {
if (std::find(old_urls.begin(), old_urls.end(), url) == old_urls.end()) {
ASSERT_FALSE(new_url);
new_url = url;
}
}
ASSERT_TRUE(new_url);
EXPECT_EQ(base::UTF8ToUTF16("chromium.test"), new_url->data().keyword());
EXPECT_EQ(base::UTF8ToUTF16("chromium.test"), new_url->data().short_name());
EXPECT_EQ(searchable_url.spec(), new_url->data().url());
}
// Test fixture for SearchEngineTabHelper class in incognito mode. // Test fixture for SearchEngineTabHelper class in incognito mode.
class SearchEngineTabHelperIncognitoTest : public SearchEngineTabHelperTest { class SearchEngineTabHelperIncognitoTest : public SearchEngineTabHelperTest {
protected: protected:
...@@ -140,7 +198,7 @@ class SearchEngineTabHelperIncognitoTest : public SearchEngineTabHelperTest { ...@@ -140,7 +198,7 @@ class SearchEngineTabHelperIncognitoTest : public SearchEngineTabHelperTest {
// mode. // mode.
TEST_F(SearchEngineTabHelperIncognitoTest, TEST_F(SearchEngineTabHelperIncognitoTest,
NotAddTemplateURLByOpenSearchUnderIncognito) { NotAddTemplateURLByOpenSearchUnderIncognito) {
GURL page_url("https://chrooome.com"); GURL page_url("https://chromium.test");
GURL osdd_url = server_.GetURL(kOpenSearchXmlFilePath); GURL osdd_url = server_.GetURL(kOpenSearchXmlFilePath);
// Record the original TemplateURLs in TemplateURLService. // Record the original TemplateURLs in TemplateURLService.
...@@ -159,3 +217,35 @@ TEST_F(SearchEngineTabHelperIncognitoTest, ...@@ -159,3 +217,35 @@ TEST_F(SearchEngineTabHelperIncognitoTest,
})); }));
EXPECT_EQ(old_urls, template_url_service()->GetTemplateURLs()); EXPECT_EQ(old_urls, template_url_service()->GetTemplateURLs());
} }
// Tests that SearchEngineTabHelper doesn't add TemplateURL to
// TemplateURLService when a <form> submission generates a searchable URL and
// leads to a successful navigation under incognito mode.
TEST_F(SearchEngineTabHelperIncognitoTest,
NotAddTemplateURLBySearchableURLUnderIncognito) {
GURL page_url("https://chromium.test");
GURL searchable_url(
"https://chromium.test/index.php?title=chrooome&search={searchTerms}");
// HTML of the search engine page, with a <form> navigates to pony.html as
// search result page.
NSString* html = [NSString
stringWithFormat:@"<html><form id='f' action='%s'></form></html>",
server_.GetURL(kPonyHtmlFilePath).spec().c_str()];
// Record the original TemplateURLs in TemplateURLService.
std::vector<TemplateURL*> old_urls =
template_url_service()->GetTemplateURLs();
// Load an empty page, and send a message of openSearchUrl from Js.
LoadHtml(html, page_url);
SendMessageOfSearchableUrl(searchable_url);
SubmitWebViewFormWithId(web_state(), "f");
// Wait for TemplateURL added to TemplateURLService.
ASSERT_FALSE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
base::RunLoop().RunUntilIdle();
return template_url_service()->GetTemplateURLs().size() ==
old_urls.size() + 1;
}));
EXPECT_EQ(old_urls, template_url_service()->GetTemplateURLs());
}
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
<OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/" xmlns:moz="http://www.mozilla.org/2006/browser/search/"> <OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/" xmlns:moz="http://www.mozilla.org/2006/browser/search/">
<ShortName>Chrooome</ShortName> <ShortName>Chrooome</ShortName>
<Description>Chrooome</Description> <Description>Chrooome</Description>
<Image height="16" width="16" type="image/x-icon">https://chrooome.com/favicon.ico</Image> <Image height="16" width="16" type="image/x-icon">https://chromium.test/favicon.ico</Image>
<Url type="text/html" method="get" template="https://chrooome.com/index.php?title=chrooome&amp;search={searchTerms}" /> <Url type="text/html" method="get" template="https://chromium.test/index.php?title=chrooome&amp;search={searchTerms}" />
<moz:SearchForm>https://en.cppreference.com/w/Special:Search</moz:SearchForm>
</OpenSearchDescription> </OpenSearchDescription>
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