Add is_captive_portal_popup param to NavigateParams.

This param will be applied to any WebContents that are newly
created during the navigation so that the WebContents can be
properly classified before any DNS requests are made. (For
WebContents that are created for captive portal resolution in
secure mode, we need to disable secure DNS.)

Bug: 10161646
Change-Id: Ia38cd9b0d6013cae7f272cc42302bba875152e58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026349
Commit-Queue: Katharine Daly <dalyk@google.com>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738300}

chrome/browser/captive_portal/captive_portal_browsertest.cc

@@ -168,6 +168,10 @@ const char* const kMockHttpsConnectionSecureDnsErr =
 // captive portal.
 const char* const kInternetConnectedTitle = "Title Of Awesomeness";
 
+// Expected title of a login page that was created in secure mode.
+const char* const kLoginSecureDnsDisabledTitle =
+    "Fake Login Page Secure Dns Disabled";
+
 // Creates a server-side redirect for use with the TestServer.
 std::string CreateServerRedirect(const std::string& dest_url) {
   const char* const kServerRedirectBase = "/server-redirect?";
@@ -1050,7 +1054,12 @@ bool CaptivePortalBrowserTest::OnIntercept(
         contents = GetContents("captive_portal/page511.html");
         headers = "HTTP/1.1 511 Network Authentication Required\n";
       } else {
-        contents = GetContents("captive_portal/login.html");
+        // These URLs should only be requested for navigations, which will have
+        // trusted_params.
+        contents =
+            params->url_request.trusted_params->disable_secure_dns
+                ? GetContents("captive_portal/login_secure_dns_disabled.html")
+                : GetContents("captive_portal/login.html");
         headers = "HTTP/1.0 200 Just Peachy\n";
       }
     } else {
@@ -1350,6 +1359,8 @@ void CaptivePortalBrowserTest::SlowLoadBehindCaptivePortal(
       login_tab = popup_browser->tab_strip_model()->GetWebContentsAt(0);
       EXPECT_TRUE(CaptivePortalTabHelper::FromWebContents(login_tab)
                       ->is_captive_portal_window());
+      EXPECT_EQ(base::ASCIIToUTF16(kLoginSecureDnsDisabledTitle),
+                login_tab->GetTitle());
     } else {
       ASSERT_EQ(initial_browser_count, browser_list_->size());
       ASSERT_EQ(initial_tab_count + 1, tab_strip_model->count());
@@ -1453,6 +1464,8 @@ void CaptivePortalBrowserTest::FastErrorBehindCaptivePortal(
       login_tab = popup_browser->tab_strip_model()->GetWebContentsAt(0);
       EXPECT_TRUE(CaptivePortalTabHelper::FromWebContents(login_tab)
                       ->is_captive_portal_window());
+      EXPECT_EQ(base::ASCIIToUTF16(kLoginSecureDnsDisabledTitle),
+                login_tab->GetTitle());
     } else {
       ASSERT_EQ(initial_browser_count, browser_list_->size());
       ASSERT_EQ(initial_tab_count + 1, tab_strip_model->count());
@@ -1947,45 +1960,6 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, LoginFastTimeout) {
         1 /* expected_portal_checks */);
 }
 
-// Test that a navigation in a tab that is part of a captive portal windoow
-// has secure DNS disabled.
-IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,
-                       CaptivePortalWindowNavigationDisableSecureDns) {
-  content::WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  CaptivePortalTabHelper::FromWebContents(web_contents)
-      ->set_is_captive_portal_window();
-
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL url(embedded_test_server()->GetURL("/title1.html"));
-  url::Origin origin = url::Origin::Create(url);
-
-  // Disable the interceptor that was set up during construction of the test.
-  // This is necessary since only one interceptor is allowed.
-  url_loader_interceptor_.reset();
-
-  std::unique_ptr<content::URLLoaderInterceptor> url_loader_interceptor;
-  bool invoked_interceptor = false;
-  url_loader_interceptor = std::make_unique<content::URLLoaderInterceptor>(
-      base::BindLambdaForTesting(
-          [&](content::URLLoaderInterceptor::RequestParams* params) {
-            if (params->url_request.url.spec().find("title1.html") !=
-                std::string::npos) {
-              invoked_interceptor = true;
-              EXPECT_TRUE(params->url_request.trusted_params);
-              EXPECT_TRUE(
-                  params->url_request.trusted_params->disable_secure_dns);
-              EXPECT_EQ(
-                  net::NetworkIsolationKey(origin, origin),
-                  params->url_request.trusted_params->network_isolation_key);
-            }
-            return false;
-          }));
-
-  ui_test_utils::NavigateToURL(browser(), url);
-  EXPECT_TRUE(invoked_interceptor);
-}
-
 // A cert error triggers a captive portal check and results in opening a login
 // tab.
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,

