Ensure FrameNavigationEntry is fully updated.

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 1156 matching lines @@
 
   InsertOrReplaceEntry(std::move(new_entry), replace_entry);
 }
 
 void NavigationControllerImpl::RendererDidNavigateToExistingPage(
     RenderFrameHostImpl* rfh,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
   // We should only get here for main frame navigations.
   DCHECK(!rfh->GetParent());
 
+  // TODO(creis): Classify location.replace as NEW_PAGE instead of EXISTING_PAGE
+  // in https://crbug.com/596707.
+
   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);
   } 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
                                                  : PAGE_TYPE_NORMAL);
   entry->SetURL(params.url);
   entry->SetReferrer(params.referrer);
   if (entry->update_virtual_url_with_url())
     UpdateVirtualURLToURL(entry, params.url);
 
-  // Update the post parameters.
-  FrameNavigationEntry* frame_entry =
-      entry->GetFrameEntry(rfh->frame_tree_node());
-  frame_entry->set_method(params.method);
-  frame_entry->set_post_id(params.post_id);
-
-  // If the document sequence number has changed due to redirects or a
-  // location.replace, then the child FrameNavigationEntries that were there
-  // before no longer apply.  We can leave them around for in-page navigations.
-  if (frame_entry->document_sequence_number() !=
-      params.document_sequence_number)
-    entry->ClearChildren(rfh->frame_tree_node());
-
-  // Update the ISN and DSN in case this was a location.replace, which can cause
-  // them to change.
-  // TODO(creis): Classify location.replace as NEW_PAGE instead of EXISTING_PAGE
-  // in https://crbug.com/596707.
-  frame_entry->set_item_sequence_number(params.item_sequence_number);
-  frame_entry->set_document_sequence_number(params.document_sequence_number);
+  // Update the existing FrameNavigationEntry to ensure all of its members
+  // reflect the parameters coming from the renderer process.
+  entry->AddOrUpdateFrameEntry(
+      rfh->frame_tree_node(), params.item_sequence_number,
+      params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
+      params.url, params.referrer, params.page_state, params.method,
+      params.post_id);
 
   // The redirected to page should not inherit the favicon from the previous
   // page.
   if (ui::PageTransitionIsRedirect(params.transition))
     entry->GetFavicon() = FaviconStatus();
 
   // The site instance will normally be the same except during session restore,
   // when no site instance will be assigned.
   DCHECK(entry->site_instance() == nullptr ||
          entry->site_instance() == rfh->GetSiteInstance());
   entry->set_site_instance(
       static_cast<SiteInstanceImpl*>(rfh->GetSiteInstance()));

[Charlie Reis, 2016/08/10 17:03:08]

This line is redundant with AddOrUpdateFrameEntry.  We should keep the comment
and DCHECK and move them above the AddOrUpdateFrameEntry call, though.

[nasko, 2016/08/10 18:08:51]

Done.

 
   // The entry we found in the list might be pending if the user hit
   // back/forward/reload. This load should commit it (since it's already in the
   // list, we can just discard the pending pointer).  We should also discard the
   // pending entry if it corresponds to a different navigation, since that one
   // is now likely canceled.  If it is not canceled, we will treat it as a new
   // navigation when it arrives, which is also ok.
   //
   // Note that we need to use the "internal" version since we don't want to
   // actually change any other state, just kill the pointer.
@@ skipping 20 matching lines @@
   // a regular user-initiated navigation.
   DCHECK_EQ(pending_entry_->GetUniqueID(), params.nav_entry_id);
   existing_entry->set_unique_id(pending_entry_->GetUniqueID());
 
   // The URL may have changed due to redirects.
   existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR
                                                           : PAGE_TYPE_NORMAL);
   if (existing_entry->update_virtual_url_with_url())
     UpdateVirtualURLToURL(existing_entry, params.url);
   existing_entry->SetURL(params.url);
   existing_entry->SetReferrer(params.referrer);

[Charlie Reis, 2016/08/10 17:03:09]

Setting referrer is redundant with AddOrUpdateFrameEntry.

I suppose I'm ok with leaving the SetURL call, since that also clears the cached
display title.

[nasko, 2016/08/10 18:08:51]

Done.

 
-  // The page may have been requested with a different HTTP method.
-  FrameNavigationEntry* frame_entry =
-      existing_entry->GetFrameEntry(rfh->frame_tree_node());
-  frame_entry->set_method(params.method);
-  frame_entry->set_post_id(params.post_id);
+  // Update the existing FrameNavigationEntry to ensure all of its members
+  // reflect the parameters coming from the renderer process.
+  existing_entry->AddOrUpdateFrameEntry(
+      rfh->frame_tree_node(), params.item_sequence_number,
+      params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
+      params.url, params.referrer, params.page_state, params.method,
+      params.post_id);
 
   DiscardNonCommittedEntries();
 }
 
 void NavigationControllerImpl::RendererDidNavigateNewSubframe(
     RenderFrameHostImpl* rfh,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     bool is_in_page,
     bool replace_entry) {
   DCHECK(ui::PageTransitionCoreTypeIs(params.transition,
@@ skipping 814 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content