Replace PruneAllButActive with PruneAllButVisible.

content/browser/web_contents/navigation_controller_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
@@ skipping 2767 matching lines @@
   SiteInstance* instance3 =
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(2));
   EXPECT_NE(instance3, instance1);
 
   // The max page ID map should be copied over and updated with the max page ID
   // from the current tab.
   EXPECT_EQ(1, other_contents->GetMaxPageIDForSiteInstance(instance1));
   EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance3));
 }
 
-// Test CopyStateFromAndPrune with 2 urls, the first selected and nothing in
+// Test CopyStateFromAndPrune with 2 urls, the first selected and 1 entry in
 // the target.
 TEST_F(NavigationControllerTest, CopyStateFromAndPrune2) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo1");
   const GURL url2("http://foo2");
   const GURL url3("http://foo3");
 
   NavigateAndCommit(url1);
   NavigateAndCommit(url2);
   controller.GoBack();
-
-  scoped_ptr<TestWebContents> other_contents(
-      static_cast<TestWebContents*>(CreateTestWebContents()));
-  NavigationControllerImpl& other_controller = other_contents->GetController();
-  other_contents->ExpectSetHistoryLengthAndPrune(NULL, 1, -1);
-  other_controller.CopyStateFromAndPrune(&controller);
-
-  // other_controller should now contain the 1 url: url1.
-
-  ASSERT_EQ(1, other_controller.GetEntryCount());
-
-  ASSERT_EQ(0, other_controller.GetCurrentEntryIndex());
-
-  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
-  EXPECT_EQ(0, other_controller.GetEntryAtIndex(0)->GetPageID());
-
-  // The max page ID map should be copied over and updated with the max page ID
-  // from the current tab.
-  SiteInstance* instance1 =
-      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0));
-  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
-}
-
-// Test CopyStateFromAndPrune with 2 urls, the first selected and nothing in
+  contents()->CommitPendingNavigation();
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url3);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain: url1, url3
+
+  ASSERT_EQ(2, other_controller.GetEntryCount());
+  ASSERT_EQ(1, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url3, other_controller.GetEntryAtIndex(1)->GetURL());
+  EXPECT_EQ(0, other_controller.GetEntryAtIndex(1)->GetPageID());
+
+  // The max page ID map should be copied over and updated with the max page ID
+  // from the current tab.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(1));
+  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
+}
+
+// Test CopyStateFromAndPrune with 2 urls, the last selected and 2 entries in
 // the target.
 TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo1");
   const GURL url2("http://foo2");
   const GURL url3("http://foo3");
+  const GURL url4("http://foo4");
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url3);
+  other_contents->NavigateAndCommit(url4);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(1)), 2,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain: url1, url2, url4
+
+  ASSERT_EQ(3, other_controller.GetEntryCount());
+  ASSERT_EQ(2, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url2, other_controller.GetEntryAtIndex(1)->GetURL());
+  EXPECT_EQ(url4, other_controller.GetEntryAtIndex(2)->GetURL());
+
+  // The max page ID map should be copied over and updated with the max page ID
+  // from the current tab.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(2));
+  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
+}
+
+// Test CopyStateFromAndPrune with 2 urls, 2 entries in the target, with
+// not the last entry selected in the target.
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneNotLast) {
+  NavigationControllerImpl& controller = controller_impl();
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+  const GURL url3("http://foo3");
+  const GURL url4("http://foo4");
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url3);
+  other_contents->NavigateAndCommit(url4);
+  other_controller.GoBack();
+  other_contents->CommitPendingNavigation();
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain: url1, url2, url3
+
+  ASSERT_EQ(3, other_controller.GetEntryCount());
+  ASSERT_EQ(2, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url2, other_controller.GetEntryAtIndex(1)->GetURL());
+  EXPECT_EQ(url3, other_controller.GetEntryAtIndex(2)->GetURL());
+
+  // The max page ID map should be copied over and updated with the max page ID
+  // from the current tab.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(2));
+  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
+}
+
+// Test CopyStateFromAndPrune with 2 urls, the first selected and 1 entry plus
+// a pending entry in the target.
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneTargetPending) {
+  NavigationControllerImpl& controller = controller_impl();
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+  const GURL url3("http://foo3");
+  const GURL url4("http://foo4");
 
   NavigateAndCommit(url1);
   NavigateAndCommit(url2);
   controller.GoBack();
-
-  scoped_ptr<TestWebContents> other_contents(
-      static_cast<TestWebContents*>(CreateTestWebContents()));
-  NavigationControllerImpl& other_controller = other_contents->GetController();
+  contents()->CommitPendingNavigation();
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url3);
   other_controller.LoadURL(
-      url3, Referrer(), PAGE_TRANSITION_TYPED, std::string());
-  other_contents->ExpectSetHistoryLengthAndPrune(NULL, 1, -1);
-  other_controller.CopyStateFromAndPrune(&controller);
-
-  // other_controller should now contain 1 entry for url1, and a pending entry
-  // for url3.
-
-  ASSERT_EQ(1, other_controller.GetEntryCount());
-
-  EXPECT_EQ(0, other_controller.GetCurrentEntryIndex());
-
-  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
-
-  // And there should be a pending entry for url3.
+      url4, Referrer(), PAGE_TRANSITION_TYPED, std::string());
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain url1, url3, and a pending entry
+  // for url4.
+
+  ASSERT_EQ(2, other_controller.GetEntryCount());
+  EXPECT_EQ(1, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url3, other_controller.GetEntryAtIndex(1)->GetURL());
+
+  // And there should be a pending entry for url4.
   ASSERT_TRUE(other_controller.GetPendingEntry());
