Don't use SSLStatus from FrameHostMsg_DidCommitProvisionalLoad and instead cache it on the browser …

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index ffcd8eedd6ae59f0b8e228a1ed701b16c9ab9e2f..ecee3ff57ab4d532e54ac2419be2f73e65e05841 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -1127,7 +1127,20 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
   // type must always be normal or error.
   new_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR
                                                      : PAGE_TYPE_NORMAL);
-  new_entry->SetURL(params.url);
+  if (params.url != new_entry->GetURL()) {

[estark, 2016/08/15 08:14:08]

Is this check just an optimization? I'm concerned that two different navigations
to the same URL could have different SSL properties.

[jam, 2016/08/15 19:23:58]

It wasn't; looking at it again it was poorly written. I've reworked it a bit to
make it clearer.

When the SSLState is saved on a pending NavigationEntry from ResourceLoader, we
want to use it. That's what the url check was effectively doing.
You mean that the server would return different certificates for the same url?

[mmenke, 2016/08/15 22:48:31]

It could be two different servers with the same hostname.  Server could have
updated its cert.  Or one could be a captive portal that suddenly appeared
(Along with a cert error click through).

One response could have been served out of cache (With a cached cert), while the
next could require us to go to the server (Again, may have to click through cert
error due to captive portal for the second URL, too), etc.

+    new_entry->SetURL(params.url);
+    // New URL means that we can't use the old SSLStatus.
+    new_entry->GetSSL() = SSLStatus();
+  }
+
+  if (rfh->frame_tree_node()->IsMainFrame()) {
+    // In case of redirects (same process or across different ones), the
+    // SSLStatus was cached on the pending RFH so use it now.
+    SSLStatus ssl_status;
+    if (rfh->GetSSLStatusForPendingNavigate(params.url, &ssl_status))

[estark, 2016/08/15 08:14:08]

Just checking -- we're guaranteed that the returned SSLStatus corresponds to
*this* navigation? Is there any way we could call
SetSSLStatusForPendingNavigate() but then not Get() it until some later
navigation (different than the one that we called Set() for)?

[jam, 2016/08/15 19:23:58]

I don't think this is guaranteed. i.e. in the
SitePerProcessIgnoreCertErrorsBrowserTest.CrossSiteRedirectCertificateStore
case, the navigation IDs aren't the same for the pending naventry and the new
process's commit message, but we do want to use this cached SSLStatus on the RFH

+      new_entry->GetSSL() = ssl_status;
+  }
+
   if (update_virtual_url)
     UpdateVirtualURLToURL(new_entry.get(), params.url);
   new_entry->SetReferrer(params.referrer);