Unpunycode search keywords and short names.

components/search_engines/template_url_fetcher_unittest.cc

Index: components/search_engines/template_url_fetcher_unittest.cc
diff --git a/components/search_engines/template_url_fetcher_unittest.cc b/components/search_engines/template_url_fetcher_unittest.cc
index d69f57d153ad7bcb9bc19812615beee82fe1ef90..2d5d45ef17eaf30674e68d38ad59238e25d4b794 100644
--- a/components/search_engines/template_url_fetcher_unittest.cc
+++ b/components/search_engines/template_url_fetcher_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
 #include "base/callback_helpers.h"
 #include "base/files/file_util.h"
 #include "base/memory/scoped_ptr.h"
@@ -323,3 +325,33 @@ TEST_F(TemplateURLFetcherTest, DuplicateDownloadTest) {
   ASSERT_EQ(2, callbacks_destroyed());
   ASSERT_TRUE(last_callback_template_url());
 }
+
+TEST_F(TemplateURLFetcherTest, PunycodeTest) {
+  base::string16 keyword(base::UTF8ToUTF16("\xd1\x82\xd0\xb5\xd1\x81\xd1\x82"));
+
+  test_util()->ChangeModelToLoadState();
+  ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword));
+
+  std::string osdd_file_name("punycode_open_search.xml");

[Matt Giuca, 2015/07/17 04:13:11]

This test is not necessary since we do not expect ShortNames to be IDN-encoded.
(And if they are, we should not do anything special in that case.)

[alshabalin, 2015/07/20 14:06:38]

Done.

+  StartDownload(keyword, osdd_file_name,
+                TemplateURLFetcher::AUTODETECTED_PROVIDER, true);
+  WaitForDownloadToFinish();
+  const TemplateURL* t_url =
+      test_util()->model()->GetTemplateURLForKeyword(keyword);
+  EXPECT_EQ(keyword, t_url->short_name());
+}
+
+TEST_F(TemplateURLFetcherTest, UnicodeTest) {

[Matt Giuca, 2015/07/17 04:13:11]

This is a good test! I'm kind of surprised that none of the other tests in this
file actually check *any* fields of the TemplateURL object they get back from
GetTemplateURLForKeyword. If you're interested, you could go to the above tests
and add some assertions to that effect.

(i.e., this is the very first assertion in this file for t-url->short_name().)

[alshabalin, 2015/07/20 14:06:38]

Added a short_name() assertion to BasicAutodectedTest.

+  base::string16 keyword(base::UTF8ToUTF16("\xd1\x82\xd0\xb5\xd1\x81\xd1\x82"));
+
+  test_util()->ChangeModelToLoadState();
+  ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword));
+
+  std::string osdd_file_name("unicode_open_search.xml");
+  StartDownload(keyword, osdd_file_name,
+                TemplateURLFetcher::AUTODETECTED_PROVIDER, true);
+  WaitForDownloadToFinish();
+  const TemplateURL* t_url =
+      test_util()->model()->GetTemplateURLForKeyword(keyword);
+  EXPECT_EQ(keyword, t_url->short_name());

[Matt Giuca, 2015/07/17 04:13:11]

It's not clear why you expect short_name to equal keyword (other than the
coincidence that the keyword and short name are the same string). For a more
robust test, can you change the keyword to be a different string to the short
name, and introduce a new string "expected_short_name" for this test?

[alshabalin, 2015/07/20 14:06:37]

Done. (Although I didn't introduce expected_short_name, making expectation
base::UTF8ToUTF16 of a string literal)

+}