chrome/browser/ssl/chrome_security_blocking_page_factory.cc

@@ -415,11 +415,11 @@ void ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents(
             ->test_url(),
         ui::PAGE_TRANSITION_TYPED);
     params.disposition = WindowOpenDisposition::NEW_POPUP;
+    params.is_captive_portal_popup = true;
     Navigate(&params);
     content::WebContents* new_contents = params.navigated_or_inserted_contents;
     CaptivePortalTabHelper* captive_portal_tab_helper =
         CaptivePortalTabHelper::FromWebContents(new_contents);
-    captive_portal_tab_helper->set_is_captive_portal_window();
     captive_portal_tab_helper->SetIsLoginTab();
     return;
   }

chrome/browser/ui/browser_navigator.cc

@@ -37,6 +37,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/url_constants.h"
+#include "components/captive_portal/core/buildflags.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_url_handler.h"
 #include "content/public/browser/navigation_entry.h"
@@ -59,6 +60,10 @@
 #include "ui/aura/window.h"
 #endif
 
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+#include "components/captive_portal/content/captive_portal_tab_helper.h"
+#endif
+
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
@@ -435,6 +440,14 @@ std::unique_ptr<content::WebContents> CreateTargetContents(
       ->SetExtensionAppById(params.extension_app_id);
 #endif
 
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  if (params.is_captive_portal_popup) {
+    DCHECK_EQ(WindowOpenDisposition::NEW_POPUP, params.disposition);
+    CaptivePortalTabHelper::FromWebContents(target_contents.get())
+        ->set_is_captive_portal_window();
+  }
+#endif
+
   return target_contents;
 }
 

chrome/browser/ui/browser_navigator_browsertest.cc

@@ -31,6 +31,7 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/captive_portal/core/buildflags.h"
 #include "components/omnibox/browser/omnibox_edit_model.h"
 #include "components/omnibox/browser/omnibox_view.h"
 #include "components/prefs/pref_service.h"
@@ -48,6 +49,10 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "services/network/public/cpp/resource_request_body.h"
 
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+#include "components/captive_portal/content/captive_portal_tab_helper.h"
+#endif
+
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/web_applications/extensions/web_app_extension_shortcut.h"
@@ -613,6 +618,28 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewPopupTrusted) {
   EXPECT_TRUE(params.browser->is_trusted_source());
 }
 
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+// This test verifies that navigating with WindowOpenDisposition = NEW_POPUP
+// and is_captive_portal_popup = true results in a new WebContents where
+// is_captive_portal_window() is true.
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
+                       Disposition_NewPopupCaptivePortal) {
+  NavigateParams params(MakeNavigateParams());
+  params.disposition = WindowOpenDisposition::NEW_POPUP;
+  params.is_captive_portal_popup = true;
+  params.window_bounds = gfx::Rect(0, 0, 200, 200);
+  // Wait for new popup to to load and gain focus.
+  ui_test_utils::NavigateToURL(&params);
+
+  // Navigate() should have opened a new popup window of TYPE_TRUSTED_POPUP.
+  EXPECT_NE(browser(), params.browser);
+  EXPECT_TRUE(params.browser->is_type_popup());
+  EXPECT_TRUE(CaptivePortalTabHelper::FromWebContents(
+                  params.navigated_or_inserted_contents)
+                  ->is_captive_portal_window());
+}
+#endif
+
 // This test verifies that navigating with WindowOpenDisposition = NEW_WINDOW
 // always opens a new window.
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewWindow) {

chrome/browser/ui/browser_navigator_params.h

@@ -199,6 +199,10 @@ struct NavigateParams {
   // NO_ACTION, |window_action| will be set to SHOW_WINDOW.
   WindowAction window_action = NO_ACTION;
 
+  // Whether the browser is being created for captive portal resolution. If
+  // true, |disposition| should be NEW_POPUP.
+  bool is_captive_portal_popup = false;
+
   // If false then the navigation was not initiated by a user gesture.
   bool user_gesture = true;
 

chrome/test/data/captive_portal/login_secure_dns_disabled.html

+<html>
+<head>
+<title>Fake Login Page Secure Dns Disabled</title>
+
+<script>
+
+function submitForm() {
+  document.getElementById('form').submit();
+}
+
+</script>
+
+</head>
+<body>
+<form id='form' action="login.html" method="post">
+<input type="submit" value="Login" />
+</form>
+</body>
+</html>