Keep a copy of page id in RenderViewHost.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index ff3c1382cf72be50421641755221f429c984fe8d..9af7c969eba5eafe5e12db3dd28fd1b825bb0132 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -526,8 +526,9 @@ void RenderFrameHostImpl::OnDidRedirectProvisionalLoad(
     int32 page_id,
     const GURL& source_url,
     const GURL& target_url) {
+  CHECK_EQ(render_view_host_->page_id_, page_id);

[Tom Sepez, 2014/08/07 17:07:51]

Would prefer not to crash the browser here, instead try to kill the renderer via
BadMessageReceived.

[Charlie Reis, 2014/08/07 17:38:00]

That wouldn't let us see the cause of the kill in the crash reports, though,
would it?  How would we detect if these specific checks were failing in the
wild?  For context, we're planning to leave this in for a few days on Dev
channel and then remove the IPC parameter entirely.

(Also, what's the difference between BrowserMessageFilter::BadMessageReceived
and RPH::ReceivedBadMessage?)

Note: Let's be sure to do the same thing in RenderViewHostImpl.

[Avi (use Gerrit), 2014/08/07 18:37:06]

I was thinking about crash keys, but that's in the wrong process. In many cases
a UMA stat is recorded, but that wouldn't help us here.

I don't have an answer.
You should totally unify those.

   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
@@ -547,6 +548,9 @@ void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
       Read(&msg, &iter, &validated_params))
     return;
 
+  // Update the RVH's current page ID so that other IPCs make sense.

[Charlie Reis, 2014/08/07 17:01:20]

Let's fix my wording from the draft CL:
// Update the RVH's current page ID so that future IPCs from the renderer
correspond to the new page.

[Avi (use Gerrit), 2014/08/07 18:37:06]

Done.

+  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.
@@ -859,6 +863,7 @@ void RenderFrameHostImpl::OnUpdateTitle(
     int32 page_id,
     const base::string16& title,
     blink::WebTextDirection title_direction) {
+  CHECK_EQ(render_view_host_->page_id_, page_id);

[Tom Sepez, 2014/08/07 17:07:51]

avoid browser crash here, too, kill renderer.

   // 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) {
@@ -866,7 +871,7 @@ void RenderFrameHostImpl::OnUpdateTitle(
     return;
   }
 
-  delegate_->UpdateTitle(this, page_id, title,
+  delegate_->UpdateTitle(this, render_view_host_->page_id_, title,
                          WebTextDirectionToChromeTextDirection(
                              title_direction));
 }