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..83452ce0619e946335019b84c83e1e9d2bad3bb6 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -1178,6 +1178,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 +1190,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.
+    if (params.url.GetOrigin() != entry->GetURL().GetOrigin())

[felt, 2016/09/06 20:09:55]

Oof. This fixes the obvious common case but I am concerned it does not really
fix the whole issue (corner cases).

The tricky thing here is that navigating within the same origin could
theoretically still change the state, right? For example, if the new page has
mixed content or if there are malicious subresources on the new page.

[jam, 2016/09/06 20:20:37]

If these events occur (mixed content or malicious subresources), the relevant
notifications will still be fired from the renderer and the SSLStatus will be
updated. They'll fire after the did commit provisional load IPC which is what
calls this method.

[jam, 2016/09/06 20:28:54]

ok thinking some more about this, one issue which I think you bring up is:
-user visits https://foo.com/mixedcontent
-the SSLStatus is later updated because of mixed content from that page
-page uses window.location.replace to go to https://foo.com/goodpage
-we don't create a new SSLStatus and use the old bad one

so it seems we would end up in a situation where a bad icon is used when it
should be good. the other way around shouldn't be possible (because of my
earlier reply). i'll look some more into differentiating this case.

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