-
-  EXPECT_EQ(url3, other_controller.GetPendingEntry()->GetURL());
+  EXPECT_EQ(url4, other_controller.GetPendingEntry()->GetURL());
 
   // The max page ID map should be copied over and updated with the max page ID
   // from the current tab.
   SiteInstance* instance1 =
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0));
   EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
 }
 
+// Test CopyStateFromAndPrune with 1 url in the source, 1 entry and a pending
+// client redirect entry (with the same page ID) in the target.  This used to
+// crash because the last committed entry would be pruned but max_page_id
+// remembered the page ID (http://crbug.com/234809).
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneTargetPending2) {
+  NavigationControllerImpl& controller = controller_impl();
+  const GURL url1("http://foo1");
+  const GURL url2a("http://foo2/a");
+  const GURL url2b("http://foo2/b");
+
+  NavigateAndCommit(url1);
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url2a);
+  // Simulate a client redirect, which has the same page ID as entry 2a.
+  other_controller.LoadURL(
+      url2b, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  other_controller.GetPendingEntry()->SetPageID(
+      other_controller.GetLastCommittedEntry()->GetPageID());
+
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain url1, url2a, and a pending entry
+  // for url2b.
+
+  ASSERT_EQ(2, other_controller.GetEntryCount());
+  EXPECT_EQ(1, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url2a, other_controller.GetEntryAtIndex(1)->GetURL());
+
+  // And there should be a pending entry for url4.
+  ASSERT_TRUE(other_controller.GetPendingEntry());
+  EXPECT_EQ(url2b, other_controller.GetPendingEntry()->GetURL());
+
+  // Let the pending entry commit.
+  other_contents->CommitPendingNavigation();
+
+  // The max page ID map should be copied over and updated with the max page ID
+  // from the current tab.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(1));
+  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
+}
+
+// Test CopyStateFromAndPrune with 2 urls, a back navigation pending in the
+// source, and 1 entry in the target. The back pending entry should be ignored.
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneSourcePending) {
+  NavigationControllerImpl& controller = controller_impl();
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+  const GURL url3("http://foo3");
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+  controller.GoBack();
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url3);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller);
+
+  // other_controller should now contain: url1, url2, url3
+
+  ASSERT_EQ(3, other_controller.GetEntryCount());
+  ASSERT_EQ(2, other_controller.GetCurrentEntryIndex());
+
+  EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->GetURL());
+  EXPECT_EQ(url2, other_controller.GetEntryAtIndex(1)->GetURL());
+  EXPECT_EQ(url3, other_controller.GetEntryAtIndex(2)->GetURL());
+  EXPECT_EQ(0, other_controller.GetEntryAtIndex(2)->GetPageID());
+
+  // The max page ID map should be copied over and updated with the max page ID
+  // from the current tab.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(2));
+  EXPECT_EQ(0, other_contents->GetMaxPageIDForSiteInstance(instance1));
+}
+
 // Tests CopyStateFromAndPrune with 3 urls in source, 1 in dest,
 // when the max entry count is 3.  We should prune one entry.
 TEST_F(NavigationControllerTest, CopyStateFromAndPruneMaxEntries) {
   NavigationControllerImpl& controller = controller_impl();
   size_t original_count = NavigationControllerImpl::max_entry_count();
   const int kMaxEntryCount = 3;
 
   NavigationControllerImpl::set_max_entry_count_for_testing(kMaxEntryCount);
 
   const GURL url1("http://foo/1");
@@ skipping 80 matching lines @@
   EXPECT_EQ(url3, nav_params.a.url);
   process()->sink().ClearMessages();
 
   // Make sure an extravagant history.go() doesn't break.
   contents()->OnGoToEntryAtOffset(120);  // Out of bounds.
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   message = process()->sink().GetFirstMessageMatching(ViewMsg_Navigate::ID);
   EXPECT_TRUE(message == NULL);
 }
 
