New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 751949 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug
Team-Security-UX

Blocked on:
issue 762333
issue 762430

Blocking:
issue 448486



Sign in to add a comment

Interstitial refactor: expand NavigationRequest::OnRequestFailed() path to allow optional error page html

Project Member Reported by est...@chromium.org, Aug 3 2017

Issue description

Design doc pointer: https://docs.google.com/document/d/1rEBpw5V-Nn1UIi8CIFa5ZZvwlR08SkY3CogvWE2UMFs/edit#

We want certificate errors to commit as navigations like net errors. For net errors, an error code is passed down to the renderer starting in NavigationRequest::OnRequestFailed, and the renderer generates the HTML to commit as the error page UI. In the certificate error case, however, we need to generate the HTML in the browser process.

In  issue 751946 , we're adding a mechanism to get the error page HTML into NavigationRequest::OnRequestFailed(). Now we need to get it into the renderer and committed.

Add an error page data: URI argument to RenderFrameHostImpl::FailedNavigation, FrameMsg_FailedNavigation, RenderFrameImpl::OnFailedNavigation, and RenderFrameImpl::LoadNavigationErrorPage. (Or, maybe instead of adding a new argument, change the last argument from an error code into an ErrorParams struct that contains the error code and, optionally, error page HTML in the form of a data: URI.)

In RenderFrameImpl::LoadNavigationErrorPage, if error page HTML is provided as an argument, then use that, overriding the |error_html| retrieved from GetContentClient().
 
Blocking: 448486
Cc: lgar...@chromium.org
lgarron: I'm assigning all these interstitial bugs to me just so they have an owner, but this might be a good chunk for you to bite off to start.

Comment 2 by est...@chromium.org, Aug 18 2017

Cc: -lgar...@chromium.org est...@chromium.org
Owner: lgar...@chromium.org
Lucas: bumping some more bugs over to you in case you get blocked at any point.
Labels: Proj-CommittedInterstitials
Blocking: 762333
Blockedon: 762430
Blockedon: 762333
Blocking: -762333
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac

commit 75d2c9f905a4e994d6f41c5104a24f7a0e4745ac
Author: Lucas Garron <lgarron@chromium.org>
Date: Tue Sep 12 12:28:21 2017

Committed Interstitial error page HTML (1/2): attach net_error and error_page_content to ThrottleCheckResult.

Changes NavigationThrottle::ThrottleCheckResult into a struct that holds a net
error and an optional error page content (i.e. HTML + inline JS/CSS). The old
ThrottleCheckResult enum is changed to ThrottleAction and included in the
struct.

TBRing jochen@ for mechanical changes.

TBR=jochen@chromium.org

Bug:  751949 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I2b4836d1be24251704f25b05f0fd0c38bb74c341
Reviewed-on: https://chromium-review.googlesource.com/647656
Commit-Queue: Lucas Garron <lgarron@chromium.org>
Reviewed-by: Lucas Garron <lgarron@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501244}
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/chrome/browser/extensions/extension_navigation_throttle_unittest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/chrome/browser/subresource_filter/subresource_filter_test_harness.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/chrome/browser/supervised_user/supervised_user_google_auth_navigation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/chrome/browser/supervised_user/supervised_user_navigation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/subframe_navigation_filtering_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/subframe_navigation_filtering_throttle_unittest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/devtools/protocol/network_handler.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/form_submission_throttle_browsertest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/mixed_content_navigation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/mixed_content_navigation_throttle.h
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/navigation_handle_impl_browsertest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/navigation_handle_impl_unittest.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/browser/loader/navigation_resource_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/public/browser/navigation_throttle.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/public/browser/navigation_throttle.h
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/75d2c9f905a4e994d6f41c5104a24f7a0e4745ac/extensions/browser/extension_navigation_throttle.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a15d3fccbc47ad0f54c5f24c135e5b1918007499

commit a15d3fccbc47ad0f54c5f24c135e5b1918007499
Author: Lucas Garron <lgarron@chromium.org>
Date: Thu Nov 02 02:48:18 2017

Committed Interstitials: expand NavigationRequest::OnRequestFailed() path to allow optional error page html (2/2)

Plumb failed request error page data URL from NavigationThrottle to RenderFrameImpl.
As part of this, modify DidFailProvisionalLoad in WebFrameClient and all its subclasses
to take an optional error_page_content string.

Bug:  751949 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I43d7329a8dc6e479fb9c281f2efbe832f5341eb0
Reviewed-on: https://chromium-review.googlesource.com/625337
Commit-Queue: Lucas Garron <lgarron@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513385}
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/browser/frame_host/navigation_handle_impl_browsertest.cc
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/common/frame_messages.h
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/public/browser/navigation_throttle.h
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/a15d3fccbc47ad0f54c5f24c135e5b1918007499/content/renderer/render_frame_impl.h

Labels: -M-63 M-64
Status: Fixed (was: Assigned)

Sign in to add a comment