Interstitial refactor: store SSL state on navigation entry when requests fail |
||||||
Issue descriptionCurrently, the SSLStatus on a NavigationHandle only gets populated in WillProcessResponse(). But we are converting certificate interstitials into something more similar to net error pages, meaning that requests with certificate errors will get cancelled to commit an error page. When that error page commits, we need the committed navigation entry to have the correct SSL state... which it won't have because the state won't have been set on the NavigationHandle. Issue 751941 introduced a NavigationHandle method through which certificate errors pass. We need to construct an SSLStatus for the NavigationHandle in that method, from the SSLInfo, so that when a navigation entry commits for the certificate error, there will be accurate SSL information on the NavigationHandle for the committed NavigationEntry to use. (This change will obviate SSLBlockingPage::OverrideEntry and friends.)
,
Aug 9 2017
Update, per revised design: the SSLStatus should get updated in NavigationRequest::OnRequestFailed (there won't be a special hook for certificate errors).
,
Aug 9 2017
,
Sep 5 2017
,
Sep 8 2017
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c1edb5abd7a5f5a57675beb79f2403f05658f068 commit c1edb5abd7a5f5a57675beb79f2403f05658f068 Author: Lucas Garron <lgarron@chromium.org> Date: Wed Nov 08 03:31:13 2017 Set SSL status in NavigationHandleImpl::WillFailRequest. Bug: 751951 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation Change-Id: Id08c5f559720e4ba533c0ae3665f69a805129450 Reviewed-on: https://chromium-review.googlesource.com/752002 Commit-Queue: Lucas Garron <lgarron@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Cr-Commit-Position: refs/heads/master@{#514732} [modify] https://crrev.com/c1edb5abd7a5f5a57675beb79f2403f05658f068/chrome/browser/ssl/ssl_browser_tests.cc [modify] https://crrev.com/c1edb5abd7a5f5a57675beb79f2403f05658f068/content/browser/frame_host/navigation_handle_impl.cc [modify] https://crrev.com/c1edb5abd7a5f5a57675beb79f2403f05658f068/content/browser/frame_host/navigation_handle_impl_unittest.cc
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32e219d348b9fe922303132e7c146ec1d6de65ea commit 32e219d348b9fe922303132e7c146ec1d6de65ea Author: Matt Menke <mmenke@chromium.org> Date: Wed Nov 08 14:09:00 2017 Disable some browser_tests on the network mojo bot. In particular, disable recently added DicePrepareMigrationBrowserTests, and SSLUITestCommittedInterstitials.ErrorPage (Which likely regressed in https://chromium-review.googlesource.com/c/chromium/src/+/752002). TBR=jam@chromium.org NOTRY=true Bug: 771908 , 751951 Change-Id: Id02fbf7d3e8ca10859de521ca2b7ecffdc4cf45e Reviewed-on: https://chromium-review.googlesource.com/758598 Commit-Queue: Matt Menke <mmenke@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#514813} [modify] https://crrev.com/32e219d348b9fe922303132e7c146ec1d6de65ea/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Nov 10 2017
,
Nov 13 2017
Can this bug be closed?
,
Nov 13 2017
Yes, it can! :-) TEST= 1) Enable chrome://flags/#committed-interstitials and restart 2) Visit https://expired.badssl.com/ 3) Check that the omnibox shows a red triabgle with an exclamation mark and the words "Not Secure" |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by est...@chromium.org
, Aug 3 2017