Avoid stale navigation requests without excessive page id knowledge in the renderer process.

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 20c00ee3edf6db7935e6a6736402a33b134708f7..e27ae3aed49a3f02e928e96bb82b14d505013fac 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -3,9 +3,11 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/strings/stringprintf.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
@@ -22,7 +24,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, LoadDataWithBaseURL) {
 
   const NavigationController& controller =
       shell()->web_contents()->GetController();
-  // load data. Blocks until it is done.
+  // Load data. Blocks until it is done.
   content::LoadDataWithBaseURL(shell(), history_url, data, base_url);
 
   // We should use history_url instead of the base_url as the original url of
@@ -30,5 +32,45 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, LoadDataWithBaseURL) {
   // paths in the data, or enforcing same origin policy.
   EXPECT_EQ(controller.GetVisibleEntry()->GetOriginalRequestURL(), history_url);
 }
+
+// The renderer uses the position in the history list as a clue to whether a
+// navigation is stale. In the case where the entry limit is reached and the
+// history list is pruned, make sure that there is no mismatch that would cause
+// it to start incorrectly rejecting navigations as stale. See
+// http://crbug.com/89798.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       DontIgnoreBackAfterNavEntryLimit) {
+  NavigationController& controller =
+      shell()->web_contents()->GetController();
+
+  const int kMaxEntryCount =
+      static_cast<int>(NavigationControllerImpl::max_entry_count());
+
+  // Load up to the max count, all entries should be there.
+  for (int url_index = 0; url_index < kMaxEntryCount; ++url_index) {
+    GURL url(base::StringPrintf("http://127.0.0.1/%d", url_index));

[Charlie Reis, 2014/12/04 23:13:58]

Where is the test server for this?  (I'm surprised it returns true from
NavigateToURL.)

[Avi (use Gerrit), 2014/12/05 22:16:39]

Changing to a data URL. (It doesn't return true on Android.)

+    EXPECT_TRUE(NavigateToURL(shell(), url));
+  }
+
+  EXPECT_EQ(controller.GetEntryCount(), kMaxEntryCount);
+
+  // Navigate twice more more.
+  for (int url_index = kMaxEntryCount;
+       url_index < kMaxEntryCount + 2; ++url_index) {
+    GURL url(base::StringPrintf("http://127.0.0.1/%d", url_index));
+    EXPECT_TRUE(NavigateToURL(shell(), url));
+  }
+
+  // We expect http://www.a.com/0 and /1 to be gone.
+  EXPECT_EQ(controller.GetEntryCount(), kMaxEntryCount);
+  EXPECT_EQ(controller.GetEntryAtIndex(0)->GetURL(),

[Charlie Reis, 2014/12/04 23:13:58]

Throughout this test, let's reverse the arguments.  It should be
EXPECT_EQ(expected, actual), which affects what's printed when it fails.

[Avi (use Gerrit), 2014/12/05 22:16:39]

Done.

+            GURL("http://127.0.0.1/2"));
+
+  // Now try to go back. This should not hang.
+  ASSERT_TRUE(controller.CanGoBack());
+  controller.GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));

[Charlie Reis, 2014/12/04 23:13:58]

Let's verify that we ended up on the expected URL, too.  Hanging is just one way
it could go wrong.

[Avi (use Gerrit), 2014/12/05 22:16:39]

Done.

+}
+
 }  // namespace content