Keep a copy of page id in RenderViewHost.

Note that this copy may not be completely in sync with the renderer’s copy. That’s OK.

BUG=407376
TEST=no visible change
R=creis@chromium.org, nasko@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#293771}

content/browser/frame_host/navigation_controller_impl_unittest.cc

@@ -1003,7 +1003,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;
 
@@ -1031,7 +1031,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

@@ -346,7 +346,7 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidFailLoadWithError,
                         OnDidFailLoadWithError)
     IPC_MESSAGE_HANDLER_GENERIC(FrameHostMsg_DidCommitProvisionalLoad,
-                                OnNavigate(msg))
+                                OnDidCommitProvisionalLoad(msg))
     IPC_MESSAGE_HANDLER(FrameHostMsg_OpenURL, OnOpenURL)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DocumentOnLoadCompleted,
                         OnDocumentOnLoadCompleted)
@@ -362,6 +362,7 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidAccessInitialDocument,
                         OnDidAccessInitialDocument)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidDisownOpener, OnDidDisownOpener)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_DidAssignPageId, OnDidAssignPageId)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateTitle, OnUpdateTitle)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateEncoding, OnUpdateEncoding)
     IPC_MESSAGE_HANDLER(FrameHostMsg_BeginNavigation,
@@ -588,7 +589,7 @@ void RenderFrameHostImpl::OnDidRedirectProvisionalLoad(
 // level frame.  If the user explicitly requests a subframe navigation, we will
 // get a new page_id because we need to create a new navigation entry for that
 // action.
-void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
+void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
   // Read the parameters out of the IPC message directly to avoid making another
   // copy when we filter the URLs.
   PickleIterator iter(msg);
@@ -596,7 +597,7 @@ void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
   if (!IPC::ParamTraits<FrameHostMsg_DidCommitProvisionalLoad_Params>::
       Read(&msg, &iter, &validated_params))
     return;
-  TRACE_EVENT1("navigation", "RenderFrameHostImpl::OnNavigate",
+  TRACE_EVENT1("navigation", "RenderFrameHostImpl::OnDidCommitProvisionalLoad",
                "url", validated_params.url.possibly_invalid_spec());
 
   // If we're waiting for a cross-site beforeunload ack from this renderer and
@@ -745,13 +746,13 @@ void RenderFrameHostImpl::OnBeforeUnloadACK(
   render_view_host_->decrement_in_flight_event_count();
   render_view_host_->StopHangMonitorTimeout();
   // If this renderer navigated while the beforeunload request was in flight, we
-  // may have cleared this state in OnNavigate, in which case we can ignore
-  // this message.
+  // may have cleared this state in OnDidCommitProvisionalLoad, in which case we
+  // can ignore this message.
   // However renderer might also be swapped out but we still want to proceed
   // with navigation, otherwise it would block future navigations. This can
   // happen when pending cross-site navigation is canceled by a second one just
-  // before OnNavigate while current RVH is waiting for commit but second
-  // navigation is started from the beginning.
+  // before OnDidCommitProvisionalLoad while current RVH is waiting for commit
+  // but second navigation is started from the beginning.
   if (!render_view_host_->is_waiting_for_beforeunload_ack_) {
     return;
   }
@@ -928,6 +929,12 @@ void RenderFrameHostImpl::OnDidDisownOpener() {
   delegate_->DidDisownOpener(this);
 }
 
+void RenderFrameHostImpl::OnDidAssignPageId(int32 page_id) {
+  // Update the RVH's current page ID so that future IPCs from the renderer
+  // correspond to the new page.
+  render_view_host_->page_id_ = page_id;
+}
+
 void RenderFrameHostImpl::OnUpdateTitle(
     int32 page_id,
     const base::string16& title,

content/browser/frame_host/render_frame_host_impl.h

@@ -323,7 +323,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void OnDidRedirectProvisionalLoad(int32 page_id,
                                     const GURL& source_url,
                                     const GURL& target_url);
-  void OnNavigate(const IPC::Message& msg);
+  void OnDidCommitProvisionalLoad(const IPC::Message& msg);
   void OnBeforeUnloadACK(
       bool proceed,
       const base::TimeTicks& renderer_before_unload_start_time,
@@ -351,6 +351,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
                                           size_t end_offset);
   void OnDidAccessInitialDocument();
   void OnDidDisownOpener();
+  void OnDidAssignPageId(int32 page_id);
   void OnUpdateTitle(int32 page_id,
                      const base::string16& title,
                      blink::WebTextDirection title_direction);

content/browser/renderer_host/render_view_host_impl.cc

@@ -184,6 +184,7 @@ RenderViewHostImpl::RenderViewHostImpl(
       instance_(static_cast<SiteInstanceImpl*>(instance)),
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
+      page_id_(-1),
       main_frame_routing_id_(main_frame_routing_id),
       run_modal_reply_msg_(NULL),
       run_modal_opener_id_(MSG_ROUTING_NONE),

content/browser/renderer_host/render_view_host_impl.h

@@ -466,6 +466,10 @@ class CONTENT_EXPORT RenderViewHostImpl
   // See BindingsPolicy for details.
   int enabled_bindings_;
 
+  // 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.

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/common/frame_messages.h

@@ -580,6 +580,10 @@ IPC_MESSAGE_ROUTED0(FrameHostMsg_DidAccessInitialDocument)
 // the window. Sent for top-level frames.
 IPC_MESSAGE_ROUTED0(FrameHostMsg_DidDisownOpener)
 
+// Notifies the browser that a page id was assigned.
+IPC_MESSAGE_ROUTED1(FrameHostMsg_DidAssignPageId,
+                    int32 /* page_id */)
+
 // Changes the title for the page in the UI when the page is navigated or the
 // title changes. Sent for top-level frames.
 IPC_MESSAGE_ROUTED3(FrameHostMsg_UpdateTitle,

content/renderer/render_frame_impl.cc

@@ -2236,6 +2236,8 @@ void RenderFrameImpl::didCommitProvisionalLoad(
     }
   }
 
+  Send(new FrameHostMsg_DidAssignPageId(routing_id_, render_view_->page_id_));
+
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers_,
                     DidCommitProvisionalLoad(frame, is_new_navigation));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_,

content/test/test_render_frame_host.cc

@@ -111,7 +111,7 @@ void TestRenderFrameHost::SendNavigateWithFile(
 void TestRenderFrameHost::SendNavigateWithParams(
     FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
   FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), *params);
-  OnNavigate(msg);
+  OnDidCommitProvisionalLoad(msg);
 }
 
 void TestRenderFrameHost::SendNavigateWithRedirects(
@@ -163,7 +163,7 @@ void TestRenderFrameHost::SendNavigateWithParameters(
       file_path_for_history_item);
 
   FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), params);
-  OnNavigate(msg);
+  OnDidCommitProvisionalLoad(msg);
 }
 
 void TestRenderFrameHost::SendBeginNavigationWithURL(const GURL& url) {

content/test/test_render_view_host.cc

@@ -350,9 +350,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

@@ -250,7 +250,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);