A number of issues weren't addressed with the earlier patch for prerender + browsing history, particularly for instant pages.

  - The "remove first entry" option used by instant was not being handled correctly when there was only one committed entry in the NavigationController.

  - Renderer-issued navigations which were committed in the browser but not yet known by the browser/NavigationController were being incorrectly pruned. This did not happen regularly in the prerender case, but does in the instant case, particularly when changing what's typed in the omnibox.

  - Some additional sanity testing to make sure that the message is sent to the correct render process.

  - Additional unit tests are added.

BUG=89798
TEST=NavigationControllerTest.CopyStateFromAndPrune*, RenderViewTest.SetHistoryLengthAndPrune.


Review URL: http://codereview.chromium.org/7618016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@96724 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 365 insertions, 57 deletions

diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index 6b842ef..0ff0e31 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -88,17 +88,27 @@ class PrerenderContents::TabContentsDelegateImpl
     return false;
   }
 
-  bool CanDownload(TabContents* source, int request_id) OVERRIDE {
+  virtual bool CanDownload(TabContents* source, int request_id) OVERRIDE {
     prerender_contents_->Destroy(FINAL_STATUS_DOWNLOAD);
     // Cancel the download.
     return false;
   }
 
-  void OnStartDownload(TabContents* source, DownloadItem* download) OVERRIDE {
+  virtual void OnStartDownload(TabContents* source,
+                               DownloadItem* download) OVERRIDE {
     // Prerendered pages should never be able to download files.
     NOTREACHED();
   }
 
+  virtual bool OnGoToEntryOffset(int offset) OVERRIDE {
+    // This isn't allowed because the history merge operation
+    // does not work if there are renderer issued challenges.
+    // TODO(cbentzel): Cancel in this case? May not need to do
+    // since render-issued offset navigations are not guaranteed,
+    // but indicates that the page cares about the history.
+    return false;
+  }
+
   // Commits the History of Pages to the given TabContents.
   void CommitHistory(TabContentsWrapper* tab) {
     for (size_t i = 0; i < add_page_vector_.size(); ++i)
@@ -604,4 +614,13 @@ Value* PrerenderContents::GetAsValue() const {
   return dict_value;
 }
 
+bool PrerenderContents::IsCrossSiteNavigationPending() const {
+  if (!prerender_contents_.get() || !prerender_contents_->tab_contents())
+    return false;
+  const TabContents* tab_contents = prerender_contents_->tab_contents();
+  return (tab_contents->GetSiteInstance() !=
+          tab_contents->GetPendingSiteInstance());
+}
+
+
 }  // namespace prerender
diff --git a/chrome/browser/prerender/prerender_contents.h b/chrome/browser/prerender/prerender_contents.h
index 48727ac..9a6d4df 100644
--- a/chrome/browser/prerender/prerender_contents.h
+++ b/chrome/browser/prerender/prerender_contents.h
@@ -161,6 +161,11 @@ class PrerenderContents : public NotificationObserver,
 
   base::Value* GetAsValue() const;
 
+  // Returns whether a pending cross-site navigation is happening.
+  // This could happen with renderer-issued navigations, such as a
+  // MouseEvent being dispatched by a link to a website installed as an app.
+  bool IsCrossSiteNavigationPending() const;
+
  protected:
   PrerenderContents(PrerenderManager* prerender_manager,
                     PrerenderTracker* prerender_tracker,
diff --git a/chrome/browser/prerender/prerender_final_status.cc b/chrome/browser/prerender/prerender_final_status.cc
index e3fa134..98e9ba9 100644
--- a/chrome/browser/prerender/prerender_final_status.cc
+++ b/chrome/browser/prerender/prerender_final_status.cc
@@ -45,6 +45,7 @@ const char* kFinalStatusNames[] = {
   "Cache or History Cleared",
   "Cancelled",
   "SSL Error",
+  "Cross-Site Navigation Pending",
   "Max",
 };
 COMPILE_ASSERT(arraysize(kFinalStatusNames) == FINAL_STATUS_MAX + 1,
diff --git a/chrome/browser/prerender/prerender_final_status.h b/chrome/browser/prerender/prerender_final_status.h
index e43462b..57619d5 100644
--- a/chrome/browser/prerender/prerender_final_status.h
+++ b/chrome/browser/prerender/prerender_final_status.h
@@ -46,6 +46,7 @@ enum FinalStatus {
   FINAL_STATUS_CACHE_OR_HISTORY_CLEARED = 31,
   FINAL_STATUS_CANCELLED = 32,
   FINAL_STATUS_SSL_ERROR = 33,
+  FINAL_STATUS_CROSS_SITE_NAVIGATION_PENDING = 34,
   FINAL_STATUS_MAX,
 };
 
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 99d9e2d..670f5ab 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -452,6 +452,14 @@ bool PrerenderManager::MaybeUsePrerenderedPage(TabContents* tab_contents,
     return false;
   }
 
+  // If the prerendered page is in the middle of a cross-site navigation,
+  // don't swap it in because there isn't a good way to merge histories.
+  if (prerender_contents->IsCrossSiteNavigationPending()) {
+    prerender_contents.release()->Destroy(
+        FINAL_STATUS_CROSS_SITE_NAVIGATION_PENDING);
+    return false;
+  }
+
   int child_id, route_id;
   CHECK(prerender_contents->GetChildId(&child_id));
   CHECK(prerender_contents->GetRouteId(&route_id));
diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc
index 8be92f9..8e77317 100644
--- a/chrome/browser/tab_contents/web_contents_unittest.cc
+++ b/chrome/browser/tab_contents/web_contents_unittest.cc
@@ -1711,6 +1711,9 @@ TEST_F(TabContentsTest, CopyStateFromAndPruneSourceInterstitial) {
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
   other_contents->NavigateAndCommit(url3);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      other_controller.GetEntryAtIndex(0)->site_instance(), 1,
+      other_controller.GetEntryAtIndex(0)->page_id());
   other_controller.CopyStateFromAndPrune(&controller(), false);
 
   // The merged controller should only have two entries: url1 and url2.
@@ -1751,7 +1754,9 @@ TEST_F(TabContentsTest, CopyStateFromAndPruneTargetInterstitial) {
   interstitial->TestDidNavigate(1, url3);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_EQ(2, other_controller.entry_count());
-
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      other_controller.GetEntryAtIndex(0)->site_instance(), 1,
+      other_controller.GetEntryAtIndex(0)->page_id());
   other_controller.CopyStateFromAndPrune(&controller(), false);
 
   // The merged controller should only have two entries: url1 and url2.
diff --git a/chrome/test/base/render_view_test.cc b/chrome/test/base/render_view_test.cc
index e66b244..f197e91 100644
--- a/chrome/test/base/render_view_test.cc
+++ b/chrome/test/base/render_view_test.cc
@@ -319,3 +319,10 @@ bool RenderViewTest::SimulateElementClick(const std::string& element_id) {
   view_->OnMessageReceived(*input_message);
   return true;
 }
+
+void RenderViewTest::ClearHistory() {
+  view_->page_id_ = -1;
+  view_->history_list_offset_ = -1;
+  view_->history_list_length_ = 0;
+  view_->history_page_ids_.clear();
+}
diff --git a/chrome/test/base/render_view_test.h b/chrome/test/base/render_view_test.h
index bebe27c..a72afc0 100644
--- a/chrome/test/base/render_view_test.h
+++ b/chrome/test/base/render_view_test.h
@@ -84,6 +84,9 @@ class RenderViewTest : public testing::Test {
   // the element was not found).
   bool SimulateElementClick(const std::string& element_id);
 
+  // Clears anything associated with the browsing history.
+  void ClearHistory();
+
   // testing::Test
   virtual void SetUp();
 
diff --git a/content/browser/tab_contents/navigation_controller.cc b/content/browser/tab_contents/navigation_controller.cc
index 0144fef..315a4711 100644
--- a/content/browser/tab_contents/navigation_controller.cc
+++ b/content/browser/tab_contents/navigation_controller.cc
@@ -883,6 +883,14 @@ void NavigationController::CopyStateFrom(const NavigationController& source) {
 
 void NavigationController::CopyStateFromAndPrune(NavigationController* source,
                                                  bool remove_first_entry) {
+  // The SiteInstance and page_id of the last committed entry needs to be
+  // remembered at this point, in case there is only one committed entry
+  // and it is pruned.
+  NavigationEntry* last_committed = GetLastCommittedEntry();
+  SiteInstance* site_instance =
+      last_committed ? last_committed->site_instance() : NULL;
+  int32 minimum_page_id = last_committed ? last_committed->page_id() : -1;
+
   // This code is intended for use when the last entry is the active entry.
   DCHECK((transient_entry_index_ != -1 &&
           transient_entry_index_ == entry_count() - 1) ||
@@ -891,6 +899,16 @@ void NavigationController::CopyStateFromAndPrune(NavigationController* source,
          (!pending_entry_ && last_committed_entry_index_ == entry_count() - 1));
 
   if (remove_first_entry && entry_count()) {
+    // If there is only one committed entry and |remove_first_entry| is true,
+    // it needs to be pruned. This is accomplished by specifying a larger
+    // |minimum_page_id| than the committed entry's page_id in the
+    // ViewMsg_SetHistoryLengthAndPrune message. However, any pages which are
+    // committed between now and when the RenderView handles the message will
+    // need to be retained. Both constraints can be met by incrementing the
+    // |minimum_page_id| by 1.
+    DCHECK(minimum_page_id >= 0);
+    if (entry_count() == 1)
+      ++minimum_page_id;
     // Save then restore the pending entry (RemoveEntryAtIndexInternal chucks
     // the pending entry).
     NavigationEntry* pending_entry = pending_entry_;
@@ -932,8 +950,9 @@ void NavigationController::CopyStateFromAndPrune(NavigationController* source,
       last_committed_entry_index_--;
   }
 
-  // Update the history in the RenderView.
-  tab_contents_->SetHistoryLengthAndClear(max_source_index);
+  tab_contents_->SetHistoryLengthAndPrune(site_instance,
+                                          max_source_index,
+                                          minimum_page_id);
 }
 
 void NavigationController::PruneAllButActive() {
diff --git a/content/browser/tab_contents/navigation_controller_unittest.cc b/content/browser/tab_contents/navigation_controller_unittest.cc
index f7992c2..4e1ec10 100644
--- a/content/browser/tab_contents/navigation_controller_unittest.cc
+++ b/content/browser/tab_contents/navigation_controller_unittest.cc
@@ -1923,6 +1923,9 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune) {
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
   other_contents->NavigateAndCommit(url3);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      other_controller.GetEntryAtIndex(0)->site_instance(), 2,
+      other_controller.GetEntryAtIndex(0)->page_id());
   other_controller.CopyStateFromAndPrune(&controller(), false);
 
   // other_controller should now contain the 3 urls: url1, url2 and url3.
@@ -1949,6 +1952,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune2) {
 
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
+  other_contents->ExpectSetHistoryLengthAndPrune(NULL, 1, -1);
   other_controller.CopyStateFromAndPrune(&controller(), false);
 
   // other_controller should now contain the 1 url: url1.
@@ -1974,6 +1978,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) {
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
   other_controller.LoadURL(url3, GURL(), PageTransition::TYPED);
+  other_contents->ExpectSetHistoryLengthAndPrune(NULL, 1, -1);
   other_controller.CopyStateFromAndPrune(&controller(), false);
 
   // other_controller should now contain 1 entry for url1, and a pending entry
@@ -2000,6 +2005,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune4) {
 
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
+  other_contents->ExpectSetHistoryLengthAndPrune(NULL, 1, -1);
   other_controller.CopyStateFromAndPrune(&controller(), true);
 
   // other_controller should now contain 1 entry for url1.
@@ -2025,6 +2031,10 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune5) {
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
   other_contents->NavigateAndCommit(url2);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      other_controller.GetEntryAtIndex(0)->site_instance(),
+      1,
+      other_controller.GetEntryAtIndex(0)->page_id() + 1);
   other_controller.CopyStateFromAndPrune(&controller(), true);
 
   // other_controller should now contain 1 entry, url1.
@@ -2050,6 +2060,10 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune6) {
   NavigationController& other_controller = other_contents->controller();
   other_contents->NavigateAndCommit(url2);
   other_controller.LoadURL(url3, GURL(), PageTransition::TYPED);
+  other_contents->ExpectSetHistoryLengthAndPrune(
+      other_controller.GetEntryAtIndex(0)->site_instance(),
+      1,
+      other_controller.GetEntryAtIndex(0)->page_id() + 1);
   other_controller.CopyStateFromAndPrune(&controller(), true);
 
   // other_controller should now contain 2 entries: url1, and url3.
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index 1fdccbf..b14a4cc 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -596,12 +596,29 @@ bool TabContents::NavigateToEntry(
   return true;
 }
 
-void TabContents::SetHistoryLengthAndClear(int history_length) {
+void TabContents::SetHistoryLengthAndPrune(const SiteInstance* site_instance,
+                                           int history_length,
+                                           int32 minimum_page_id) {
+  // SetHistoryLengthAndPrune doesn't handle pending cross-site navigations
+  // cleanly. Since it's only used when swapping in instant and prerendered
+  // TabContents, checks are done at a higher level to ensure that the pages
+  // are not swapped in during this case.
+  if (render_manager_.pending_render_view_host()) {
+    NOTREACHED();
+    return;
+  }
   RenderViewHost* rvh = render_view_host();
-  if (!rvh)
+  if (!rvh) {
+    NOTREACHED();
     return;
-  rvh->Send(new ViewMsg_SetHistoryLengthAndClear(rvh->routing_id(),
-                                                 history_length));
+  }
+  if (site_instance && rvh->site_instance() != site_instance) {
+    NOTREACHED();
+    return;
+  }
+  rvh->Send(new ViewMsg_SetHistoryLengthAndPrune(rvh->routing_id(),
+                                                 history_length,
+                                                 minimum_page_id));
 }
 
 void TabContents::Stop() {
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 13b5c28..3f416e4 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -626,9 +626,10 @@ class TabContents : public PageNavigator,
   // Sets the history for this tab_contents to |history_length| entries, and
   // moves the current page_id to the last entry in the list if it's valid.
   // This is mainly used when a prerendered page is swapped into the current
-  // tab.
-  void SetHistoryLengthAndClear(int history_length);
-
+  // tab. The method is virtual for testing.
+  virtual void SetHistoryLengthAndPrune(const SiteInstance* site_instance,
+                                        int merge_history_length,
+                                        int32 minimum_page_id);
 
   // Misc non-view stuff -------------------------------------------------------
 
diff --git a/content/browser/tab_contents/test_tab_contents.cc b/content/browser/tab_contents/test_tab_contents.cc
index 4252114..13cb1d9 100644
--- a/content/browser/tab_contents/test_tab_contents.cc
+++ b/content/browser/tab_contents/test_tab_contents.cc
@@ -17,7 +17,14 @@ TestTabContents::TestTabContents(content::BrowserContext* browser_context,
                                  SiteInstance* instance)
     : TabContents(browser_context, instance, MSG_ROUTING_NONE, NULL, NULL),
       transition_cross_site(false),
-      delegate_view_override_(NULL) {
+      delegate_view_override_(NULL),
+      expect_set_history_length_and_prune_(false),
+      expect_set_history_length_and_prune_site_instance_(NULL),
+      expect_set_history_length_and_prune_history_length_(0),
+      expect_set_history_length_and_prune_min_page_id_(-1) {
+}
+
+TestTabContents::~TestTabContents() {
 }
 
 TestRenderViewHost* TestTabContents::pending_rvh() const {
@@ -90,3 +97,23 @@ RenderViewHostDelegate::View* TestTabContents::GetViewDelegate() {
     return delegate_view_override_;
   return TabContents::GetViewDelegate();
 }
+
+void TestTabContents::ExpectSetHistoryLengthAndPrune(
+    const SiteInstance* site_instance,
+    int history_length,
+    int32 min_page_id) {
+  expect_set_history_length_and_prune_ = true;
+  expect_set_history_length_and_prune_site_instance_ = site_instance;
+  expect_set_history_length_and_prune_history_length_ = history_length;
+  expect_set_history_length_and_prune_min_page_id_ = min_page_id;
+}
+
+void TestTabContents::SetHistoryLengthAndPrune(
+    const SiteInstance* site_instance, int history_length, int32 min_page_id) {
+  EXPECT_TRUE(expect_set_history_length_and_prune_);
+  expect_set_history_length_and_prune_ = false;
+  EXPECT_EQ(expect_set_history_length_and_prune_site_instance_, site_instance);
+  EXPECT_EQ(expect_set_history_length_and_prune_history_length_,
+            history_length);
+  EXPECT_EQ(expect_set_history_length_and_prune_min_page_id_, min_page_id);
+}
diff --git a/content/browser/tab_contents/test_tab_contents.h b/content/browser/tab_contents/test_tab_contents.h
index fb7b1bc..72170ab 100644
--- a/content/browser/tab_contents/test_tab_contents.h
+++ b/content/browser/tab_contents/test_tab_contents.h
@@ -17,6 +17,7 @@ class TestTabContents : public TabContents {
  public:
   TestTabContents(content::BrowserContext* browser_context,
                   SiteInstance* instance);
+  virtual ~TestTabContents();
 
   TestRenderViewHost* pending_rvh() const;
 
@@ -42,12 +43,12 @@ class TestTabContents : public TabContents {
 
   // Prevent interaction with views.
   virtual bool CreateRenderViewForRenderManager(
-      RenderViewHost* render_view_host);
-  virtual void UpdateRenderViewSizeForRenderManager() {}
+      RenderViewHost* render_view_host) OVERRIDE;
+  virtual void UpdateRenderViewSizeForRenderManager() OVERRIDE {}
 
   // Returns a clone of this TestTabContents. The returned object is also a
   // TestTabContents. The caller owns the returned object.
-  virtual TabContents* Clone();
+  virtual TabContents* Clone() OVERRIDE;
 
   // Creates a pending navigation to the given URL with the default parameters
   // and then commits the load with a page ID one larger than any seen. This
@@ -67,12 +68,33 @@ class TestTabContents : public TabContents {
   bool transition_cross_site;
 
   // Allow mocking of the RenderViewHostDelegate::View.
-  virtual RenderViewHostDelegate::View* GetViewDelegate();
+  virtual RenderViewHostDelegate::View* GetViewDelegate() OVERRIDE;
   void set_view_delegate(RenderViewHostDelegate::View* view) {
     delegate_view_override_ = view;
   }
+
+  // Establish expected arguments for |SetHistoryLengthAndPrune()|. When
+  // |SetHistoryLengthAndPrune()| is called, the arguments are compared
+  // with the expected arguments specified here.
+  void ExpectSetHistoryLengthAndPrune(const SiteInstance* site_instance,
+                                      int history_length,
+                                      int32 min_page_id);
+
+  // Compares the arguments passed in with the expected arguments passed in
+  // to |ExpectSetHistoryLengthAndPrune()|.
+  virtual void SetHistoryLengthAndPrune(const SiteInstance* site_instance,
+                                        int history_length,
+                                        int32 min_page_id) OVERRIDE;
+
  private:
   RenderViewHostDelegate::View* delegate_view_override_;
+
+  // Expectations for arguments of |SetHistoryLengthAndPrune()|.
+  bool expect_set_history_length_and_prune_;
+  scoped_refptr<const SiteInstance>
+    expect_set_history_length_and_prune_site_instance_;
+  int expect_set_history_length_and_prune_history_length_;
+  int32 expect_set_history_length_and_prune_min_page_id_;
 };
 
 #endif  // CONTENT_BROWSER_TAB_CONTENTS_TEST_TAB_CONTENTS_H_
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index c0850c9..da0687c 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -748,17 +748,19 @@ IPC_STRUCT_END()
 IPC_MESSAGE_CONTROL1(ViewMsg_SetNextPageID,
                      int32 /* next_page_id */)
 
-// Sets the history length of page_ids for a RenderView to
-// |length| entries, and moves the current page_id to the last
-// entry if it is valid.
-// The main use for this is prerendered pages, but Instant pages also use this.
-// For example, assume that there are 3 entries in the history when a
-// prerendered page is created. The new prerendered page will have a single
-// entry history like [7]. When it is swapped in, we need to extend the history
-// so it has a total length of 4 (3 for the previous history, 1 for the
-// prerendered page), so it looks like [-1 -1 -1 7].
-IPC_MESSAGE_ROUTED1(ViewMsg_SetHistoryLengthAndClear,
-                    int /* length */)
+// Sent to the RenderView when a prerendered or instant page is committed
+// to an existing tab. The existing tab has a history of
+// |merged_history_length| which precedes the current history of pages
+// in the render view. All page_ids >= |minimum_page_id| are appended to
+// this new history in the same order.
+//
+// For example, suppose the history of page_ids in the instant RenderView
+// is [4 7 8]. This instant RenderView is committed, and merged into
+// an existing tab with 3 history items, with every page with page_id >= 7
+// is preserved. The resulting page history is [-1 -1 -1 7 8].
+IPC_MESSAGE_ROUTED2(ViewMsg_SetHistoryLengthAndPrune,
+                    int, /* merge_history_length */
+                    int32 /* minimum_page_id */)
 
 // Sends System Colors corresponding to a set of CSS color keywords
 // down the pipe.
diff --git a/content/renderer/render_view.cc b/content/renderer/render_view.cc
index d23bf5e..065593a 100644
--- a/content/renderer/render_view.cc
+++ b/content/renderer/render_view.cc
@@ -708,8 +708,9 @@ bool RenderView::OnMessageReceived(const IPC::Message& message) {
 #endif
     IPC_MESSAGE_HANDLER(ViewMsg_UpdateRemoteAccessClientFirewallTraversal,
                         OnUpdateRemoteAccessClientFirewallTraversal)
-    IPC_MESSAGE_HANDLER(ViewMsg_SetHistoryLengthAndClear,
-                        OnSetHistoryLengthAndClear)
+    IPC_MESSAGE_HANDLER(ViewMsg_SetHistoryLengthAndPrune,
+                        OnSetHistoryLengthAndPrune)
+
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(handled = RenderWidget::OnMessageReceived(message))
   IPC_END_MESSAGE_MAP()
@@ -1005,6 +1006,27 @@ void RenderView::OnSelectRange(const gfx::Point& start, const gfx::Point& end) {
   handling_select_range_ = false;
 }
 
+void RenderView::OnSetHistoryLengthAndPrune(int history_length,
+                                            int32 minimum_page_id) {
+  DCHECK(history_length >= 0);
+  DCHECK(history_list_offset_ == history_list_length_ - 1);
+  DCHECK(minimum_page_id >= -1);
+
+  // Generate the new list.
+  std::vector<int32> new_history_page_ids(history_length, -1);
+  for (size_t i = 0; i < history_page_ids_.size(); ++i) {
+    if (minimum_page_id >= 0 && history_page_ids_[i] < minimum_page_id)
+      continue;
+    new_history_page_ids.push_back(history_page_ids_[i]);
+  }
+  new_history_page_ids.swap(history_page_ids_);
+
+  // Update indexes.
+  history_list_length_ = history_page_ids_.size();
+  history_list_offset_ = history_list_length_ - 1;
+}
+
+
 void RenderView::OnSetInitialFocus(bool reverse) {
   if (!webview())
     return;
@@ -1032,32 +1054,6 @@ void RenderView::OnScrollFocusedEditableNodeIntoView() {
   }
 }
 
-void RenderView::OnSetHistoryLengthAndClear(int history_length) {
-  DCHECK(history_length >= 0);
-
-  // history_list_length_ may be 0 if this is called between
-  // a navigate and a commit of the provisional load. Otherwise,
-  // only add one entry, regardless of how long the current history is.
-  // TODO(cbentzel): Investigate what happens if a prerendered page
-  //   navigates to several entries before it is swapped in. Cropping
-  //   those may be a bad idea.
-  int new_history_list_length = history_length;
-  if (history_list_length_ > 0)
-    ++new_history_list_length;
-
-  DCHECK(page_id_ == -1 ||
-         (history_list_offset_ >= 0 &&
-          page_id_ == history_page_ids_[history_list_offset_]));
-
-  // Generate the new list.
-  std::vector<int32> new_history_page_ids(new_history_list_length, -1);
-  if (page_id_ != -1)
-    new_history_page_ids[new_history_list_length - 1] = page_id_;
-  new_history_page_ids.swap(history_page_ids_);
-  history_list_offset_ = new_history_list_length - 1;
-  history_list_length_ = new_history_list_length;
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 // Tell the embedding application that the URL of the active page has changed
diff --git a/content/renderer/render_view.h b/content/renderer/render_view.h
index a467ff7..55705e3 100644
--- a/content/renderer/render_view.h
+++ b/content/renderer/render_view.h
@@ -666,6 +666,7 @@ class RenderView : public RenderWidget,
 #if defined(OS_MACOSX)
   FRIEND_TEST_ALL_PREFIXES(RenderViewTest, MacTestCmdUp);
 #endif
+  FRIEND_TEST_ALL_PREFIXES(RenderViewTest, SetHistoryLengthAndPrune);
 
   typedef std::map<GURL, double> HostZoomLevels;
 
@@ -834,6 +835,7 @@ class RenderView : public RenderWidget,
   void OnSetBackground(const SkBitmap& background);
   void OnSetWebUIProperty(const std::string& name, const std::string& value);
   void OnSetEditCommandsForNextKeyEvent(const EditCommands& edit_commands);
+  void OnSetHistoryLengthAndPrune(int history_length, int32 minimum_page_id);
   void OnSetInitialFocus(bool reverse);
 #if defined(OS_MACOSX)
   void OnSetInLiveResize(bool in_live_resize);
@@ -855,7 +857,6 @@ class RenderView : public RenderWidget,
   void OnUpdateTargetURLAck();
   void OnUpdateWebPreferences(const WebPreferences& prefs);
   void OnUpdateRemoteAccessClientFirewallTraversal(const std::string& policy);
-  void OnSetHistoryLengthAndClear(int history_length);
 
 #if defined(OS_MACOSX)
   void OnWindowFrameChanged(const gfx::Rect& window_frame,
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 5a85672..b175a5e 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -914,3 +914,163 @@ TEST_F(RenderViewTest, UpdateTargetURLWithInvalidURL) {
   view_->setMouseOverURL(WebKit::WebURL(invalid_gurl));
   EXPECT_EQ(invalid_gurl, view_->target_url_);
 }
+
+TEST_F(RenderViewTest, SetHistoryLengthAndPrune) {
+  int expected_page_id = -1;
+
+  // No history to merge and no committed pages.
+  view_->OnSetHistoryLengthAndPrune(0, -1);
+  EXPECT_EQ(0, view_->history_list_length_);
+  EXPECT_EQ(-1, view_->history_list_offset_);
+
+  // History to merge and no committed pages.
+  view_->OnSetHistoryLengthAndPrune(2, -1);
+  EXPECT_EQ(2, view_->history_list_length_);
+  EXPECT_EQ(1, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  ClearHistory();
+
+  // No history to merge and a committed page to be kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(0, expected_page_id);
+  EXPECT_EQ(1, view_->history_list_length_);
+  EXPECT_EQ(0, view_->history_list_offset_);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[0]);
+  ClearHistory();
+
+  // No history to merge and a committed page to be pruned.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(0, expected_page_id + 1);
+  EXPECT_EQ(0, view_->history_list_length_);
+  EXPECT_EQ(-1, view_->history_list_offset_);
+  ClearHistory();
+
+  // No history to merge and a committed page that the browser was unaware of.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(0, -1);
+  EXPECT_EQ(1, view_->history_list_length_);
+  EXPECT_EQ(0, view_->history_list_offset_);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[0]);
+  ClearHistory();
+
+  // History to merge and a committed page to be kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(2, expected_page_id);
+  EXPECT_EQ(3, view_->history_list_length_);
+  EXPECT_EQ(2, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[2]);
+  ClearHistory();
+
+  // History to merge and a committed page to be pruned.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(2, expected_page_id + 1);
+  EXPECT_EQ(2, view_->history_list_length_);
+  EXPECT_EQ(1, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  ClearHistory();
+
+  // History to merge and a committed page that the browser was unaware of.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->OnSetHistoryLengthAndPrune(2, -1);
+  EXPECT_EQ(3, view_->history_list_length_);
+  EXPECT_EQ(2, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[2]);
+  ClearHistory();
+
+  int expected_page_id_2 = -1;
+
+  // No history to merge and two committed pages, both to be kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(0, expected_page_id);
+  EXPECT_EQ(2, view_->history_list_length_);
+  EXPECT_EQ(1, view_->history_list_offset_);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[0]);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[1]);
+  ClearHistory();
+
+  // No history to merge and two committed pages, and only the second is kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(0, expected_page_id_2);
+  EXPECT_EQ(1, view_->history_list_length_);
+  EXPECT_EQ(0, view_->history_list_offset_);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[0]);
+  ClearHistory();
+
+  // No history to merge and two committed pages, both of which the browser was
+  // unaware of.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(0, -1);
+  EXPECT_EQ(2, view_->history_list_length_);
+  EXPECT_EQ(1, view_->history_list_offset_);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[0]);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[1]);
+  ClearHistory();
+
+  // History to merge and two committed pages, both to be kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(2, expected_page_id);
+  EXPECT_EQ(4, view_->history_list_length_);
+  EXPECT_EQ(3, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[2]);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[3]);
+  ClearHistory();
+
+  // History to merge and two committed pages, and only the second is kept.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(2, expected_page_id_2);
+  EXPECT_EQ(3, view_->history_list_length_);
+  EXPECT_EQ(2, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[2]);
+  ClearHistory();
+
+  // History to merge and two committed pages, both of which the browser was
+  // unaware of.
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id = view_->page_id_;
+  view_->didCommitProvisionalLoad(GetMainFrame(), true);
+  expected_page_id_2 = view_->page_id_;
+  EXPECT_GT(expected_page_id_2, expected_page_id);
+  view_->OnSetHistoryLengthAndPrune(2, -1);
+  EXPECT_EQ(4, view_->history_list_length_);
+  EXPECT_EQ(3, view_->history_list_offset_);
+  EXPECT_EQ(-1, view_->history_page_ids_[0]);
+  EXPECT_EQ(-1, view_->history_page_ids_[1]);
+  EXPECT_EQ(expected_page_id, view_->history_page_ids_[2]);
+  EXPECT_EQ(expected_page_id_2, view_->history_page_ids_[3]);
+}