Keep a copy of page id in RenderViewHost.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/browser/frame_host/render_frame_host_impl.h"
 
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/user_metrics_action.h"
@@ skipping 508 matching lines @@
   GetProcess()->FilterURL(false, &validated_url);
 
   frame_tree_node_->navigator()->DidFailLoadWithError(
       this, validated_url, error_code, error_description);
 }
 
 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
 // notification containing parameters identifying the navigation.
 //
 // Subframes are identified by the page transition type.  For subframes loaded
 // as part of a wider page load, the page_id will be the same as for the top
 // 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) {
   // Read the parameters out of the IPC message directly to avoid making another
   // copy when we filter the URLs.
   PickleIterator iter(msg);
   FrameHostMsg_DidCommitProvisionalLoad_Params validated_params;
   if (!IPC::ParamTraits<FrameHostMsg_DidCommitProvisionalLoad_Params>::
       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.
   // We do not want to cancel the pending navigation in this case, since the
   // old page will soon be stopped.  Instead, treat this as a beforeunload ack
   // to allow the pending navigation to continue.
   if (render_view_host_->is_waiting_for_beforeunload_ack_ &&
       render_view_host_->unload_ack_is_for_cross_site_transition_ &&
       PageTransitionIsMainFrame(validated_params.transition)) {
     OnBeforeUnloadACK(true, send_before_unload_start_time_,
@@ skipping 292 matching lines @@
 void RenderFrameHostImpl::OnDidDisownOpener() {
   // This message is only sent for top-level frames. TODO(avi): when frame tree
   // mirroring works correctly, add a check here to enforce it.
   delegate_->DidDisownOpener(this);
 }
 
 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) {
     NOTREACHED() << "Renderer sent too many characters in title.";
     return;
   }
 
-  delegate_->UpdateTitle(this, page_id, title,
+  delegate_->UpdateTitle(this, render_view_host_->page_id_, title,
                          WebTextDirectionToChromeTextDirection(
                              title_direction));
 }
 
 void RenderFrameHostImpl::OnUpdateEncoding(const std::string& encoding_name) {
   // This message is only sent for top-level frames. TODO(avi): when frame tree
   // mirroring works correctly, add a check here to enforce it.
   delegate_->UpdateEncoding(this, encoding_name);
 }
 
@@ skipping 291 matching lines @@
       FROM_HERE,
       base::Bind(
           &TransitionRequestManager::SetHasPendingTransitionRequest,
           base::Unretained(TransitionRequestManager::GetInstance()),
           GetProcess()->GetID(),
           routing_id_,
           has_pending_request));
 }
 
 }  // namespace content