Don't clear the forward history when replacing entries.

content/browser/frame_host/navigation_controller_impl_browsertest.cc

Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index ba4a49d1d4de3f05005e651f220a64d46afd8550..8a6c268390b96616340672f04479e4680073abc6 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -186,9 +186,19 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   EXPECT_EQ(1, controller.GetEntryCount());
   EXPECT_EQ(1, RendererHistoryLength(shell()));
 
-  EXPECT_TRUE(NavigateToURLAndReplace(shell(), GURL("data:text/html,page2")));
-  EXPECT_EQ(1, controller.GetEntryCount());
-  EXPECT_EQ(1, RendererHistoryLength(shell()));
+  EXPECT_TRUE(NavigateToURL(shell(), GURL("data:text/html,page2")));
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(2, RendererHistoryLength(shell()));
+
+  // Go back and then replace the first entry.
+  controller.GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_TRUE(controller.CanGoForward());
+
+  EXPECT_TRUE(NavigateToURLAndReplace(shell(), GURL("data:text/html,page1b")));
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(2, RendererHistoryLength(shell()));
+  EXPECT_TRUE(controller.CanGoForward());

[Avi (use Gerrit), 2015/07/16 20:00:16]

Are you sure that this change keeps testing the same code in RenderView that the
old code did? More precisely, this is testing the "&&
!navigation_state->start_params().should_replace_current_entry" part of
RenderFrameImpl::didCommitProvisionalLoad. Can you delete that clause and make
sure that this test fails?

[Charlie Reis, 2015/07/16 20:17:43]

Good point.  The fact that I only had one additional entry made the expectation
(2) the same as the buggy answer from the renderer (2).  Sure enough, the test
passed when removing that clause.

I updated it in two ways: I preserved your original test (causing it to fail
without the RenderFrameImpl clause), and I changed my new case to have 3 entries
in history (which independently fails without the RenderFrameImpl clause).

 
   // Note that there's no way to access the renderer's notion of the history
   // offset via JavaScript. Checking just the history length, though, is enough;