Fix incorrect SSL state being shown for client redirects.

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 e49372f1eaa7a76db3cd551e03ea13a037c56f2c..22b0ed27ac14a3602fd8c5626b7ac2a537221f7d 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -825,7 +825,7 @@ bool NavigationControllerImpl::RendererDidNavigate(
       break;
     case NAVIGATION_TYPE_EXISTING_PAGE:
       details->did_replace_entry = details->is_in_page;
-      RendererDidNavigateToExistingPage(rfh, params);
+      RendererDidNavigateToExistingPage(rfh, params, details->is_in_page);
       break;
     case NAVIGATION_TYPE_SAME_PAGE:
       RendererDidNavigateToSamePage(rfh, params);
@@ -1170,7 +1170,8 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
 
 void NavigationControllerImpl::RendererDidNavigateToExistingPage(
     RenderFrameHostImpl* rfh,
-    const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
+    bool is_in_page) {
   // We should only get here for main frame navigations.
   DCHECK(!rfh->GetParent());
 
@@ -1178,6 +1179,7 @@ void NavigationControllerImpl::RendererDidNavigateToExistingPage(
   // in https://crbug.com/596707.
 
   NavigationEntryImpl* entry;
+  NavigationHandleImpl* handle = rfh->navigation_handle();
   if (params.intended_as_new_entry) {
     // This was intended as a new entry but the pending entry was lost in the
     // meanwhile and no new page was created. We are stuck at the last committed
@@ -1189,13 +1191,20 @@ void NavigationControllerImpl::RendererDidNavigateToExistingPage(
 
     // Needed for the restore case, where the serialized NavigationEntry doesn't
     // have the SSL state.
-    NavigationHandleImpl* handle = rfh->navigation_handle();
     entry->GetSSL() = handle->ssl_status();
   } else {
     // This is renderer-initiated. The only kinds of renderer-initated
     // navigations that are EXISTING_PAGE are reloads and location.replace,
     // which land us at the last committed entry.
     entry = GetLastCommittedEntry();
+
+    // Until https://crbug.com/596707 is fixed (see comment at top of method),
+    // we will get here for location.replace for both same origin and different
+    // origin. For the former, there won't be a network request that gives the
+    // SSLStatus so we don't want to change this. For the latter, there will be
+    // a request and so we need to update the SSLStatus.

[Charlie Reis, 2016/09/06 23:43:14]

This comment seems stale-- it's referring to the old origin check.  Can you
update it to explain why the SSLStatus shouldn't be updated for in page
navigations?  (I'm not sure why that failed in the MixedContentWithBrokenSHA1
and MixedContent tests, since those don't appear to have in-page navigations.)

[jam, 2016/09/07 00:03:56]

Done
As an example, they loaded
chrome/test/data/ssl/page_runs_insecure_content_in_iframe.html which includes
iframe_with_insecure_content.html and that runs randomize_hash.js:

document.body.innerHTML = "I'm content from an HTTP script." 
location.hash = Math.random();



The location.hash call ended up going to line 1200 above. There was no actual
navigation (i.e. is_in_page), and so blindly resetting  entry->GetSSL() =
handle->ssl_status(); meant that we lost the ssl state (since the
navigationhandle had an empty SSLStatus)

+    if (!is_in_page)
+      entry->GetSSL() = handle->ssl_status();
   }
   DCHECK(entry);