Make interstitial links open in a new tab

chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

 // Copyright (c) 2012 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.
 //
 // This test creates a fake safebrowsing service, where we can inject known-
 // threat urls.  It then uses a real browser to go to these urls, and sends
 // "goback" or "proceed" commands and verifies they work.
 
 #include <algorithm>
 
@@ skipping 24 matching lines @@
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/browser/threat_details.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
 #include "components/safe_browsing_db/database_manager.h"
 #include "components/safe_browsing_db/test_database_manager.h"
 #include "components/safe_browsing_db/util.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_interstitials/core/urls.h"
 #include "components/security_state/core/security_state.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/security_style_explanations.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/mock_cert_verifier.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/url_request/url_request_mock_http_job.h"
 #include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using chrome_browser_interstitials::SecurityInterstitialIDNTest;
 using content::BrowserThread;
@@ skipping 652 matching lines @@
     EXPECT_EQ(security_state::MALICIOUS_CONTENT_STATUS_NONE,
               security_info.malicious_content_status);
   }
 
   bool hit_report_sent() {
     return static_cast<FakeSafeBrowsingUIManager*>(
                factory_.test_safe_browsing_service()->ui_manager().get())
         ->hit_report_sent();
   }
 
+  void MockHelpCenterUrl(InterstitialPage* interstitial_page) {

[estark, 2017/06/27 00:36:21]

nit: maybe add a brief comment above this method to explain why it exists (e.g.:
"Modify the interstitial to use a test server URL for its help center link.")

[sperigo, 2017/06/27 17:31:58]

Done.

+    ASSERT_TRUE(https_server_.Start());
+    scoped_refptr<net::X509Certificate> cert(https_server_.GetCertificate());
+    net::CertVerifyResult verify_result;
+    verify_result.is_issued_by_known_root = true;
+    verify_result.verified_cert = cert;
+    verify_result.cert_status = 0;
+    mock_cert_verifier()->AddResultForCert(cert.get(), verify_result, net::OK);
+
+    SafeBrowsingBlockingPage* sb_interstitial =
+        static_cast<SafeBrowsingBlockingPage*>(
+            interstitial_page->GetDelegateForTesting());
+    security_interstitials::SecurityInterstitialControllerClient* client =
+        sb_interstitial->controller();
+
+    const GURL mock_help_center_url = https_server_.GetURL("/title1.html");

[estark, 2017/06/27 00:36:21]

nit: const GURL&
or just inline it below:
SetBaseHelpCenterUrlForTesting(https_server_.GetURL(...))

[sperigo, 2017/06/27 17:31:58]

Done.

+    client->SetBaseHelpCenterUrlForTesting(mock_help_center_url);
+  }
+
  protected:
   TestThreatDetailsFactory details_factory_;
 
  private:
   // Adds a safebrowsing result of the current test threat to the fake
   // safebrowsing service, navigates to that page, and returns the url.
   // The various wrappers supply different URLs.
   GURL SetupWarningAndNavigateToURL(GURL url, Browser* browser) {
     SetURLThreatType(url, testing::get<0>(GetParam()));
     ui_test_utils::NavigateToURL(browser, url);
@@ skipping 46 matching lines @@
   EXPECT_TRUE(ClickAndWaitForDetach("primary-button"));
 
   AssertNoInterstitial(false);   // Assert the interstitial is gone
   EXPECT_EQ(GURL(url::kAboutBlankURL),  // Back to "about:blank"
             browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, VisitWhitePaper) {
   SetupWarningAndNavigate(browser());
 
+  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+  WebContents* interstitial_tab =

[estark, 2017/06/27 00:36:21]

nit: ASSERT_TRUE(interstitial_tab) after this line to make the test fail nicely
instead of crashing if for some reason the tab doesn't exist

[sperigo, 2017/06/27 17:31:58]

Done.

+      browser()->tab_strip_model()->GetActiveWebContents();
+
   EXPECT_EQ(VISIBLE, GetVisibility("whitepaper-link"));
-  EXPECT_TRUE(ClickAndWaitForDetach("whitepaper-link"));
+  content::TestNavigationObserver nav_observer(nullptr);
+  nav_observer.StartWatchingNewWebContents();
+  EXPECT_TRUE(Click("whitepaper-link"));
 
-  AssertNoInterstitial(false);  // Assert the interstitial is gone
-  EXPECT_EQ(GetWhitePaperUrl(),
-            browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
+  nav_observer.Wait();
+
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+
+  AssertNoInterstitial(false);  // Assert the interstitial is not present in

[estark, 2017/06/27 00:36:21]

nit: comment formatting is a little weird, you can probably just it on the line
above

[sperigo, 2017/06/27 17:31:59]

Done.

+                                // the foreground tab.
+
+  // Foreground tab displays the help center.
+  WebContents* new_tab = browser()->tab_strip_model()->GetActiveWebContents();

[estark, 2017/06/27 00:36:21]

nit: ASSERT_TRUE(new_tab) after this line to make the test fail nicely instead
of crashing if for some reason the tab doesn't exist

[sperigo, 2017/06/27 17:31:58]

Done.

+  EXPECT_EQ(GetWhitePaperUrl(), new_tab->GetURL());
+
+  // Interstitial should still display in the background tab.
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+  EXPECT_EQ(interstitial_tab,
+            browser()->tab_strip_model()->GetActiveWebContents());
+  EXPECT_TRUE(YesInterstitial());
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, Proceed) {
   GURL url = SetupWarningAndNavigate(browser());
 
   EXPECT_TRUE(ClickAndWaitForDetach("proceed-link"));
   AssertNoInterstitial(true);  // Assert the interstitial is gone.
   EXPECT_EQ(url,
             browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
 }
@@ skipping 288 matching lines @@
 
   // Check navigation entry state.
   ASSERT_TRUE(controller.GetVisibleEntry());
   EXPECT_EQ(url, controller.GetVisibleEntry()->GetURL());
   ASSERT_TRUE(controller.GetPendingEntry());
   EXPECT_EQ(url, controller.GetPendingEntry()->GetURL());
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest, LearnMore) {
   SetupWarningAndNavigate(browser());
-  EXPECT_TRUE(ClickAndWaitForDetach("learn-more-link"));
-  AssertNoInterstitial(false);  // Assert the interstitial is gone
 
-  // We are in the help page.
-  EXPECT_EQ(
-      GURL("https://support.google.com/chrome/answer/99020").GetWithEmptyPath(),
-      browser()
-          ->tab_strip_model()
-          ->GetActiveWebContents()
-          ->GetURL()
-          .GetWithEmptyPath());
+  WebContents* interstitial_tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(interstitial_tab);
+
+  MockHelpCenterUrl(interstitial_tab->GetInterstitialPage());
+
+  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+
+  content::TestNavigationObserver nav_observer(nullptr);
+  nav_observer.StartWatchingNewWebContents();
+  SendCommand(security_interstitials::CMD_OPEN_HELP_CENTER);
+  nav_observer.Wait();
+
+  // A new tab has been opened.
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+
+  // Interstitial does not display in the foreground tab.
+  EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+  WebContents* new_tab = browser()->tab_strip_model()->GetWebContentsAt(1);

[estark, 2017/06/27 00:36:21]

same nit about ASSERT_TRUE(new_tab)

[sperigo, 2017/06/27 17:31:58]

Done.

+  EXPECT_FALSE(new_tab->ShowingInterstitialPage());
+
+  // Interstitial still displays in the background tab.
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+  EXPECT_EQ(interstitial_tab,
+            browser()->tab_strip_model()->GetActiveWebContents());
+  EXPECT_TRUE(YesInterstitial());
 }
 
 IN_PROC_BROWSER_TEST_P(SafeBrowsingBlockingPageBrowserTest,
                        Histograms_DontProceed) {
   base::HistogramTester histograms;
   std::string prefix;
   SBThreatType threat_type = testing::get<0>(GetParam());
   if (threat_type == SB_THREAT_TYPE_URL_MALWARE)
     prefix = "malware";
   else if (threat_type == SB_THREAT_TYPE_URL_PHISHING)
@@ skipping 458 matching lines @@
 
 INSTANTIATE_TEST_CASE_P(
     SafeBrowsingBlockingPageIDNTestWithThreatType,
     SafeBrowsingBlockingPageIDNTest,
     testing::Combine(testing::Values(false, true),
                      testing::Values(SB_THREAT_TYPE_URL_MALWARE,
                                      SB_THREAT_TYPE_URL_PHISHING,
                                      SB_THREAT_TYPE_URL_UNWANTED)));
 
 }  // namespace safe_browsing