Omnibox - Open Search - Handle Lack of Short Name Smartly

BUG=714081 Review-Url: https://codereview.chromium.org/2902043004 Cr-Commit-Position: refs/heads/master@{#476125}
cbchan · Jun 1, 2017 · ee5a39c · ee5a39c
1 parent 6995b86
commit ee5a39c
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 4 deletions.
diff --git a/chrome/browser/search_engines/template_url_fetcher_unittest.cc b/chrome/browser/search_engines/template_url_fetcher_unittest.cc
@@ -182,6 +182,25 @@ TEST_F(TemplateURLFetcherTest, BasicAutodetectedTest) {
   EXPECT_TRUE(t_url->safe_for_autoreplace());
 }
 
+// This test is similar to the BasicAutodetectedTest except the xml file
+// provided doesn't include a short name for the search engine.  We should
+// fall back to the hostname.
+TEST_F(TemplateURLFetcherTest, InvalidShortName) {
+  base::string16 keyword(ASCIIToUTF16("test"));
+
+  test_util()->ChangeModelToLoadState();
+  ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword));
+
+  std::string osdd_file_name("simple_open_search_no_name.xml");
+  StartDownload(keyword, osdd_file_name, true);
+  WaitForDownloadToFinish();
+
+  const TemplateURL* t_url =
+      test_util()->model()->GetTemplateURLForKeyword(keyword);
+  ASSERT_TRUE(t_url);
+  EXPECT_EQ(ASCIIToUTF16("example.com"), t_url->short_name());
+}
+
 TEST_F(TemplateURLFetcherTest, DuplicatesThrownAway) {
   base::string16 keyword(ASCIIToUTF16("test"));
 

diff --git a/chrome/test/data/simple_open_search_no_name.xml b/chrome/test/data/simple_open_search_no_name.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0"?>
+<OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/">
+<ShortName/>
+<Description>Not a creative description.</Description>
+<Url type="text/html" template="http://example.com/{searchTerms}/other_stuff"/>
+</OpenSearchDescription>
diff --git a/components/search_engines/template_url_data.cc b/components/search_engines/template_url_data.cc
@@ -78,8 +78,6 @@ TemplateURLData::~TemplateURLData() {
 }
 
 void TemplateURLData::SetShortName(const base::string16& short_name) {
-  DCHECK(!short_name.empty());
-
   // Remove tabs, carriage returns, and the like, as they can corrupt
   // how the short name is displayed.
   short_name_ = base::CollapseWhitespace(short_name, true);

diff --git a/components/search_engines/template_url_parser.cc b/components/search_engines/template_url_parser.cc
@@ -303,8 +303,7 @@ void TemplateURLParsingContext::CharactersImpl(void* ctx,
 std::unique_ptr<TemplateURL> TemplateURLParsingContext::GetTemplateURL(
     const SearchTermsData& search_terms_data) {
   // TODO(jcampan): Support engines that use POST; see http://crbug.com/18107
-  if (method_ == TemplateURLParsingContext::POST ||
-      data_.short_name().empty() || !IsHTTPRef(data_.url()) ||
+  if (method_ == TemplateURLParsingContext::POST || !IsHTTPRef(data_.url()) ||
       !IsHTTPRef(data_.suggestions_url))
     return nullptr;
   if (suggestion_method_ == TemplateURLParsingContext::POST)
@@ -321,6 +320,10 @@ std::unique_ptr<TemplateURL> TemplateURLParsingContext::GetTemplateURL(
   if (!has_custom_keyword_)
     data_.SetKeyword(TemplateURL::GenerateKeyword(search_url));
 
+  // If the OSDD omits or has an empty short name, use the keyword.
+  if (data_.short_name().empty())
+    data_.SetShortName(data_.keyword());
+
   // Bail if the search URL is empty or if either TemplateURLRef is invalid.
   std::unique_ptr<TemplateURL> template_url =
       base::MakeUnique<TemplateURL>(data_);

diff --git a/components/test/data/search_engines/simple_open_search_no_name.xml b/components/test/data/search_engines/simple_open_search_no_name.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0"?>
+<OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/">
+<ShortName/>
+<Description>Not a creative description.</Description>
+<Url type="text/html" template="http://example.com/{searchTerms}/other_stuff"/>
+</OpenSearchDescription>