Fix renderer crashes when frame gets detached while injectng user scripts.

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/command_line.h"
 #include "base/debug/alias.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/i18n/char_iterator.h"
+#include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/process/kill.h"
 #include "base/process/process.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/child/appcache/appcache_dispatcher.h"
 #include "content/child/plugin_messages.h"
 #include "content/child/quota_dispatcher.h"
 #include "content/child/request_extra_data.h"
 #include "content/child/service_worker/web_service_worker_provider_impl.h"
@@ skipping 1013 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->close();
+  // We might detach frame as a result of observer actions. Defer closing
+  // of frame until event loop so that observer still have valid frame object.
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&RenderFrameImpl::closeFrame, frame));

[dcheng, 2014/02/26 19:37:23]

We can simplify this slightly:
base::Bind(&WebFrame::close, base::Unretained(frame))

No need to thunk through a static. That being said, I've given this some thought
and I think we should move this logic into WebFrame. So you might be able to
remove this part from your CL completely (we'll still need the null checks that
were added)

 
   if (is_subframe) {
     delete this;
     // Object is invalid after this point.
   }
 }
 
+// static
+void RenderFrameImpl::closeFrame(blink::WebFrame* frame) {
+  // |frame| is invalid after here.
+  frame->close();
+}
+
 void RenderFrameImpl::willClose(blink::WebFrame* frame) {
   // Call back to RenderViewImpl for observers to be notified.
   // TODO(nasko): Remove once we have RenderFrameObserver.
   render_view_->willClose(frame);
 }
 
 void RenderFrameImpl::didChangeName(blink::WebFrame* frame,
                                     const blink::WebString& name) {
   if (!render_view_->renderer_preferences_.report_frame_name_changes)
     return;
@@ skipping 384 matching lines @@
     // frame->tree()->top().
     if (frame == render_view_->webview()->mainFrame()) {
       render_view_->Send(new ViewHostMsg_DocumentAvailableInMainFrame(
           render_view_->GetRoutingID()));
     }
   }
 
   // Call back to RenderViewImpl for observers to be notified.
   // TODO(nasko): Remove once we have RenderFrameObserver.
   render_view_->didCreateDocumentElement(frame);
+  // Above code will call RenderViewObservers which can run Javascript that
+  // removes |frame|. If this frame was subframe it will get destroyed
+  // synchronously together with this object.
 }
 
 void RenderFrameImpl::didReceiveTitle(blink::WebFrame* frame,
                                       const blink::WebString& title,
                                       blink::WebTextDirection direction) {
   // TODO(nasko): Investigate wheather implementation should move here.
   render_view_->didReceiveTitle(frame, title, direction);
 }
 
 void RenderFrameImpl::didChangeIcon(blink::WebFrame* frame,
                                     blink::WebIconURL::Type icon_type) {
   // TODO(nasko): Investigate wheather implementation should move here.
   render_view_->didChangeIcon(frame, icon_type);
 }
 
 void RenderFrameImpl::didFinishDocumentLoad(blink::WebFrame* frame) {
   WebDataSource* ds = frame->dataSource();
   DocumentState* document_state = DocumentState::FromDataSource(ds);
   document_state->set_finish_document_load_time(Time::Now());
 
   Send(
       new FrameHostMsg_DidFinishDocumentLoad(routing_id_, frame->identifier()));
 
   // Call back to RenderViewImpl for observers to be notified.
   // TODO(nasko): Remove once we have RenderFrameObserver for this method.
   render_view_->didFinishDocumentLoad(frame);
 
-  // Check whether we have new encoding name.
-  render_view_->UpdateEncoding(frame, frame->view()->pageEncoding().utf8());
+  // Above code will call RenderViewObservers which can run Javascript that
+  // removes |frame|. If this frame was subframe it will get destroyed
+  // synchronously together with this object.
+  if (frame->view()) {
+    // Check whether we have new encoding name.
+    render_view_->UpdateEncoding(frame, frame->view()->pageEncoding().utf8());
+  }
 }
 
 void RenderFrameImpl::didHandleOnloadEvents(blink::WebFrame* frame) {
   // TODO(nasko): Move implementation here. Needed state:
   // * page_id_
   render_view_->didHandleOnloadEvents(frame);
 }
 
 void RenderFrameImpl::didFailLoad(blink::WebFrame* frame,
                                   const blink::WebURLError& error) {
@@ skipping 713 matching lines @@
 
 void RenderFrameImpl::didStartLoading() {
   Send(new FrameHostMsg_DidStartLoading(routing_id_));
 }
 
 void RenderFrameImpl::didStopLoading() {
   Send(new FrameHostMsg_DidStopLoading(routing_id_));
 }
 
 }  // namespace content