Properly decode IDN in interstitials

chrome/browser/interstitials/security_interstitial_page_browsertest.cc

Index: chrome/browser/interstitials/security_interstitial_page_browsertest.cc
diff --git a/chrome/browser/interstitials/security_interstitial_page_browsertest.cc b/chrome/browser/interstitials/security_interstitial_page_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b9545809475af932333d2ccc384f61da46c5f45c
--- /dev/null
+++ b/chrome/browser/interstitials/security_interstitial_page_browsertest.cc
@@ -0,0 +1,78 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/interstitials/security_interstitial_page.h"
+
+#include "base/macros.h"
+#include "base/prefs/pref_service.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/interstitial_page.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+class SecurityInterstitialPageIDNTest : public InProcessBrowserTest {
+ public:
+  SecurityInterstitialPageIDNTest() {}
+  // InProcessBrowserTest:
+  void SetUpOnMainThread() override {
+    // Clear AcceptLanguages to force punycode decoding.
+    browser()->profile()->GetPrefs()->SetString(prefs::kAcceptLanguages,
+                                                std::string());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SecurityInterstitialPageIDNTest);
+};
+
+// A fake interstitial page for testing the output of
+// |GetFormattedHostName|.
+class TestSecurityInterstitialPage : public SecurityInterstitialPage {
+ public:
+  TestSecurityInterstitialPage(content::WebContents* web_contents,
+                               const GURL& url)
+      : SecurityInterstitialPage(web_contents, url) {}
+  ~TestSecurityInterstitialPage() override {}
+
+  void ExpectHostname(const std::string& expected_hostname) {
+    EXPECT_EQ(expected_hostname, base::UTF16ToUTF8(GetFormattedHostName()));
+  }
+
+ protected:
+  // SecurityInterstitialPage implementation
+  bool ShouldCreateNewNavigation() const override { return true; }
+  void PopulateInterstitialStrings(
+      base::DictionaryValue* load_time_data) override {}
+};
+
+// Test that |GetFormattedHostname| properly decodes IDN.
+IN_PROC_BROWSER_TEST_F(SecurityInterstitialPageIDNTest,

[estark, 2015/03/24 19:10:13]

This is really a unit test for |GetFormattedHostName|, but it is written as a
browser test to get a WebContents. Not sure if that's correct or not.

[meacer, 2015/03/24 20:22:24]

I agree, it looks like this is testing |GetFormattedHostName| instead. How about
not writing a browsertest for |SecurityInterstitialPage| but writing a base
class that all implementors of |SecurityInterstitialPage| can derive in their
tests? (see my comments in ssl_browser_tests.cc)

[estark, 2015/03/24 22:35:47]

Done.

+                       DecodeIDNInInterstitials) {
+  const char kHostname[] = "xn--d1abbgf6aiiy.xn--p1ai";
+  const char kHostnameUnicode[] =
+      "\u043f\u0440\u0435\u0437\u0438\u0434\u0435\u043d\u0442."
+      "\u0440\u0444";
+
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  DCHECK(contents);
+
+  std::string url_spec = base::StringPrintf("https://%s", kHostname);
+  GURL url(url_spec);
+
+  TestSecurityInterstitialPage* interstitial =
+      new TestSecurityInterstitialPage(contents, url);
+  interstitial->ExpectHostname(kHostnameUnicode);
+}
+
+}  // namespace