Track down crash during RenderFrameImpl::didCommitProvisionalLoad.

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 bc43dc39a3be545f3d88307cc15e0e988d125049..1ac3bc57251b718063f805cb0ec55a72ac81620c 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1497,8 +1497,20 @@ void RenderFrameImpl::OnSwapOut(
   // Now that all of the cleanup is complete and the browser side is notified,
   // start using the RenderFrameProxy, if one is created.
   if (proxy && swapped_out_forbidden) {
+    // The swap call deletes this RenderFrame via frameDetached.  Do not access
+    // any members after this call.
+    // TODO(creis): WebFrame::swap() can return false.  Most of those cases
+    // should be due to the frame being detached during unload (in which case
+    // the necessary cleanup has happened anyway), but it might be possible for
+    // it to return false without detaching.  Catch those cases below to track
+    // down https://crbug.com/575245.
     frame_->swap(proxy->web_frame());

[Charlie Reis, 2016/01/13 23:30:49]

dcheng: I'm not sure what we should do differently here if swap returns false. 
In some of those cases, the frame will have been detached (via JS), and in
others it might not (e.g., m_provisionalDocumentLoader check in
FrameLoader::prepareForCommit).

Can swap have an invariant that the frame is always detached afterward?  (It
might be able to make the distinction between the cases mentioned above.)  

Do we need the return value, given that no one is listening to it?

[dcheng, 2016/01/14 00:15:25]

We need to figure out the behavior of sync navigations in unload events first.
From my really simple testing, it appears that Chrome is the only browser (I
haven't tested Edge) that allows a fragment navigation in an unload event to
take effect. japhet@ points out that we should probably test pushState /
replaceState / about:blank as well.

This is an edge case I'm perfectly happy not to support, but we should probably
get some language in the spec about this (I don't see anything about this right
now). It's also sort of a web-facing change, even if it's just Chrome.
Well, if we wanted to preserve current Chrome behavior, running code after the
failed swap could be incorrect: for example, mandoline updated some WebFrame
pointers after calling swap(). These updates are incorrect if the swap() is
cancelled.

 
+    // For main frames, the swap should have cleared the RenderView's pointer to
+    // this frame.
+    if (is_main_frame)
+      CHECK(!render_view->main_render_frame_);
+
     if (is_loading)
       proxy->OnDidStartLoading();
   }