Additional fixes for prerender/instant + browsing history.

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 dd31fb7e922778713effed968a234cb92882269d..6b63784fd6ad99254bc8a316c7a047d596456d6b 100644
--- a/content/renderer/render_view.cc
+++ b/content/renderer/render_view.cc
@@ -1020,9 +1020,25 @@ void RenderView::OnScrollFocusedEditableNodeIntoView() {
   }
 }
 
-void RenderView::OnSetHistoryLengthAndClear(int history_length) {
+void RenderView::OnSetHistoryLengthAndClear(int history_length, int32 page_id) {
   DCHECK(history_length >= 0);
 
+  if (page_id >= 0) {
+    // It's possible that the underlying page could have done navigation
+    // at this point, which this isn't prepared for.
+    // TODO(cbentzel): Send a message to the browser? There will be a
+    // mismatch between NavigationController and RenderView here.
+    if (page_id_ != page_id) {

[Charlie Reis, 2011/08/11 20:49:24]

Dropping this message doesn't seem like the right thing to do if we know this
case is possible-- that means we know we'll end up with the broken back button
again, right?  It'll happen if the prerendered page starts navigating a second
time just before it's swapped in.  (Not common, but possible to write a test
for.)

Given this inherent race, are we really gaining something from verifying the
browser and renderer page_ids match?  Or should we just make sure the browser
handles it properly if it gets a new page_id from a renderer it has just swapped
in?  (Maybe it already does handle this case properly?)

+      return;
+    }
+  } else {
+    // The prerendered page could have been swapped after the initial Navigate
+    // message is sent but prior to the commit of the provisional load being
+    // done, and sent to the browser. In that case, we can't validate so
+    // use the current one.
+    DCHECK(page_id == -1);

[Charlie Reis, 2011/08/11 20:49:24]

I'm wondering if this whole block would be clearer by starting with
DCHECK(page_id >= -1) on line 1025 and putting this comment before the "if". 
Also, drop "done, and" to make it the comment bit easier to follow.

(Followup: I wrote this before the comment above, so maybe it's unnecessary.)

+  }
+
   // 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.