[WebLayer] Bring in WebView's ContentRendererClient::HasErrorPage() impl

ContentRendererClient::HasErrorPage(http_status_error) determines whether the embedder has an error page for the given http status error. This method is used when a navigation results in an empty document (https://cs.chromium.org/chromium/src/content/renderer/render_frame_impl.cc?q=render_frame_impl&sq=package:chromium&g=0&l=4740). When it returns true, RenderFrameImpl will do the following: - Allow the embedder to display an error page if desired (via ContentRendererClient::PrepareErrorPageForHttpStatusError()). - Commit the navigation as a failed navigation. The default implementation returns false, which means that such navigations commit as successful rather than failed navigations. This CL does the following: - Copies //android_webview's implementation of HasErrorPage() into //weblayer to have WebLayer regard such navigations as failed navigations. - Adds a browsertest that navigates to an empty page returning a 404 and waits for the navigation to fail (this test fails without the production change in this CL, since prior to this change the navigation in question would actually complete in WebLayer). - Adds a page whose presence is necessary for navigations made in ssl_browsertest.cc to continue to complete following the production change in this CL (prior to this change the fact that the page was missing was not a problem, due to the very fact that WebLayer completed rather than failed navigations to empty pages with 404's). As detailed in crbug.com/1024212, it's not clear whether //android_webview *not* implementing ContentRendererClient::PrepareErrorPageForHttpStatusError() is intentional. The resolution of that discussion will impact whether/how we implement that method for WebLayer as well, but in the meantime, we are now matching WebView's behavior in this case. Bug: 1024212 Change-Id: I1e8012afd8ddf2f7cd3d1fcb6fcfd65421ddd31f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1914388Reviewed-by: Bo <boliu@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#715271}

[WebLayer] Bring in WebView's ContentRendererClient::HasErrorPage() impl
ContentRendererClient::HasErrorPage(http_status_error) determines whether the embedder has an error page for the given http status error. This method is used when a navigation results in an empty document (https://cs.chromium.org/chromium/src/content/renderer/render_frame_impl.cc?q=render_frame_impl&sq=package:chromium&g=0&l=4740). When it returns true, RenderFrameImpl will do the following: - Allow the embedder to display an error page if desired (via ContentRendererClient::PrepareErrorPageForHttpStatusError()). - Commit the navigation as a failed navigation. The default implementation returns false, which means that such navigations commit as successful rather than failed navigations. This CL does the following: - Copies //android_webview's implementation of HasErrorPage() into //weblayer to have WebLayer regard such navigations as failed navigations. - Adds a browsertest that navigates to an empty page returning a 404 and waits for the navigation to fail (this test fails without the production change in this CL, since prior to this change the navigation in question would actually complete in WebLayer). - Adds a page whose presence is necessary for navigations made in ssl_browsertest.cc to continue to complete following the production change in this CL (prior to this change the fact that the page was missing was not a problem, due to the very fact that WebLayer completed rather than failed navigations to empty pages with 404's). As detailed in crbug.com/1024212, it's not clear whether //android_webview *not* implementing ContentRendererClient::PrepareErrorPageForHttpStatusError() is intentional. The resolution of that discussion will impact whether/how we implement that method for WebLayer as well, but in the meantime, we are now matching WebView's behavior in this case. Bug: 1024212 Change-Id: I1e8012afd8ddf2f7cd3d1fcb6fcfd65421ddd31f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1914388Reviewed-by: Bo <boliu@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#715271}
826f07c2 · Colin Blundell · Commit Bot · 6f0694ef · 826f07c2 · 826f07c2
Commit 826f07c2 authored Nov 14, 2019 by Colin Blundell Committed by Commit Bot Nov 14, 2019
6 changed files
--- a/weblayer/browser/errorpage_browsertest.cc
+++ b/weblayer/browser/errorpage_browsertest.cc
@@ -33,4 +33,14 @@ IN_PROC_BROWSER_TEST_F(ErrorPageBrowserTest, NameNotResolved) {
 #endif
 }

+// Verifies that navigating to a URL that returns a 404 with an empty body
+// results in the navigation failing.
+IN_PROC_BROWSER_TEST_F(ErrorPageBrowserTest, 404WithEmptyBody) {
+  EXPECT_TRUE(embedded_test_server()->Start());
+
+  GURL error_page_url = embedded_test_server()->GetURL("/empty404.html");
+
+  NavigateAndWaitForFailure(error_page_url, shell());
+}
+
 }  // namespace weblayer
--- a/weblayer/renderer/content_renderer_client_impl.cc
+++ b/weblayer/renderer/content_renderer_client_impl.cc
@@ -85,6 +85,10 @@ void ContentRendererClientImpl::RenderFrameCreated(
  SSLErrorHelper::Create(render_frame);
 }

+bool ContentRendererClientImpl::HasErrorPage(int http_status_code) {
+  return http_status_code >= 400;
+}
+
 void ContentRendererClientImpl::PrepareErrorPage(
    content::RenderFrame* render_frame,
    const blink::WebURLError& error,

--- a/weblayer/renderer/content_renderer_client_impl.h
+++ b/weblayer/renderer/content_renderer_client_impl.h
@@ -17,6 +17,7 @@ class ContentRendererClientImpl : public content::ContentRendererClient {

  // content::ContentRendererClient:
  void RenderFrameCreated(content::RenderFrame* render_frame) override;
+  bool HasErrorPage(int http_status_code) override;
  void PrepareErrorPage(content::RenderFrame* render_frame,
                        const blink::WebURLError& error,
                        const std::string& http_method,

--- a/weblayer/test/data/empty404.html
+++ b/weblayer/test/data/empty404.html
--- a/weblayer/test/data/empty404.html.mock-http-headers
+++ b/weblayer/test/data/empty404.html.mock-http-headers
+HTTP/1.0 404 Not Found
+Content-type: text/html
--- a/weblayer/test/data/simple_page2.html
+++ b/weblayer/test/data/simple_page2.html
+<html>
+<head><title>Yet another simple page</title></head>
+<body>
+Basic html test.
+</body>
+</html>