Fix the page's SSL status not being set on restore.

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 1168 matching lines @@
 
   NavigationEntryImpl* entry;
   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
     // entry.
     entry = GetLastCommittedEntry();
   } else if (params.nav_entry_id) {
     // This is a browser-initiated navigation (back/forward/reload).
     entry = GetEntryWithUniqueID(params.nav_entry_id);
+
+    // 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();

[nasko, 2016/09/01 01:35:29]

I think we might need this to happen also in RendererDidNavigateToSamePage. It
is called when the user presses enter in the omnibox. However, if the server
does a redirect to different URL, we still will go through there. This happens
in cases where the user session has expired and the server redirects to the
login page.

[jam, 2016/09/01 02:58:56]

I tried pressing enter in the omnibox and it goes through
RendererDidNavigateToSamePage, however the SSL certificate is correct in that
case (since the previous one is used). Do you mean something else?

[nasko, 2016/09/01 06:27:55]

They key part is for the server to do a redirect. Similar test exists in
content/
https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_co...,
though it doesn't test SSL state, maybe we can add those checks there?

In general, is there a good reason not to do this at RendererDidNavigate
generically? Maybe around here -
https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_co....

[jam, 2016/09/01 15:44:09]

Got it, thanks. Ok I added a test and a fix.
Yep I agree that would be the easiest. However the previous change that added
this code already couldn't do it in all the codepaths of
RendererDidNavigateToNewPage. Then this fix couldn't do it in all codepaths of
RendererDidNavigateToExistingPage either (see patchset 1 which initially tried
this, but there were test failures). 

   } 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();
   }
   DCHECK(entry);
 
   // The URL may have changed due to redirects.
   entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR
@@ skipping 902 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content