Keep a copy of page id in RenderViewHost.

Also, ensure that it stays in sync with the renderer's copy.

BUG=99379,369661
TEST=no CHECKs hit in the wild, all tests stay green

Review URL: https://codereview.chromium.org/423393008

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

content/browser/frame_host/navigation_controller_impl_unittest.cc

@@ -1004,7 +1004,7 @@ TEST_F(NavigationControllerTest, LoadURL_RedirectAbortDoesntShowPendingURL) {
   const GURL kExistingURL("http://foo/eh");
   controller.LoadURL(kExistingURL, content::Referrer(),
                      content::PAGE_TRANSITION_TYPED, std::string());
-  main_test_rfh()->SendNavigate(0, kExistingURL);
+  main_test_rfh()->SendNavigate(1, kExistingURL);
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
 
@@ -1032,7 +1032,7 @@ TEST_F(NavigationControllerTest, LoadURL_RedirectAbortDoesntShowPendingURL) {
   const GURL kRedirectURL("http://foo/see");
   main_test_rfh()->OnMessageReceived(
       FrameHostMsg_DidRedirectProvisionalLoad(0,  // routing_id
-                                              -1,  // pending page_id
+                                              1,  // pending page_id
                                               kNewURL,  // old url
                                               kRedirectURL));  // new url
 

content/browser/frame_host/render_frame_host_impl.cc

@@ -552,8 +552,9 @@ void RenderFrameHostImpl::OnDidRedirectProvisionalLoad(
     int32 page_id,
     const GURL& source_url,
     const GURL& target_url) {
+  CHECK_EQ(render_view_host_->page_id_, page_id);
   frame_tree_node_->navigator()->DidRedirectProvisionalLoad(
-      this, page_id, source_url, target_url);
+      this, render_view_host_->page_id_, source_url, target_url);
 }
 
 // Called when the renderer navigates.  For every frame loaded, we'll get this
@@ -573,6 +574,10 @@ void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
       Read(&msg, &iter, &validated_params))
     return;
 
+  // Update the RVH's current page ID so that future IPCs from the renderer
+  // correspond to the new page.
+  render_view_host_->page_id_ = validated_params.page_id;
+
   // If we're waiting for a cross-site beforeunload ack from this renderer and
   // we receive a Navigate message from the main frame, then the renderer was
   // navigating already and sent it before hearing the ViewMsg_Stop message.
@@ -885,6 +890,7 @@ void RenderFrameHostImpl::OnUpdateTitle(
     int32 page_id,
     const base::string16& title,
     blink::WebTextDirection title_direction) {
+  CHECK_EQ(render_view_host_->page_id_, page_id);
   // This message is only sent for top-level frames. TODO(avi): when frame tree
   // mirroring works correctly, add a check here to enforce it.
   if (title.length() > kMaxTitleChars) {
@@ -892,7 +898,7 @@ void RenderFrameHostImpl::OnUpdateTitle(
     return;
   }
 
-  delegate_->UpdateTitle(this, page_id, title,
+  delegate_->UpdateTitle(this, render_view_host_->page_id_, title,
                          WebTextDirectionToChromeTextDirection(
                              title_direction));
 }

content/browser/renderer_host/render_view_host_impl.cc

@@ -190,6 +190,7 @@ RenderViewHostImpl::RenderViewHostImpl(
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
       navigations_suspended_(false),
+      page_id_(-1),
       main_frame_routing_id_(main_frame_routing_id),
       run_modal_reply_msg_(NULL),
       run_modal_opener_id_(MSG_ROUTING_NONE),
@@ -283,8 +284,10 @@ bool RenderViewHostImpl::CreateRenderView(
   // Ensure the RenderView starts with a next_page_id larger than any existing
   // page ID it might be asked to render.
   int32 next_page_id = 1;
-  if (max_page_id > -1)
+  if (max_page_id > -1) {
     next_page_id = max_page_id + 1;
+    page_id_ = max_page_id;
+  }
 
   ViewMsg_New_Params params;
   params.renderer_preferences =
@@ -1111,6 +1114,7 @@ void RenderViewHostImpl::OnRenderProcessGone(int status, int exit_code) {
 }
 
 void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) {
+  CHECK_EQ(page_id_, page_id);
   // Without this check, the renderer can trick the browser into using
   // filenames it can't access in a future session restore.
   if (!CanAccessFilesOfPageState(state)) {
@@ -1118,12 +1122,13 @@ void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) {
     return;
   }
 
-  delegate_->UpdateState(this, page_id, state);
+  delegate_->UpdateState(this, page_id_, state);
 }
 
 void RenderViewHostImpl::OnUpdateTargetURL(int32 page_id, const GURL& url) {
+  CHECK_EQ(page_id_, page_id);
   if (IsRVHStateActive(rvh_state_))
-    delegate_->UpdateTargetURL(page_id, url);
+    delegate_->UpdateTargetURL(page_id_, url);
 
   // Send a notification back to the renderer that we are ready to
   // receive more target urls.

content/browser/renderer_host/render_view_host_impl.h

@@ -547,6 +547,11 @@ class CONTENT_EXPORT RenderViewHostImpl
   // TODO(nasko): Move to RenderFrameHost, as this is per-frame state.
   scoped_ptr<FrameMsg_Navigate_Params> suspended_nav_params_;
 
+  // The most recent page ID we've heard from the renderer process.  This is
+  // used as context when other session history related IPCs arrive.
+  // TODO(creis): Allocate this in WebContents/NavigationController instead.
+  int32 page_id_;
+
   // The current state of this RVH.
   // TODO(nasko): Move to RenderFrameHost, as this is per-frame state.
   RenderViewHostImplState rvh_state_;

content/browser/renderer_host/render_view_host_unittest.cc

@@ -214,12 +214,12 @@ TEST_F(RenderViewHostTest, MessageWithBadHistoryItemFiles) {
   EXPECT_TRUE(PathService::Get(base::DIR_TEMP, &file_path));
   file_path = file_path.AppendASCII("foo");
   EXPECT_EQ(0, process()->bad_msg_count());
-  test_rvh()->TestOnUpdateStateWithFile(process()->GetID(), file_path);
+  test_rvh()->TestOnUpdateStateWithFile(-1, file_path);
   EXPECT_EQ(1, process()->bad_msg_count());
 
   ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile(
       process()->GetID(), file_path);
-  test_rvh()->TestOnUpdateStateWithFile(process()->GetID(), file_path);
+  test_rvh()->TestOnUpdateStateWithFile(-1, file_path);
   EXPECT_EQ(1, process()->bad_msg_count());
 }
 

content/test/test_render_view_host.cc

@@ -345,9 +345,9 @@ void TestRenderViewHost::TestOnStartDragging(
 }
 
 void TestRenderViewHost::TestOnUpdateStateWithFile(
-    int process_id,
+    int page_id,
     const base::FilePath& file_path) {
-  OnUpdateState(process_id,
+  OnUpdateState(page_id,
                 PageState::CreateForTesting(GURL("http://www.google.com"),
                                             false,
                                             "data",

content/test/test_render_view_host.h

@@ -251,7 +251,7 @@ class TestRenderViewHost
       FrameHostMsg_DidCommitProvisionalLoad_Params* params);
 
   void TestOnUpdateStateWithFile(
-      int process_id, const base::FilePath& file_path);
+      int page_id, const base::FilePath& file_path);
 
   void TestOnStartDragging(const DropData& drop_data);