Handle should_replace_current_entry in prerender.

content/browser/frame_host/navigation_controller_impl_unittest.cc

Index: content/browser/frame_host/navigation_controller_impl_unittest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index 2f0cac26e334e62d84934cca5cf0d8dcde4c1eea..0fb057130bdfd5acf5f35cf017a82b70386b6222 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -3093,7 +3093,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain the 3 urls: url1, url2 and url3.
 
@@ -3139,7 +3139,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune2) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain: url1, url3
 
@@ -3177,7 +3177,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(1)), 2,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain: url1, url2, url4
 
@@ -3217,7 +3217,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneNotLast) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain: url1, url2, url3
 
@@ -3258,7 +3258,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneTargetPending) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain url1, url3, and a pending entry
   // for url4.
@@ -3305,7 +3305,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneTargetPending2) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 1,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain url1, url2a, and a pending entry
   // for url2b.
@@ -3349,7 +3349,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneSourcePending) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // other_controller should now contain: url1, url2, url3
 
@@ -3396,7 +3396,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneMaxEntries) {
   other_contents->ExpectSetHistoryLengthAndPrune(
       GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
       other_controller.GetEntryAtIndex(0)->GetPageID());
-  other_controller.CopyStateFromAndPrune(&controller);
+  other_controller.CopyStateFromAndPrune(&controller, false);
 
   // We should have received a pruned notification.
   EXPECT_EQ(1, listener.notification_count_);
@@ -3419,6 +3419,107 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPruneMaxEntries) {
   NavigationControllerImpl::set_max_entry_count_for_testing(original_count);
 }
 
+// Tests CopyStateFromAndPrune with 2 urls in source, 1 in dest, with
+// replace_entry set to true.
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneReplaceEntry) {
+  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);
+
+  // First two entries should have the same SiteInstance.
+  SiteInstance* instance1 =
+      GetSiteInstanceFromEntry(controller.GetEntryAtIndex(0));
+  SiteInstance* instance2 =
+      GetSiteInstanceFromEntry(controller.GetEntryAtIndex(1));
+  EXPECT_EQ(instance1, instance2);
+  EXPECT_EQ(0, controller.GetEntryAtIndex(0)->GetPageID());
+  EXPECT_EQ(1, controller.GetEntryAtIndex(1)->GetPageID());
+  EXPECT_EQ(1, contents()->GetMaxPageIDForSiteInstance(instance1));
+
+  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, true);
+
+  // other_controller should now contain the 2 urls: url1 and 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(0)->GetPageID());
+  EXPECT_EQ(0, other_controller.GetEntryAtIndex(1)->GetPageID());
+
+  // A new SiteInstance should be used for the new tab.
+  SiteInstance* instance3 =
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(1));
+  EXPECT_NE(instance3, instance1);

[Charlie Reis, 2013/12/11 02:57:39]

Yikes, these are two different SiteInstances for the same site in the same tab. 
I suppose we decided that we allow that for prerendering by putting them in
different BrowsingInstances...

Please update the comment to say "A new SiteInstance in a different
BrowsingInstance should be used for the new entry" and add a check that
IsRelatedSiteInstance returns false.

[davidben, 2013/12/11 21:20:59]

I just got that from the existing CopyStateFromAndPrune test. Updated both
versions.

+
+  // 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));
+}
+
+// Tests CopyStateFromAndPrune with 3 urls in source, 1 in dest, when the max
+// entry count is 3 and replace_entry is true.  We should not prune entries.
+TEST_F(NavigationControllerTest, CopyStateFromAndPruneMaxEntriesReplaceEntry) {
+  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");
+  const GURL url2("http://foo/2");
+  const GURL url3("http://foo/3");
+  const GURL url4("http://foo/4");
+
+  // Create a PrunedListener to observe prune notifications.
+  PrunedListener listener(&controller);
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+  NavigateAndCommit(url3);
+
+  scoped_ptr<TestWebContents> other_contents(
+      static_cast<TestWebContents*>(CreateTestWebContents()));
+  NavigationControllerImpl& other_controller = other_contents->GetController();
+  other_contents->NavigateAndCommit(url4);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      GetSiteInstanceFromEntry(other_controller.GetEntryAtIndex(0)), 2,
+      other_controller.GetEntryAtIndex(0)->GetPageID());
+  other_controller.CopyStateFromAndPrune(&controller, true);
+
+  // We should have received no pruned notification.
+  EXPECT_EQ(0, listener.notification_count_);
+
+  // other_controller should now contain only 3 urls: url1, url2 and 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());
+  EXPECT_EQ(0, other_controller.GetEntryAtIndex(0)->GetPageID());
+  EXPECT_EQ(1, other_controller.GetEntryAtIndex(1)->GetPageID());
+  EXPECT_EQ(0, other_controller.GetEntryAtIndex(2)->GetPageID());
+
+  NavigationControllerImpl::set_max_entry_count_for_testing(original_count);
+}
+
 // Tests that navigations initiated from the page (with the history object)
 // work as expected without navigation entries.
 TEST_F(NavigationControllerTest, HistoryNavigate) {