Browser Plugin: New Implementation (Browser Side)

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 7613dcfd42b69bf5517ba666fdf81a44d781f3c3..01c782b8f044c3aff38fcd0341c157cb103b7079 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -29,6 +29,7 @@
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/common/accessibility_messages.h"
+#include "content/common/browser_plugin_messages.h"
 #include "content/common/content_constants_internal.h"
 #include "content/common/desktop_notification_messages.h"
 #include "content/common/drag_messages.h"
@@ -827,7 +828,10 @@ bool RenderViewHostImpl::SuddenTerminationAllowed() const {
 // RenderViewHostImpl, IPC message handlers:
 
 bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
-  if (!BrowserMessageFilter::CheckCanDispatchOnUI(msg, this))
+  // Allow BrowserPluginHostMsg_* sync messages to run on the UI thread.

[Charlie Reis, 2012/09/13 00:51:44]

This doesn't sound safe.  Why is it ok to process sync messages on the UI thread
here, if we've gone out of our way to prevent it otherwise?

[Fady Samuel, 2012/09/13 14:17:18]

I've copied-and-pasted BrowserMessageFilter::CheckCanDispatchOnUI. My
understanding is 1. platform apps will not support windowed plugins (asked in an
email thread for confirmation). 2. Browser Plugin messages will not participate
in this deadlock cycle in the comment below.

John or someone else might be able to validate my two assumptions/claims above.

bool BrowserMessageFilter::CheckCanDispatchOnUI(const IPC::Message& message,
                                                IPC::Sender* sender) {
#if defined(OS_WIN) && !defined(USE_AURA)
  // 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 false;
  }
#endif
  return true;
}

[Charlie Reis, 2012/09/13 16:55:30]

Ok.  Let's put that justification in the comment above so that it's clear why we
think this is safe.

[lazyboy, 2012/09/13 18:52:49]

Done.

+  if (msg.type() != BrowserPluginHostMsg_HandleInputEvent::ID &&
+      msg.type() != BrowserPluginHostMsg_ResizeGuest::ID &&
+      !BrowserMessageFilter::CheckCanDispatchOnUI(msg, this))
     return true;
 
   // Filter out most IPC messages if this renderer is swapped out.