Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Unified Diff: content/browser/frame_host/navigation_controller_impl.cc

Issue 1777903003: Ensure the NavigationHandle's nav entry ID is updated during transfers. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Review feedback Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 e24b58e84c6d09a62902022699c380921c95f04c..ca01a83d3aadc682cce76075e05cf23df366027b 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -830,6 +830,12 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
LoadEntry(std::move(entry));
}
+bool NavigationControllerImpl::PendingEntryMatchesHandle(
+ NavigationHandleImpl* handle) const {
+ return pending_entry_ &&
+ pending_entry_->GetUniqueID() == handle->pending_nav_entry_id();
+}
+
bool NavigationControllerImpl::RendererDidNavigate(
RenderFrameHostImpl* rfh,
const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
@@ -854,19 +860,25 @@ bool NavigationControllerImpl::RendererDidNavigate(
pending_entry_->restore_type() != NavigationEntryImpl::RESTORE_NONE)
pending_entry_->set_restore_type(NavigationEntryImpl::RESTORE_NONE);
- // If we are doing a cross-site reload, we need to replace the existing
- // navigation entry, not add another entry to the history. This has the side
- // effect of removing forward browsing history, if such existed. Or if we are
- // doing a cross-site redirect navigation, we will do a similar thing.
+ // If the pending entry matches this commit, check if it says to replace the
+ // current entry (which preserves forward history). This is used for things
+ // like cross-process location.replace and client redirects, as well as
+ // cross-process reloads (which can happen with embedder features like
+ // hosted apps).
//
// If this is an error load, we may have already removed the pending entry
// when we got the notice of the load failure. If so, look at the copy of the
- // pending parameters that were saved.
- if (params.url_is_unreachable && failed_pending_entry_id_ != 0) {
+ // pending parameters that were saved and see if they match the handle.
+ NavigationHandleImpl* handle = rfh->navigation_handle();
+ DCHECK(handle);
+ if (params.url_is_unreachable && failed_pending_entry_id_ != 0 &&
+ handle->pending_nav_entry_id() == failed_pending_entry_id_) {
details->did_replace_entry = failed_pending_entry_should_replace_;
+ } else if (PendingEntryMatchesHandle(handle) &&
+ pending_entry_->should_replace_entry()) {
+ details->did_replace_entry = pending_entry_->should_replace_entry();
} else {
- details->did_replace_entry = pending_entry_ &&
- pending_entry_->should_replace_entry();
+ details->did_replace_entry = false;
}
// Do navigation-type specific actions. These will make and commit an entry.
@@ -1109,8 +1121,8 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
// TODO(csharrison): Investigate whether we can remove some of the coarser
// checks.
NavigationHandleImpl* handle = rfh->navigation_handle();
- if (pending_entry_ && handle &&
- handle->pending_nav_entry_id() == pending_entry_->GetUniqueID() &&
+ DCHECK(handle);
+ if (PendingEntryMatchesHandle(handle) &&
pending_entry_index_ == -1 &&
nasko 2016/03/11 20:21:56 nit: Wouldn't this fit on the previous line? git c
Charlie Reis 2016/03/11 21:11:37 Done.
(!pending_entry_->site_instance() ||
pending_entry_->site_instance() == rfh->GetSiteInstance())) {

Powered by Google App Engine
This is Rietveld 408576698