Make interstitial links open in a new tab

chrome/browser/ssl/ssl_browser_tests.cc

Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 9f72ac7c7f02c5ec1e91b1e745d51af683f86b7a..4f1a8fe8291bb5b292543a943450aac51129ebad 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -64,6 +64,7 @@
 #include "components/network_time/network_time_tracker.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/security_interstitials/content/security_interstitial_controller_client.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/security_state/core/security_state.h"
@@ -135,6 +136,7 @@ using content::NavigationController;
 using content::NavigationEntry;
 using content::SSLStatus;
 using content::WebContents;
+using security_interstitials::SecurityInterstitialControllerClient;
 using web_modal::WebContentsModalDialogManager;
 
 const base::FilePath::CharType kDocRoot[] =
@@ -663,6 +665,12 @@ class SSLUITest : public InProcessBrowserTest {
     }
   }
 
+  // Helper function for TestInterstitialLinksOpenInNewTab.
+  security_interstitials::SecurityInterstitialControllerClient*
+  GetControllerClientFromInterstitialPage(SSLBlockingPage* ssl_interstitial) {
+    return ssl_interstitial->controller();
+  }
+
   net::EmbeddedTestServer https_server_;
   net::EmbeddedTestServer https_server_expired_;
   net::EmbeddedTestServer https_server_mismatched_;
@@ -2972,8 +2980,65 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialJavaScriptGoesBack) {
   EXPECT_EQ("about:blank", tab->GetVisibleURL().spec());
 }
 
+// Verifies that links in the interstitial open in a new tab.
+// https://crbug.com/717616
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialLinksOpenInNewTab) {
+  HostContentSettingsMapFactory::GetForProfile(browser()->profile())
+      ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT,
+                                 CONTENT_SETTING_BLOCK);

[meacer, 2017/06/23 20:28:32]

This will block javascript on all pages, shouldn't be necessary for this test
case.

[sperigo, 2017/06/23 21:32:11]

Done.

+
+  ASSERT_TRUE(https_server_.Start());
+  ASSERT_TRUE(https_server_expired_.Start());
+
+  WebContents* interstitial_tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_expired_.GetURL("/ssl/google.html"));
+  content::WaitForInterstitialAttach(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  InterstitialPage* interstitial_page = interstitial_tab->GetInterstitialPage();
+  ASSERT_TRUE(
+      content::WaitForRenderFrameReady(interstitial_page->GetMainFrame()));
+  CheckAuthenticationBrokenState(interstitial_tab,
+                                 net::CERT_STATUS_DATE_INVALID,
+                                 AuthState::SHOWING_INTERSTITIAL);
+  ASSERT_EQ(SSLBlockingPage::kTypeForTesting,
+            interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+
+  content::TestNavigationObserver nav_observer(nullptr);
+  nav_observer.StartWatchingNewWebContents();
+
+  SSLBlockingPage* ssl_interstitial =
+      static_cast<SSLBlockingPage*>(interstitial_page->GetDelegateForTesting());
+  security_interstitials::SecurityInterstitialControllerClient* client =
+      GetControllerClientFromInterstitialPage(ssl_interstitial);
+
+  // Mocking out the help center URL so that our test will hit the test server
+  // instead of a real server.
+  // NOTE: The CMD_OPEN_HELP_CENTER code in
+  // components/security_interstitials/core/ssl_error_ui.cc ends up appending
+  // a path to whatever URL is passed to it. Since that path doesn't exist on
+  // our test server, this results in a 404. This is expected behavior, and
+  // things are still working as expected so long as the test passes!
+  GURL mock_help_center_url = https_server_.GetURL("/title1.html");

[meacer, 2017/06/23 20:28:31]

nit: You can make this const as it's a read only variable. Makes it slightly
easier to reason about the code.

(const GURL mock_help_center_url = ...)

[sperigo, 2017/06/23 21:32:11]

Done.

+  client->SetHelpCenterUrlForTesting(mock_help_center_url);
+
+  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+
+  int result = -1;
+  std::string javascript =

[meacer, 2017/06/23 20:28:31]

nit: Also const

[sperigo, 2017/06/23 21:32:11]

Done.

+      base::StringPrintf("window.domAutomationController.send(%d);",
+                         security_interstitials::CMD_OPEN_HELP_CENTER);
+  ASSERT_TRUE(content::ExecuteScriptAndExtractInt(
+      interstitial_page->GetMainFrame(), javascript, &result));
+  EXPECT_EQ(security_interstitials::CMD_OPEN_HELP_CENTER, result);
+
+  nav_observer.Wait();
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+}

[meacer, 2017/06/23 20:28:32]

As an extra, you can also check the URL of the newly opened tab here.

Since the new tab will be foreground,
browser()->tab_strip_model()->GetActiveWebContents() should give you the new
tab, and then you can check tab->GetURL() value on that new tab.

[sperigo, 2017/06/23 21:32:11]

Done.

[sperigo, 2017/06/23 21:32:11]

Good idea!

+
 // Verifies that switching tabs, while showing interstitial page, will not
-// affect the visibility of the interestitial.
+// affect the visibility of the interstitial.
 // https://crbug.com/381439
 IN_PROC_BROWSER_TEST_F(SSLUITest, InterstitialNotAffectedByHideShow) {
   ASSERT_TRUE(https_server_expired_.Start());