When a proxy is detached, immediately delete its associated provisional frame.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 69b323bfe8b23a263509f8b251fd8ba9011056da..a04147ba921d797bb998848c9adc7958c05609cc 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1004,6 +1004,7 @@ void RenderFrameImpl::CreateFrame(
     render_frame = RenderFrameImpl::Create(proxy->render_view(), routing_id);
     render_frame->InitializeBlameContext(nullptr);
     render_frame->proxy_routing_id_ = proxy_routing_id;
+    proxy->set_provisional_frame_routing_id(routing_id);
     web_frame = blink::WebLocalFrame::createProvisional(
         render_frame, proxy->web_frame(), replicated_state.sandbox_flags);
   }
@@ -1610,19 +1611,6 @@ void RenderFrameImpl::OnNavigate(
     const CommonNavigationParams& common_params,
     const StartNavigationParams& start_params,
     const RequestNavigationParams& request_params) {
-  // If this RenderFrame is going to replace a RenderFrameProxy, it is possible
-  // that the proxy was detached before this navigation request was received.
-  // In that case, abort the navigation.  See https://crbug.com/526304 and
-  // https://crbug.com/568676.
-  // TODO(nasko, alexmos): Eventually, the browser process will send an IPC to
-  // clean this frame up after https://crbug.com/548275 is fixed.
-  if (proxy_routing_id_ != MSG_ROUTING_NONE) {
-    RenderFrameProxy* proxy =
-        RenderFrameProxy::FromRoutingID(proxy_routing_id_);
-    if (!proxy)
-      return;
-  }
-
   RenderThreadImpl* render_thread_impl = RenderThreadImpl::current();
   // Can be NULL in tests.
   if (render_thread_impl)
@@ -3138,6 +3126,21 @@ void RenderFrameImpl::frameDetached(blink::WebLocalFrame* frame,
   frame->close();
   frame_ = nullptr;
 
+  // If this was a provisional frame with an associated proxy, tell the proxy
+  // that it's no longer associated with this frame.
+  if (proxy_routing_id_ != MSG_ROUTING_NONE) {
+    RenderFrameProxy* proxy =
+        RenderFrameProxy::FromRoutingID(proxy_routing_id_);
+
+    // |proxy| should always exist.  Detaching the proxy would've also detached
+    // this provisional frame.  The proxy should also not be associated with
+    // another provisional frame at this point.
+    CHECK(proxy);
+    CHECK_EQ(proxy->provisional_frame_routing_id(), routing_id_);

[Charlie Reis, 2017/01/18 00:18:41]

I'm ok with these checks-- I agree with the reasoning.

nit: routing_id_ seems like the expected value to me, so maybe swap the order of
the parameters to CHECK_EQ?

[alexmos, 2017/01/23 22:20:59]

Done.

+
+    proxy->set_provisional_frame_routing_id(MSG_ROUTING_NONE);
+  }
+
   delete this;
   // Object is invalid after this point.
 }
@@ -4994,14 +4997,13 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
 bool RenderFrameImpl::SwapIn() {
   CHECK_NE(proxy_routing_id_, MSG_ROUTING_NONE);
   CHECK(!in_frame_tree_);
-  RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(proxy_routing_id_);
 
-  // The proxy might have been detached while the provisional LocalFrame was
-  // being navigated.  In that case, don't swap the frame back in the tree
-  // and return early (to avoid sending confusing IPCs to the browser
-  // process).  See https://crbug.com/526304 and https://crbug.com/568676.
-  if (!proxy)
-    return false;
+  // The proxy should always exist.  If it was detached while the provisional
+  // LocalFrame was being navigated, the provisional frame would've been
+  // cleaned up by RenderFrameProxy::frameDetached.  See
+  // https://crbug.com/526304 and https://crbug.com/568676 for context.
+  RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(proxy_routing_id_);
+  CHECK(proxy);
 
   int proxy_routing_id = proxy_routing_id_;
   if (!proxy->web_frame()->swap(frame_))