Background: https://chromium-review.googlesource.com/c/chromium/src/+/609542/14/content/browser/loader/navigation_url_loader_network_service.cc#348
Right now both SSLInfo and a bool |should_ssl_errors_be_fatal| are surfaced for requests that fail due to certificate errors. As discussed in the above comment, it would make sense to put that boolean directly into SSLInfo so that we don't have to pass two arguments around so that consumers can decide how to treat certificate errors.
We'd try to get this in to M64 to unblock some PlzNavigate+network service work.
Carlos, could you take a look at this one? It's a good way to dip your toe into the committed interstitials water.
What we want to do is set a flag on net::SSLInfo called |is_fatal_cert_error|. We can set that flag in SSLClientSocketImpl::GetSSLInfo() and QuicChromiumClientSession::GetSSLInfo() when transport_security_state_->ShouldSSLErrorsBeFatal() is true.
Then we can get rid of the boolean fatal argument in a whole bunch of places: URLRequest::NotifySSLCertificateError, URLRequest::Delegate::OnSSLCertificateError and all its overrides, SSLManager::OnSSLCertificateError and the functions it calls, the SSLErrorHandler constructor, and NavigationURLLoaderDelegate::OnRequestFailed and its overrides/callsites.
Depending on how many places need to be updated, it might make sense to add the flag to SSLInfo but then update all the places that use it incrementally. If we take this approach, the priority should be to update the NavigationURLLoaderDelegate::OnRequestFailed overrides and callsites over the other uses of this flag, because that's the one that's most relevant to committed interstitials.
That CL has the changes for NavigationURLLoaderDelegate::OnRequestFailed (and its overrides and callsites), but there is a bit more of cleanup left that I will do on a separate CL, keeping the bug open until then.
Comment 1 by est...@chromium.org
, Nov 9 2017