Additional fixes for prerender/instant + browsing history.

content/renderer/render_view.cc

 // Copyright (c) 2011 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 "content/renderer/render_view.h"
 
 #include <algorithm>
 #include <cmath>
 #include <string>
 #include <vector>
@@ skipping 686 matching lines @@
     IPC_MESSAGE_HANDLER(ViewMsg_ContextMenuClosed, OnContextMenuClosed)
     // TODO(viettrungluu): Move to a separate message filter.
 #if defined(ENABLE_FLAPPER_HACKS)
     IPC_MESSAGE_HANDLER(PepperMsg_ConnectTcpACK, OnConnectTcpACK)
 #endif
 #if defined(OS_MACOSX)
     IPC_MESSAGE_HANDLER(ViewMsg_SetInLiveResize, OnSetInLiveResize)
 #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()
   return handled;
 }
 
 void RenderView::OnNavigate(const ViewMsg_Navigate_Params& params) {
   if (!webview())
     return;
 
@@ skipping 267 matching lines @@
 
 void RenderView::OnSelectRange(const gfx::Point& start, const gfx::Point& end) {
   if (!webview())
     return;
 
   handling_select_range_ = true;
   webview()->focusedFrame()->selectRange(start, end);
   handling_select_range_ = false;
 }
 
+void RenderView::OnSetHistoryLengthAndPrune(int history_length,
+                                            int32 minimum_page_id) {
+  DCHECK(history_length >= 0);
+  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) {
+    DCHECK(history_page_ids_[i] >= 0);

[Charlie Reis, 2011/08/13 18:39:24]

This isn't guaranteed, is it?  history_page_ids_ can contain -1s for pages
rendered by a different RenderView (e.g., the NTP).

[cbentzel, 2011/08/13 21:24:28]

That's correct. In that case we will still want to prune them, as they will be
earlier in the history than the minimum_page_id that we want to retain. There
would be an exception if there was a cross-boundary history.back() [or other
back navigation] but this is disallowed in both the prerender and instant case
which uses it, and the NaviigationController also does not expect it in this
case.

+    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();
+  // TODO(cbentzel): What do we do if the history_list_offset_ was not at
+  // the end of the list before?

[Charlie Reis, 2011/08/13 18:39:24]

Can we be pointing to a page that we've pruned?  We wouldn't want to be showing
a page that is no longer in the history.  What do you think is the right
behavior?

[cbentzel, 2011/08/13 21:24:28]

At this point, we can't. If the page was pruned it won't be in the
history_page_ids_ at this point. The RenderViewTest.SetHistoryLengthAndPrune
test covers some of the corner cases, such as the browser thinking there is only
one page_id which it wants to prune [in the instant case], but a renderer-issued
navigation commits in the interim.

+  history_list_offset_ = history_list_length_ - 1;
+}
+
+
 void RenderView::OnSetInitialFocus(bool reverse) {
   if (!webview())
     return;
   webview()->setInitialFocus(reverse);
 }
 
 #if defined(OS_MACOSX)
 void RenderView::OnSetInLiveResize(bool in_live_resize) {
   if (!webview())
     return;
   if (in_live_resize)
     webview()->willStartLiveResize();
   else
     webview()->willEndLiveResize();
 }
 #endif
 
 void RenderView::OnScrollFocusedEditableNodeIntoView() {
   WebKit::WebNode node = GetFocusedNode();
   if (!node.isNull()) {
     if (IsEditableNode(node))
       // TODO(varunjain): Change webkit API to scroll a particular node into
       // view and use that API here instead.
       webview()->scrollFocusedNodeIntoView();
   }
 }
 
-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
 void RenderView::UpdateURL(WebFrame* frame) {
   WebDataSource* ds = frame->dataSource();
   DCHECK(ds);
 
   const WebURLRequest& request = ds->request();
   const WebURLRequest& original_request = ds->originalRequest();
   const WebURLResponse& response = ds->response();
@@ skipping 3425 matching lines @@
 }
 #endif
 
 void RenderView::OnContextMenuClosed(
     const webkit_glue::CustomContextMenuContext& custom_context) {
   if (custom_context.is_pepper_menu)
     pepper_delegate_.OnContextMenuClosed(custom_context);
   else
     context_menu_node_.reset();
 }