Prevent RFH from attempting to transfer after it's been swapped out

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 824063f5578537866e17c5e92f9baa0695283deb..a461a527235510150b912ef22f442b6eec85fbef 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -665,6 +665,14 @@ bool NavigationHandleImpl::MaybeTransferAndProceedInternal() {
   RenderFrameHostManager* manager =
       render_frame_host_->frame_tree_node()->render_manager();
 
+  // It is possible for the RenderFrameHost to have been marked as pending
+  // deletion following the commit of another ongoing navigation. In that case,
+  // don't attempt to transfer, but don't resume the navigation either.
+  if (render_frame_host_ != manager->current_frame_host() &&

[Charlie Reis, 2016/09/30 20:25:26]

We used to use is_active() for this (in OnCrossSiteResponse).  That's probably a
better check to use here, given that we're getting |manager| from
|render_frame_host_| anyway.

[clamy, 2016/10/04 11:54:41]

Acknowledged.

+      render_frame_host_ != manager->pending_frame_host()) {
+    return false;

[clamy, 2016/09/30 14:06:15]

This will just suspend the navigation. Alternatively, I could reset the
NavigationHandle in the RFH, which would just cancel the navigation.
Other option: we delete the NavHandle when swapping out the RFH, this would
ensure the navigation will eventually stop & that no updates are sent to the
embedder about a navigation in an RFH that is pending deletion.

[Charlie Reis, 2016/09/30 20:25:26]

We definitely want to cancel the navigation.  The old code did that by deleting
cross_site_transferring_request (unless leak_requests_for_testing_ was true).
I like this idea.  Can we ensure the navigation gets canceled if the IO thread
posts its task and no handle is found?  And if we did that, could we put a
CHECK(render_frame_host_->is_active()) here?

[nasko, 2016/09/30 23:21:33]

+1 on cancelling the navigation. Keeping it suspended only creates ambiguity.

[clamy, 2016/10/04 11:54:41]

Done. The cancellation if the NavigationHandle isn't found has been in place
since the previous patch (it was what was causing issues on Android). I've added
a check a bit earlier at the start of the check for transfer sequence.

+  }
+
   // In the site-per-process model, the RenderFrameHostManager may also decide
   // (independently from the content embedder's ShouldSwapProcessesForRedirect
   // above) that a process transfer is needed. Process transfers are skipped for