Don't enable "replace current entry" if there is no entry to replace.
BUG=457149
TEST=included; as in bug
Review URL: https://codereview.chromium.org/914893002
Cr-Commit-Position: refs/heads/master@{#316031}
(cherry picked from commit b6cc1564475f04f6598ac6a228c33ff3530d4450)
(This cherry-pick omits the test as it does not merge cleanly.)
Review URL: https://codereview.chromium.org/920343004
Cr-Commit-Position: refs/branch-heads/2272@{#325}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index e501407..23cd703 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -727,7 +727,9 @@
static_cast<SiteInstanceImpl*>(params.source_site_instance.get()));
if (params.redirect_chain.size() > 0)
entry->SetRedirectChain(params.redirect_chain);
- if (params.should_replace_current_entry)
+ // Don't allow an entry replacement if there is no entry to replace.
+ // http://crbug.com/457149
+ if (params.should_replace_current_entry && entries_.size() > 0)
entry->set_should_replace_entry(true);
entry->set_should_clear_history_list(params.should_clear_history_list);
entry->SetIsOverridingUserAgent(override);
@@ -1553,6 +1555,7 @@
DiscardNonCommittedEntriesInternal();
int current_size = static_cast<int>(entries_.size());
+ DCHECK(current_size > 0 || !replace);
if (current_size > 0) {
// Prune any entries which are in front of the current entry.
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index 7820fca..e628d5f 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -1328,6 +1328,15 @@
// commit.
TEST_F(NavigationControllerTest, ResetEntryValuesAfterCommit) {
NavigationControllerImpl& controller = controller_impl();
+
+ // The value of "should replace entry" will be tested, but it's an error to
+ // specify it when there are no entries. Create a simple entry to be replaced.
+ const GURL url0("http://foo0");
+ controller.LoadURL(
+ url0, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
+ main_test_rfh()->SendNavigate(0, url0);
+
+ // Set up the pending entry.
const GURL url1("http://foo1");
controller.LoadURL(
url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
@@ -1340,8 +1349,6 @@
scoped_refptr<base::RefCountedBytes> post_data =
base::RefCountedBytes::TakeVector(&post_data_vector);
GlobalRequestID transfer_id(3, 4);
- std::vector<GURL> redirects;
- redirects.push_back(GURL("http://foo2"));
// Set non-persisted values on the pending entry.
NavigationEntryImpl* pending_entry =
@@ -1357,7 +1364,8 @@
EXPECT_TRUE(pending_entry->should_replace_entry());
EXPECT_TRUE(pending_entry->should_clear_history_list());
- main_test_rfh()->SendNavigate(0, url1);
+ // Fake a commit response.
+ main_test_rfh()->SendNavigate(1, url1);
// Certain values that are only used for pending entries get reset after
// commit.
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 8cfe73e..dcda817 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -2432,6 +2432,8 @@
// UpdateSessionHistory and update page_id_ even in this case, so that
// the current entry gets a state update and so that we don't send a
// state update to the wrong entry when we swap back in.
+ DCHECK(render_view_->history_list_length_ > 0 ||
+ !navigation_state->should_replace_current_entry());
if (GetLoadingUrl() != GURL(kSwappedOutURL) &&
!navigation_state->should_replace_current_entry()) {
// Advance our offset in session history, applying the length limit.
@@ -4077,7 +4079,9 @@
DocumentState* document_state = DocumentState::FromDataSource(ds);
NavigationState* navigation_state = document_state->navigation_state();
if (navigation_state->is_content_initiated()) {
- params.should_replace_current_entry = ds->replacesCurrentHistoryItem();
+ params.should_replace_current_entry =
+ ds->replacesCurrentHistoryItem() &&
+ render_view_->history_list_length_;
} else {
// This is necessary to preserve the should_replace_current_entry value on
// cross-process redirects, in the event it was set by a previous process.
diff --git a/content/test/data/navigation_controller/simple_page_1.html b/content/test/data/navigation_controller/simple_page_1.html
new file mode 100644
index 0000000..4b1d267
--- /dev/null
+++ b/content/test/data/navigation_controller/simple_page_1.html
@@ -0,0 +1,7 @@
+<html>
+<head>
+</head>
+<body>
+<p>Simple page 1.
+</body>
+</html>