Fix regression with back-button not working on prerendered and instant pages.

content/renderer/render_view.cc

Index: content/renderer/render_view.cc
diff --git a/content/renderer/render_view.cc b/content/renderer/render_view.cc
index 972349f99357ff3f7c6cac7319e352f3c04c2599..962fe66a5f00130dfedde848c6121fd0606e332e 100644
--- a/content/renderer/render_view.cc
+++ b/content/renderer/render_view.cc
@@ -703,6 +703,8 @@ bool RenderView::OnMessageReceived(const IPC::Message& message) {
 #endif
     IPC_MESSAGE_HANDLER(ViewMsg_UpdateRemoteAccessClientFirewallTraversal,
                         OnUpdateRemoteAccessClientFirewallTraversal)
+    IPC_MESSAGE_HANDLER(ViewMsg_SetHistoryLengthAndClear,
+                        OnSetHistoryLengthAndClear)
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(handled = RenderWidget::OnMessageReceived(message))
   IPC_END_MESSAGE_MAP()
@@ -1017,6 +1019,31 @@ 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.

[Charlie Reis, 2011/08/09 22:51:42]

Please add a TODO about investigating what happens if a prerendered page
navigates to several entries before it is swapped in.  Seems like it could cause
issues if we simply cropped those entries from the history, since the page might
assume it could go back to earlier states.

[cbentzel, 2011/08/09 23:14:23]

This does match what the NavigationController does in CopyStateFromAndPrune - it
leaves only the "active entry" - but this may be incorrect behavior. Another
approach is to cancel prerenders if location's or pushState's are attempted. 

Regardless, I added a TODO and will investigate.

+  int new_history_list_length = history_length;
+  if (history_list_length_ > 0) {

[Charlie Reis, 2011/08/09 22:51:42]

Nit: no braces on a one-liner.

[cbentzel, 2011/08/09 23:14:23]

I always mismatch the style guide here [perhaps due to latent disagreement with
the rule].

Done.

+    ++new_history_list_length;
+  }
+
+  // Remember the current page id for the end of the list.
+  int current_page_id = -1;

[Charlie Reis, 2011/08/09 22:51:42]

Is this needed, or is page_id_ sufficient?

[cbentzel, 2011/08/09 23:14:23]

page_id_ does look sufficient. I'm wondering if that should be removed longer
term to prevent redundant data which may drift. 

+  if (history_list_offset_ >= 0)
+    current_page_id = history_page_ids_[history_list_offset_];
+
+  // Generate the new list.
+  std::vector<int32> new_history_page_ids_(new_history_list_length, -1);

[Charlie Reis, 2011/08/09 22:51:42]

Nit: local variable shouldn't have an underscore, should it?

[cbentzel, 2011/08/09 23:14:23]

Done.

+  if (current_page_id != -1)
+    new_history_page_ids_[new_history_list_length - 1] = current_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