Remove incorrect sanity checks in RenderFrameHostManager::OnCrossSiteResponse.

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 0f50c83b5dde216001d5657a7af35b89dc1c69c9..f1a03687a873820cb3b6d10cfdc7a57c1f12196a 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -458,15 +458,25 @@ void RenderFrameHostManager::OnCrossSiteResponse(
   pending_render_frame_host->set_is_loading(false);
 
   // Sanity check that the params are for the correct frame and process.
-  // These should match the RenderFrameHost that made the request.
-  // If it started as a cross-process navigation via OpenURL, this is the
-  // pending one.  If it wasn't cross-process until the transfer, this is
-  // the current one.
-  int render_frame_id = pending_render_frame_host_
-                            ? pending_render_frame_host_->GetRoutingID()
-                            : render_frame_host_->GetRoutingID();
-  DCHECK_EQ(render_frame_id, pending_render_frame_host->GetRoutingID());
-  int process_id = pending_render_frame_host_ ?
+  // These should match the RenderFrameHost that made the request. If it
+  // started as a cross-process navigation via OpenURL, this is the pending
+  // one.  If it wasn't cross-process until the transfer, this is the current
+  // one.
+  //
+  // Note that having a pending RenderFrameHost does not imply that it was the
+  // one that made the request.  Suppose that during a pending cross-site
+  // navigation, the frame performs a different same-site navigation which
+  // redirects cross-site.  In this case, there will be a pending
+  // RenderFrameHost, but this request is made by the current RenderFrameHost.
+  // Later, this will create a new pending RenderFrameHost and clean up the old
+  // one.
+  int request_routing_id = pending_render_frame_host->GetRoutingID();
+  int pending_routing_id = pending_render_frame_host_ ?
+      pending_render_frame_host_->GetRoutingID() :
+      MSG_ROUTING_NONE;
+  DCHECK(request_routing_id == pending_routing_id ||
+         request_routing_id == render_frame_host_->GetRoutingID());

[Charlie Reis, 2016/02/26 23:51:19]

This seems redundant with line 438.  (It kind of was before, too, except for the
case where it was wrong.)  :)

We're basically just saying that the RFH parameter should match either the
current or pending RFH, and that's handled above.

[alexmos, 2016/02/27 00:20:28]

Acknowledged.

+  int process_id = request_routing_id == pending_routing_id ?

[Charlie Reis, 2016/02/26 23:51:19]

Looking closer, I don't think this first part is right.  The pending and current
RFHs could have the same routing ID but different process IDs, and this would
make us pick the pending RFH's process ID even if the request came from the
current RFH.

But maybe we don't need this.  The process_id DCHECK below is basically just
comparing the RFH parameter's GetProcess()->GetID() with
global_request_id.child_id now.  We could do that if we wanted, but it doesn't
seem likely to fail in practice.

[alexmos, 2016/02/27 00:20:28]

Indeed, you're right.  Thanks for catching it!  I agree the check on line 438 is
enough, so I've removed both of these DCHECKs and merged the comment here to
line 438.

       pending_render_frame_host_->GetProcess()->GetID() :
       render_frame_host_->GetProcess()->GetID();
   DCHECK_EQ(process_id, global_request_id.child_id);