-// Test call to PruneAllButActive for the only entry.
-TEST_F(NavigationControllerTest, PruneAllButActiveForSingle) {
+// Test call to PruneAllButVisible for the only entry.
+TEST_F(NavigationControllerTest, PruneAllButVisibleForSingle) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo1");
   NavigateAndCommit(url1);
 
   contents()->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(controller.GetEntryAtIndex(0)), 0,
       controller.GetEntryAtIndex(0)->GetPageID());
 
-  controller.PruneAllButActive();
+  controller.PruneAllButVisible();
 
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_EQ(controller.GetEntryAtIndex(0)->GetURL(), url1);
 }
 
-// Test call to PruneAllButActive for last entry.
-TEST_F(NavigationControllerTest, PruneAllButActiveForLast) {
+// Test call to PruneAllButVisible for first entry.
+TEST_F(NavigationControllerTest, PruneAllButVisibleForFirst) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo/1");
   const GURL url2("http://foo/2");
   const GURL url3("http://foo/3");
 
   NavigateAndCommit(url1);
   NavigateAndCommit(url2);
   NavigateAndCommit(url3);
   controller.GoBack();
   controller.GoBack();
   contents()->CommitPendingNavigation();
 
   contents()->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(controller.GetEntryAtIndex(0)), 0,
       controller.GetEntryAtIndex(0)->GetPageID());
 
-  controller.PruneAllButActive();
+  controller.PruneAllButVisible();
 
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_EQ(controller.GetEntryAtIndex(0)->GetURL(), url1);
 }
 
-// Test call to PruneAllButActive for intermediate entry.
-TEST_F(NavigationControllerTest, PruneAllButActiveForIntermediate) {
+// Test call to PruneAllButVisible for intermediate entry.
+TEST_F(NavigationControllerTest, PruneAllButVisibleForIntermediate) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo/1");
   const GURL url2("http://foo/2");
   const GURL url3("http://foo/3");
 
   NavigateAndCommit(url1);
   NavigateAndCommit(url2);
   NavigateAndCommit(url3);
   controller.GoBack();
   contents()->CommitPendingNavigation();
 
   contents()->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(controller.GetEntryAtIndex(1)), 0,
       controller.GetEntryAtIndex(1)->GetPageID());
 
-  controller.PruneAllButActive();
+  controller.PruneAllButVisible();
 
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_EQ(controller.GetEntryAtIndex(0)->GetURL(), url2);
 }
 
