Ensure that BrowserMessageFilter isn't used to process a sync message on the UI thread.

content/browser/browser_message_filter.cc

 // Copyright (c) 2010 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/browser/browser_message_filter.h"
 
 #include "base/logging.h"
 #include "base/process.h"
 #include "base/process_util.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "content/common/result_codes.h"
+#include "ipc/ipc_sync_message.h"
 
 BrowserMessageFilter::BrowserMessageFilter()
     : channel_(NULL), peer_handle_(base::kNullProcessHandle) {
 }
 
 BrowserMessageFilter::~BrowserMessageFilter() {
   base::CloseProcessHandle(peer_handle_);
 }
 
 void BrowserMessageFilter::OnFilterAdded(IPC::Channel* channel) {
@@ skipping 38 matching lines @@
 void BrowserMessageFilter::OverrideThreadForMessage(const IPC::Message& message,
                                                     BrowserThread::ID* thread) {
 }
 
 bool BrowserMessageFilter::OnMessageReceived(const IPC::Message& message) {
   BrowserThread::ID thread = BrowserThread::IO;
   OverrideThreadForMessage(message, &thread);
   if (thread == BrowserThread::IO)
     return DispatchMessage(message);
 
+  if (thread == BrowserThread::UI &&
+      !MessageCanBeDispatchedOnUI(message, this)) {
+    return true;

[brettw, 2011/03/25 05:33:26]

It seemed weird to me that this doesn't assert, but the "CanBe..." function
does. Either the assert should be moved here, or maybe the "CanBe" function
should be "Verify"

[jam, 2011/03/25 17:08:53]

I don't want the assert here since that code is shared with RVH, and I want to
keep the assert beside the big comment.

prefixing Verify to MessageCanBeDispatchedOnUI seems redundant, I'm not sure how
it helps to see that the function asserts?

+  }
+
   BrowserThread::PostTask(
       thread, FROM_HERE,
       NewRunnableMethod(
           this, &BrowserMessageFilter::DispatchMessage, message));
   return true;
 }
 
 bool BrowserMessageFilter::DispatchMessage(const IPC::Message& message) {
   bool message_was_ok = true;
   bool rv = OnMessageReceived(message, &message_was_ok);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) || rv) <<
       "Must handle messages that were dispatched to another thread!";
   if (!message_was_ok) {
     UserMetrics::RecordAction(UserMetricsAction("BadMessageTerminate_BMF"));
     BadMessageReceived();
   }
 
   return rv;
 }
 
 void BrowserMessageFilter::BadMessageReceived() {
   base::KillProcess(peer_handle(), ResultCodes::KILLED_BAD_MESSAGE, false);
 }
+
+bool BrowserMessageFilter::MessageCanBeDispatchedOnUI(
+    const IPC::Message& message, IPC::Message::Sender* sender) {
+#if defined(OS_WIN)
+  // On Windows there's a potential deadlock with sync messsages going in
+  // a circle from browser -> plugin -> renderer -> browser.
+  // On Linux we can avoid this by avoiding sync messages from browser->plugin.
+  // On Mac we avoid this by not supporting windowed plugins.
+  if (message.is_sync() && !message.is_caller_pumping_messages()) {
+    // NOTE: IF YOU HIT THIS ASSERT, THE SOLUTION IS ALMOST NEVER TO RUN A
+    // NESTED MESSAGE LOOP IN THE RENDERER!!!
+    // That introduces reentrancy which causes hard to track bugs.  You should
+    // find a way to either turn this into an asynchronous message, or one
+    // that can be answered on the IO thread.
+    NOTREACHED() << "Can't send sync messages to UI thread without pumping "
+        "messages in the renderer or else deadlocks can occur if the page "
+        "has windowed plugins! (message type " << message.type() << ")";
+    IPC::Message* reply = IPC::SyncMessage::GenerateReply(&message);
+    reply->set_reply_error();
+    sender->Send(reply);
+    return true;
+  }
+#endif
+  return false;
+}