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

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

Issue 1048963002: Fix incorrect creation of duplicate navigation entries for repeated page load failures. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@committype3
Patch Set: with more comment Created 5 years, 8 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 8ec8dc91047a1ab77effedcb448046b9479f3d1e..2ebdd261005e2f91e2f6cecadceeec6a5102ef6b 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -226,6 +226,8 @@ NavigationControllerImpl::NavigationControllerImpl(
BrowserContext* browser_context)
: browser_context_(browser_context),
pending_entry_(NULL),
+ failed_pending_entry_id_(0),
+ failed_pending_entry_should_replace_(false),
last_committed_entry_index_(-1),
pending_entry_index_(-1),
transient_entry_index_(-1),
@@ -796,11 +798,18 @@ bool NavigationControllerImpl::RendererDidNavigate(
// 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.
- details->did_replace_entry =
- pending_entry_ && pending_entry_->should_replace_entry();
+ // 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 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) {
+ details->did_replace_entry = failed_pending_entry_should_replace_;
+ } else {
+ details->did_replace_entry = pending_entry_ &&
+ pending_entry_->should_replace_entry();
+ }
// Do navigation-type specific actions. These will make and commit an entry.
details->type = ClassifyNavigation(rfh, params);
@@ -1758,17 +1767,25 @@ void NavigationControllerImpl::FinishRestore(int selected_index,
}
void NavigationControllerImpl::DiscardNonCommittedEntriesInternal() {
- DiscardPendingEntry();
+ DiscardPendingEntry(false);
DiscardTransientEntry();
}
-void NavigationControllerImpl::DiscardPendingEntry() {
+void NavigationControllerImpl::DiscardPendingEntry(bool was_failure) {
// It is not safe to call DiscardPendingEntry while NavigateToEntry is in
// progress, since this will cause a use-after-free. (We only allow this
// when the tab is being destroyed for shutdown, since it won't return to
// NavigateToEntry in that case.) http://crbug.com/347742.
CHECK(!in_navigate_to_pending_entry_ || delegate_->IsBeingDestroyed());
+ if (was_failure && pending_entry_) {
+ failed_pending_entry_id_ = pending_entry_->GetUniqueID();
+ failed_pending_entry_should_replace_ =
+ pending_entry_->should_replace_entry();
+ } else {
+ failed_pending_entry_id_ = 0;
+ }
+
if (pending_entry_index_ == -1)
delete pending_entry_;
pending_entry_ = NULL;

Powered by Google App Engine
This is Rietveld 408576698