Clear RenderFrameImpl::frame_ pointer after deleting it.

content/renderer/render_frame_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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 934 matching lines @@
 
 #if defined(OS_MACOSX) || defined(OS_ANDROID)
 void RenderFrameImpl::DidHideExternalPopupMenu() {
   // We need to clear external_popup_menu_ as soon as ExternalPopupMenu::close
   // is called. Otherwise, createExternalPopupMenu() for new popup will fail.
   external_popup_menu_.reset();
 }
 #endif
 
 bool RenderFrameImpl::OnMessageReceived(const IPC::Message& msg) {
+  // We may get here while detaching, when the WebFrame has been deleted.  Do
+  // not process any messages in this state.
+  if (!frame_)
+    return false;
+
   // TODO(kenrb): document() should not be null, but as a transitional step
   // we have RenderFrameProxy 'wrapping' a RenderFrameImpl, passing messages
   // to this method. This happens for a top-level remote frame, where a
   // document-less RenderFrame is replaced by a RenderFrameProxy but kept
   // around and is still able to receive messages.
   if (!frame_->document().isNull())
     GetContentClient()->SetActiveURL(frame_->document().url());
 
   ObserverListBase<RenderFrameObserver>::Iterator it(&observers_);
   RenderFrameObserver* observer;
@@ skipping 1132 matching lines @@
   // containing RenderViewHost (so that they have the same lifetime), so only
   // removal from the map is needed and no deletion.
   FrameMap::iterator it = g_frame_map.Get().find(frame);
   CHECK(it != g_frame_map.Get().end());
   CHECK_EQ(it->second, this);
   g_frame_map.Get().erase(it);
 
   if (is_subframe)
     frame->parent()->removeChild(frame);
 
-  // |frame| is invalid after here.
+  // |frame| is invalid after here.  Be sure to clear frame_ as well, since this
+  // object may not be deleted immediately and other methods may try to access
+  // it.
   frame->close();
+  frame_ = nullptr;
 
   if (is_subframe) {
     delete this;

[nasko, 2015/03/16 17:12:17]

Based on this, isn't it the case that frame_ is invalid between the close() call
above and the destruction of RenderFrameImpl, only if it is the main frame? Have
any of the crashes we've seen been in the case of subframes?
If that's the case, I think it will be useful to clarify this in the header file
comment, as main frame and subframe lifetimes will always behave differently.

[Charlie Reis, 2015/03/16 17:21:18]

Yes, it's only for the main frame.  Comment updated.

     // Object is invalid after this point.
   }
 }
 
 void RenderFrameImpl::frameFocused() {
   Send(new FrameHostMsg_FrameFocused(routing_id_));
 }
 
 void RenderFrameImpl::willClose(blink::WebFrame* frame) {
   DCHECK(!frame_ || frame_ == frame);
@@ skipping 2401 matching lines @@
 
 #if defined(ENABLE_BROWSER_CDMS)
 RendererCdmManager* RenderFrameImpl::GetCdmManager() {
   if (!cdm_manager_)
     cdm_manager_ = new RendererCdmManager(this);
   return cdm_manager_;
 }
 #endif  // defined(ENABLE_BROWSER_CDMS)
 
 }  // namespace content