-// Test call to PruneAllButActive for a pending entry.
-TEST_F(NavigationControllerTest, PruneAllButActiveForPending) {
+// Test call to PruneAllButVisible for a pending entry that is not yet in the
+// list of entries.
+TEST_F(NavigationControllerTest, PruneAllButVisibleForPendingNotInList) {
   NavigationControllerImpl& controller = controller_impl();
   const GURL url1("http://foo/1");
   const GURL url2("http://foo/2");
-  const GURL url3("http://foo/3");
-
-  NavigateAndCommit(url1);
-  NavigateAndCommit(url2);
-  NavigateAndCommit(url3);
-  controller.GoBack();
-
-  contents()->ExpectSetHistoryLengthAndPrune(
-      GetSiteInstanceFromEntry(controller.GetEntryAtIndex(1)), 0,
-      controller.GetEntryAtIndex(1)->GetPageID());
-
-  controller.PruneAllButActive();
-
-  EXPECT_EQ(0, controller.GetPendingEntryIndex());
-}
-
-// Test call to PruneAllButActive for a pending entry that is not yet in the
-// list of entries.
-TEST_F(NavigationControllerTest, PruneAllButActiveForPendingNotInList) {
-  NavigationControllerImpl& controller = controller_impl();
-  const GURL url1("http://foo/1");
-  const GURL url2("http://foo/2");
   const GURL url3("http://foo/3");
 
   NavigateAndCommit(url1);
   NavigateAndCommit(url2);
 
   // Create a pending entry that is not in the entry list.
   controller.LoadURL(
       url3, Referrer(), PAGE_TRANSITION_TYPED, std::string());
   EXPECT_TRUE(controller.GetPendingEntry());
   EXPECT_EQ(2, controller.GetEntryCount());
 
   contents()->ExpectSetHistoryLengthAndPrune(
       NULL, 0, controller.GetPendingEntry()->GetPageID());
-  controller.PruneAllButActive();
+  controller.PruneAllButVisible();
 
-  // We should only have the pending entry at this point, and it should still
-  // not be in the entry list.
+  // We should only have the last committed and pending entries at this point,
+  // and the pending entry should still not be in the entry list.
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(url2, controller.GetEntryAtIndex(0)->GetURL());
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_TRUE(controller.GetPendingEntry());
-  EXPECT_EQ(0, controller.GetEntryCount());
+  EXPECT_EQ(1, controller.GetEntryCount());
 
   // Try to commit the pending entry.
   test_rvh()->SendNavigate(2, url3);
   EXPECT_EQ(-1, controller.GetPendingEntryIndex());
   EXPECT_FALSE(controller.GetPendingEntry());
-  EXPECT_EQ(1, controller.GetEntryCount());
-  EXPECT_EQ(url3, controller.GetEntryAtIndex(0)->GetURL());
-}
-
-// Test call to PruneAllButActive for transient entry.
-TEST_F(NavigationControllerTest, PruneAllButActiveForTransient) {
-  NavigationControllerImpl& controller = controller_impl();
-  const GURL url0("http://foo/0");
-  const GURL url1("http://foo/1");
-  const GURL transient_url("http://foo/transient");
-
-  controller.LoadURL(
-      url0, Referrer(), PAGE_TRANSITION_TYPED, std::string());
-  test_rvh()->SendNavigate(0, url0);
-  controller.LoadURL(
-      url1, Referrer(), PAGE_TRANSITION_TYPED, std::string());
-  test_rvh()->SendNavigate(1, url1);
-
-  // Adding a transient with no pending entry.
-  NavigationEntryImpl* transient_entry = new NavigationEntryImpl;
-  transient_entry->SetURL(transient_url);
-  controller.SetTransientEntry(transient_entry);
-
-  contents()->ExpectSetHistoryLengthAndPrune(
-      GetSiteInstanceFromEntry(controller.GetTransientEntry()), 0,
-      controller.GetTransientEntry()->GetPageID());
-
-  controller.PruneAllButActive();
-
-  EXPECT_EQ(-1, controller.GetPendingEntryIndex());
-  EXPECT_EQ(controller.GetTransientEntry()->GetURL(), transient_url);
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(url3, controller.GetEntryAtIndex(1)->GetURL());
 }
 
 // Test to ensure that when we do a history navigation back to the current
 // committed page (e.g., going forward to a slow-loading page, then pressing
 // the back button), we just stop the navigation to prevent the throbber from
 // running continuously. Otherwise, the RenderViewHost forces the throbber to
 // start, but WebKit essentially ignores the navigation and never sends a
 // message to stop the throbber.
 TEST_F(NavigationControllerTest, StopOnHistoryNavigationToCurrentPage) {
   NavigationControllerImpl& controller = controller_impl();
@@ skipping 427 matching lines @@
                     PAGE_TRANSITION_LINK);
   session_helper_.AssertNavigationEquals(nav,
                                          windows_[0]->tabs[0]->navigations[0]);
   nav.set_url(url2);
   session_helper_.AssertNavigationEquals(nav,
                                          windows_[0]->tabs[0]->navigations[1]);
 }
 */
 
 }  // namespace content