Fix crash that could happen on shutdown if the ResourceMessageFilter tried to get the IO thread's message loop when it was going away. Do the same acrobatics as in BufferedResourceHandler by going to the UI thread first, and doing refcounting manually.

Also fix an issue I saw by inspection in BufferedResourceHandler, where Release should be called at the end of the function.

BUG=19415
Review URL: http://codereview.chromium.org/171055

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@23563 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 48 insertions, 23 deletions

diff --git a/chrome/browser/renderer_host/buffered_resource_handler.cc b/chrome/browser/renderer_host/buffered_resource_handler.cc
index 2903c89..1494f2a 100644
--- a/chrome/browser/renderer_host/buffered_resource_handler.cc
+++ b/chrome/browser/renderer_host/buffered_resource_handler.cc
@@ -425,14 +425,13 @@ void BufferedResourceHandler::NotifyPluginsLoaded(
 }
 
 void BufferedResourceHandler::OnPluginsLoaded() {
-  Release();
   wait_for_plugins_ = false;
-  if (!request_)
-    return;
-
-  ResourceDispatcherHost::ExtraRequestInfo* info =
-      ResourceDispatcherHost::ExtraInfoForRequest(request_);
-  host_->PauseRequest(info->process_id, info->request_id, false);
-  if (!CompleteResponseStarted(info->request_id, false))
-    host_->CancelRequest(info->process_id, info->request_id, false);
+  if (request_) {
+    ResourceDispatcherHost::ExtraRequestInfo* info =
+        ResourceDispatcherHost::ExtraInfoForRequest(request_);
+    host_->PauseRequest(info->process_id, info->request_id, false);
+    if (!CompleteResponseStarted(info->request_id, false))
+      host_->CancelRequest(info->process_id, info->request_id, false);
+  }
+  Release();
 }
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index f47db7d..c4a476d 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -387,9 +387,8 @@ void ResourceMessageFilter::OnReceiveContextMenuMsg(const IPC::Message& msg) {
 
   // Create a new ViewHostMsg_ContextMenu message.
   const ViewHostMsg_ContextMenu context_menu_message(msg.routing_id(), params);
-  render_widget_helper_->ui_loop()->PostTask(FROM_HERE,
-      new ContextMenuMessageDispatcher(render_process_id_,
-                                       context_menu_message));
+  ui_loop()->PostTask(FROM_HERE, new ContextMenuMessageDispatcher(
+      render_process_id_, context_menu_message));
 }
 
 // Called on the IPC thread:
@@ -542,19 +541,45 @@ void ResourceMessageFilter::OnLoadFont(LOGFONT font) {
 
 void ResourceMessageFilter::OnGetPlugins(bool refresh,
                                          IPC::Message* reply_msg) {
+  // Schedule plugin loading on the file thread.
+  // Note: it's possible that the only reference to this object is the task.  If
+  // If the task executes on the file thread, and before it returns, the task it
+  // posts to the IO thread runs, then this object will get destructed on the
+  // file thread.  We need this object to be destructed on the IO thread, so do
+  // the refcounting manually.
+  AddRef();
   ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &ResourceMessageFilter::OnGetPluginsOnFileThread,
-          refresh, reply_msg));
+      NewRunnableFunction(&ResourceMessageFilter::OnGetPluginsOnFileThread,
+          this, refresh, reply_msg));
 }
 
-void ResourceMessageFilter::OnGetPluginsOnFileThread(bool refresh,
-                                                     IPC::Message* reply_msg) {
+void ResourceMessageFilter::OnGetPluginsOnFileThread(
+    ResourceMessageFilter* filter,
+    bool refresh,
+    IPC::Message* reply_msg) {
   std::vector<WebPluginInfo> plugins;
   NPAPI::PluginList::Singleton()->GetPlugins(refresh, &plugins);
 
   ViewHostMsg_GetPlugins::WriteReplyParams(reply_msg, plugins);
+  // Note, we want to get to the IO thread now, but the file thread outlives it
+  // so we can't post a task to it directly as it might be in the middle of
+  // destruction.  So hop through the main thread, where the destruction of the
+  // IO thread happens and hence no race conditions exist.
+  filter->ui_loop()->PostTask(FROM_HERE,
+      NewRunnableFunction(&ResourceMessageFilter::OnNotifyPluginsLoaded,
+          filter, reply_msg));
+}
+
+void ResourceMessageFilter::OnNotifyPluginsLoaded(ResourceMessageFilter* filter,
+                                                  IPC::Message* reply_msg) {
   ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &ResourceMessageFilter::Send, reply_msg));
+      NewRunnableMethod(filter, &ResourceMessageFilter::OnPluginsLoaded,
+          reply_msg));
+}
+
+void ResourceMessageFilter::OnPluginsLoaded(IPC::Message* reply_msg) {
+  Send(reply_msg);
+  Release();
 }
 
 void ResourceMessageFilter::OnGetPluginPath(const GURL& url,
@@ -621,8 +646,7 @@ void ResourceMessageFilter::OnClipboardWriteObjects(
   Clipboard::DuplicateRemoteHandles(handle(), long_living_objects);
 #endif
 
-  render_widget_helper_->ui_loop()->PostTask(FROM_HERE,
-      new WriteClipboardTask(long_living_objects));
+  ui_loop()->PostTask(FROM_HERE, new WriteClipboardTask(long_living_objects));
 }
 
 #if !defined(OS_LINUX)
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index 40209a9..8df06f3 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -141,7 +141,12 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnGetScreenInfo(gfx::NativeViewId window, IPC::Message* reply);
 #endif
   void OnGetPlugins(bool refresh, IPC::Message* reply_msg);
-  void OnGetPluginsOnFileThread(bool refresh, IPC::Message* reply_msg);
+  static void OnGetPluginsOnFileThread(ResourceMessageFilter* filter,
+                                       bool refresh,
+                                       IPC::Message* reply_msg);
+  static void OnNotifyPluginsLoaded(ResourceMessageFilter* filter,
+                                    IPC::Message* reply_msg);
+  void OnPluginsLoaded(IPC::Message* reply_msg);
   void OnGetPluginPath(const GURL& url,
                        const GURL& policy_url,
                        const std::string& mime_type,
diff --git a/chrome/test/data/reliability/known_crashes.txt b/chrome/test/data/reliability/known_crashes.txt
index 16a5701..1ad95b3 100644
--- a/chrome/test/data/reliability/known_crashes.txt
+++ b/chrome/test/data/reliability/known_crashes.txt
@@ -112,9 +112,6 @@ PREFIX : npapi::pluginstreamurl::didreceivedata___webplugindelegatestub::ondidre
 # 19414
 PREFIX : downloadfilemanager::updatedownload
 
-# 19415
-PREFIX : messageloop::posttask___resourcemessagefilter::ongetpluginsonfilethread___runnablemethod<resourcemessagefilter,void (__thiscall resourcemessagefilter::*)(bool,ipc::message *),tuple2<bool,ipc::message *> >::run___messageloop::runtask___messageloop::dowork___base::messagepumpforui::dorunloop___base::messagepumpwin::run___messageloop::runinternal___messageloop::runhandler___messageloop::run___base::thread::threadmain___`anonymous namespace'::threadfunc
-
 # 19428
 PREFIX : webcore::rendertextcontrol::calcprefwidths___webcore::renderbox::minprefwidth___webcore::renderblock::calcinlineprefwidths