Show subframe error pages instead of interstitials for portals

Portals are embedded and cannot be interacted with by the user, so it's
not useful to present a security interstitial to the user. We now use
the same error page that would be used for subframes.

Bug: 963522
Change-Id: I750053ec4775901266555b105a50c684e8739f7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111065Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Edward Jung (EMEA) <edwardjung@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756326}

chrome/browser/portal/portal_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/devtools/devtools_window_testing.h"
+#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
 #include "chrome/browser/pdf/pdf_extension_test_util.h"
 #include "chrome/browser/task_manager/providers/task.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
@@ -21,8 +22,12 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/page_type.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -263,3 +268,55 @@ IN_PROC_BROWSER_TEST_F(PortalBrowserTest, PdfViewerLoadsInPortal) {
 
   EXPECT_TRUE(pdf_extension_test_util::EnsurePDFHasLoaded(portal_contents));
 }
+
+// Test that we do not show main frame interstitials in portal contents. We
+// should treat portals like subframes in terms of how to display the error to
+// the user.
+IN_PROC_BROWSER_TEST_F(PortalBrowserTest, ShowSubFrameErrorPage) {
+  net::EmbeddedTestServer bad_https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  bad_https_server.AddDefaultHandlers(GetChromeTestDataDir());
+  bad_https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_EXPIRED);
+  ASSERT_TRUE(bad_https_server.Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL url(embedded_test_server()->GetURL("/title1.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+  WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+
+  GURL bad_cert_url(bad_https_server.GetURL("/title1.html"));
+  content::TestNavigationObserver portal_navigation_observer(nullptr, 1);
+  portal_navigation_observer.StartWatchingNewWebContents();
+  ASSERT_EQ(true,
+            content::EvalJs(
+                contents, content::JsReplace(
+                              "new Promise((resolve) => {"
+                              "  let portal = document.createElement('portal');"
+                              "  portal.src = $1;"
+                              "  portal.onload = () => { resolve(true); };"
+                              "  document.body.appendChild(portal);"
+                              "});",
+                              bad_cert_url)));
+  portal_navigation_observer.StopWatchingNewWebContents();
+  portal_navigation_observer.Wait();
+  EXPECT_FALSE(portal_navigation_observer.last_navigation_succeeded());
+  EXPECT_EQ(net::ERR_CERT_DATE_INVALID,
+            portal_navigation_observer.last_net_error_code());
+
+  std::vector<WebContents*> inner_web_contents =
+      contents->GetInnerWebContents();
+  ASSERT_EQ(1u, inner_web_contents.size());
+  WebContents* portal_contents = inner_web_contents[0];
+
+  content::NavigationController& controller = portal_contents->GetController();
+  content::NavigationEntry* entry = controller.GetLastCommittedEntry();
+  ASSERT_TRUE(entry);
+  EXPECT_EQ(content::PAGE_TYPE_ERROR, entry->GetPageType());
+
+  // Ensure that this is the net error page and not an interstitial.
+  ASSERT_FALSE(
+      chrome_browser_interstitials::IsShowingInterstitial(portal_contents));
+  // Also ensure that the error page is using its subframe layout.
+  ASSERT_EQ(true, content::EvalJs(
+                      portal_contents,
+                      "document.documentElement.hasAttribute('subframe');"));
+}

components/neterror/resources/neterror.js

@@ -72,8 +72,9 @@ if (window.errorPageController) {
 
 // Subframes use a different layout but the same html file.  This is to make it
 // easier to support platforms that load the error page via different
-// mechanisms (Currently just iOS).
-if (window.top.location !== window.location) {
+// mechanisms (Currently just iOS). We also use the subframe style for portals
+// as they are embedded like subframes and can't be interacted with by the user.
+if (window.top.location !== window.location || window.portalHost) {
   document.documentElement.setAttribute('subframe', '');
 }
 

components/security_interstitials/content/ssl_error_navigation_throttle.cc

@@ -41,7 +41,7 @@ SSLErrorNavigationThrottle::WillFailRequest() {
 
   // Do not set special error page HTML for subframes; those are handled as
   // normal network errors.
-  if (!handle->IsInMainFrame()) {
+  if (!handle->IsInMainFrame() || handle->GetWebContents()->IsPortal()) {
     return content::NavigationThrottle::PROCEED;
   }
 
@@ -67,7 +67,7 @@ SSLErrorNavigationThrottle::WillProcessResponse() {
 
   // Do not set special error page HTML for subframes; those are handled as
   // normal network errors.
-  if (!handle->IsInMainFrame()) {
+  if (!handle->IsInMainFrame() || handle->GetWebContents()->IsPortal()) {
     return content::NavigationThrottle::PROCEED;
   }
 

content/browser/renderer_host/render_view_host_delegate.cc

@@ -69,7 +69,7 @@ RenderFrameHostImpl* RenderViewHostDelegate::GetPendingMainFrame() {
   return nullptr;
 }
 
-bool RenderViewHostDelegate::IsPortal() const {
+bool RenderViewHostDelegate::IsPortal() {
   return false;
 }
 

content/browser/renderer_host/render_view_host_delegate.h

@@ -180,7 +180,7 @@ class CONTENT_EXPORT RenderViewHostDelegate {
   virtual void DidFirstVisuallyNonEmptyPaint(RenderViewHostImpl* source) {}
 
   // Returns true if the render view is rendering a portal.
-  virtual bool IsPortal() const;
+  virtual bool IsPortal();
 
   // Called when the theme color for the underlying document as specified
   // by theme-color meta tag has changed.

content/browser/renderer_host/render_widget_host_delegate.cc

@@ -159,7 +159,7 @@ RenderWidgetHostDelegate::GetFocusedFrameFromFocusedDelegate() {
   return nullptr;
 }
 
-bool RenderWidgetHostDelegate::IsPortal() const {
+bool RenderWidgetHostDelegate::IsPortal() {
   return false;
 }
 

content/browser/renderer_host/render_widget_host_delegate.h

@@ -329,7 +329,7 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
       viz::VerticalScrollDirection scroll_direction) {}
 
   // Returns true if the delegate is a portal.
-  virtual bool IsPortal() const;
+  virtual bool IsPortal();
 
   // Notify the delegate that the screen orientation has been changed.
   virtual void DidChangeScreenOrientation() {}

content/browser/web_contents/web_contents_impl.cc

@@ -5240,7 +5240,7 @@ void WebContentsImpl::DidFirstVisuallyNonEmptyPaint(
   }
 }
 
-bool WebContentsImpl::IsPortal() const {
+bool WebContentsImpl::IsPortal() {
   return portal();
 }
 

content/browser/web_contents/web_contents_impl.h

@@ -393,6 +393,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
                               RenderFrameHost* render_frame_host,
                               bool is_full_page) override;
   bool IsInnerWebContentsForGuest() override;
+  bool IsPortal() override;
   RenderFrameHostImpl* GetOuterWebContentsFrame() override;
   WebContentsImpl* GetOuterWebContents() override;
   WebContentsImpl* GetOutermostWebContents() override;
@@ -784,7 +785,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   bool IsSpatialNavigationDisabled() const override;
   RenderFrameHostImpl* GetPendingMainFrame() override;
   void DidFirstVisuallyNonEmptyPaint(RenderViewHostImpl* source) override;
-  bool IsPortal() const override;
   void OnThemeColorChanged(RenderViewHostImpl* source) override;
 
   // NavigatorDelegate ---------------------------------------------------------

content/public/browser/web_contents.h

@@ -612,6 +612,11 @@ class WebContents : public PageNavigator,
   // GetOuterWebContents instead.
   virtual bool IsInnerWebContentsForGuest() = 0;
 
+  // Returns whether this WebContents is a portal. This returns true even when
+  // this WebContents is not attached to its portal host's WebContents tree.
+  // This value may change over time due to portal activation and adoption.
+  virtual bool IsPortal() = 0;
+
   // Returns the outer WebContents frame, the same frame that this WebContents
   // was attached in AttachToOuterWebContentsFrame().
   virtual RenderFrameHost* GetOuterWebContentsFrame() = 0;