Index: content/browser/web_contents/navigation_controller_impl.cc |
diff --git a/content/browser/web_contents/navigation_controller_impl.cc b/content/browser/web_contents/navigation_controller_impl.cc |
index 8bd29ee87dbbf48fe621b6de74eee40f6cf1544a..82dc3af49955df55bef7782f8d10c8148cdb129c 100644 |
--- a/content/browser/web_contents/navigation_controller_impl.cc |
+++ b/content/browser/web_contents/navigation_controller_impl.cc |
@@ -1204,6 +1204,9 @@ void NavigationControllerImpl::CopyStateFrom( |
void NavigationControllerImpl::CopyStateFromAndPrune( |
NavigationController* temp) { |
+ // It is up to callers to check the invariants before calling this. |
+ CHECK(CanPruneAllButVisible()); |
+ |
NavigationControllerImpl* source = |
static_cast<NavigationControllerImpl*>(temp); |
// The SiteInstance and page_id of the last committed entry needs to be |
@@ -1213,32 +1216,23 @@ void NavigationControllerImpl::CopyStateFromAndPrune( |
NavigationEntryImpl* last_committed = |
NavigationEntryImpl::FromNavigationEntry(GetLastCommittedEntry()); |
scoped_refptr<SiteInstance> site_instance( |
- last_committed ? last_committed->site_instance() : NULL); |
- int32 minimum_page_id = last_committed ? last_committed->GetPageID() : -1; |
- int32 max_page_id = last_committed ? |
- web_contents_->GetMaxPageIDForSiteInstance(site_instance.get()) : -1; |
- |
- // This code is intended for use when the last entry is the active entry. |
- DCHECK( |
- (transient_entry_index_ != -1 && |
- transient_entry_index_ == GetEntryCount() - 1) || |
- (pending_entry_ && (pending_entry_index_ == -1 || |
- pending_entry_index_ == GetEntryCount() - 1)) || |
- (!pending_entry_ && last_committed_entry_index_ == GetEntryCount() - 1)); |
+ last_committed->site_instance()); |
+ int32 minimum_page_id = last_committed->GetPageID(); |
+ int32 max_page_id = |
+ web_contents_->GetMaxPageIDForSiteInstance(site_instance.get()); |
// Remove all the entries leaving the active entry. |
- PruneAllButActiveInternal(); |
+ PruneAllButVisibleInternal(); |
- // We now have zero or one entries. Ensure that adding the entries from |
- // source won't put us over the limit. |
- DCHECK(GetEntryCount() == 0 || GetEntryCount() == 1); |
- if (GetEntryCount() > 0) |
- source->PruneOldestEntryIfFull(); |
+ // We now have one entry, possibly with a new pending entry. Ensure that |
+ // adding the entries from source won't put us over the limit. |
+ DCHECK_EQ(1, GetEntryCount()); |
+ source->PruneOldestEntryIfFull(); |
// Insert the entries from source. Don't use source->GetCurrentEntryIndex as |
- // we don't want to copy over the transient entry. |
- int max_source_index = source->pending_entry_index_ != -1 ? |
- source->pending_entry_index_ : source->last_committed_entry_index_; |
+ // we don't want to copy over the transient entry. Ignore any pending entry, |
+ // since it has not committed in source. |
+ int max_source_index = source->last_committed_entry_index_; |
if (max_source_index == -1) |
max_source_index = source->GetEntryCount(); |
else |
@@ -1247,15 +1241,6 @@ void NavigationControllerImpl::CopyStateFromAndPrune( |
// Adjust indices such that the last entry and pending are at the end now. |
last_committed_entry_index_ = GetEntryCount() - 1; |
- if (pending_entry_index_ != -1) |
- pending_entry_index_ = GetEntryCount() - 1; |
- if (transient_entry_index_ != -1) { |
- // There's a transient entry. In this case we want the last committed to |
- // point to the previous entry. |
- transient_entry_index_ = GetEntryCount() - 1; |
- if (last_committed_entry_index_ != -1) |
- last_committed_entry_index_--; |
- } |
web_contents_->SetHistoryLengthAndPrune(site_instance.get(), |
max_source_index, |
@@ -1274,13 +1259,32 @@ void NavigationControllerImpl::CopyStateFromAndPrune( |
} |
} |
-void NavigationControllerImpl::PruneAllButActive() { |
- PruneAllButActiveInternal(); |
+bool NavigationControllerImpl::CanPruneAllButVisible() { |
+ // If there is no last committed entry, we cannot prune. Even if there is a |
+ // pending entry, it may not commit, leaving this WebContents blank, despite |
+ // possibly giving it new entries via CopyStateFromAndPrune. |
+ if (last_committed_entry_index_ == -1) |
+ return false; |
- // If there is an entry left, we need to update the session history length of |
- // the RenderView. |
- if (!GetActiveEntry()) |
- return; |
+ // We cannot prune if there is a pending entry at an existing entry index. |
+ // It may not commit, so we have to keep the last committed entry, and thus |
+ // there is no sensible place to keep the pending entry. It is ok to have |
+ // a new pending entry, which can optionally commit as a new navigation. |
+ if (pending_entry_index_ != -1) |
+ return false; |
+ |
+ // We should not prune if we are currently showing a transient entry. |
+ if (transient_entry_index_ != -1) |
+ return false; |
+ |
+ return true; |
+} |
+ |
+void NavigationControllerImpl::PruneAllButVisible() { |
+ PruneAllButVisibleInternal(); |
+ |
+ // We should still have a last committed entry. |
+ DCHECK_NE(-1, last_committed_entry_index_); |
NavigationEntryImpl* entry = |
NavigationEntryImpl::FromNavigationEntry(GetActiveEntry()); |
@@ -1293,43 +1297,16 @@ void NavigationControllerImpl::PruneAllButActive() { |
entry->site_instance(), 0, entry->GetPageID()); |
} |
-void NavigationControllerImpl::PruneAllButActiveInternal() { |
- if (transient_entry_index_ != -1) { |
- // There is a transient entry. Prune up to it. |
- DCHECK_EQ(GetEntryCount() - 1, transient_entry_index_); |
- entries_.erase(entries_.begin(), entries_.begin() + transient_entry_index_); |
- transient_entry_index_ = 0; |
- last_committed_entry_index_ = -1; |
- pending_entry_index_ = -1; |
- } else if (!pending_entry_) { |
- // There's no pending entry. Leave the last entry (if there is one). |
- if (!GetEntryCount()) |
- return; |
- |
- DCHECK_GE(last_committed_entry_index_, 0); |
- entries_.erase(entries_.begin(), |
- entries_.begin() + last_committed_entry_index_); |
- entries_.erase(entries_.begin() + 1, entries_.end()); |
- last_committed_entry_index_ = 0; |
- } else if (pending_entry_index_ != -1) { |
- entries_.erase(entries_.begin(), entries_.begin() + pending_entry_index_); |
- entries_.erase(entries_.begin() + 1, entries_.end()); |
- pending_entry_index_ = 0; |
- last_committed_entry_index_ = 0; |
- } else { |
- // There is a pending_entry, but it's not in entries_. |
- pending_entry_index_ = -1; |
- last_committed_entry_index_ = -1; |
- entries_.clear(); |
- } |
+void NavigationControllerImpl::PruneAllButVisibleInternal() { |
+ // It is up to callers to check the invariants before calling this. |
+ CHECK(CanPruneAllButVisible()); |
- if (web_contents_->GetInterstitialPage()) { |
- // Normally the interstitial page hides itself if the user doesn't proceeed. |
- // This would result in showing a NavigationEntry we just removed. Set this |
- // so the interstitial triggers a reload if the user doesn't proceed. |
- static_cast<InterstitialPageImpl*>(web_contents_->GetInterstitialPage())-> |
- set_reload_on_dont_proceed(true); |
- } |
+ // Erase all entries but the last committed entry. There may still be a |
+ // new pending entry after this. |
+ entries_.erase(entries_.begin(), |
+ entries_.begin() + last_committed_entry_index_); |
+ entries_.erase(entries_.begin() + 1, entries_.end()); |
+ last_committed_entry_index_ = 0; |
} |
void NavigationControllerImpl::ClearAllScreenshots() { |