Browser Plugin: New Implementation (Browser Side)

content/browser/renderer_host/render_view_host_impl.cc

 // Copyright (c) 2012 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/renderer_host/render_view_host_impl.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 11 matching lines @@
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/cross_site_request_manager.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/host_zoom_map_impl.h"
 #include "content/browser/power_save_blocker.h"
 #include "content/browser/renderer_host/dip_util.h"
 #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"
 #include "content/common/inter_process_time_ticks_converter.h"
 #include "content/common/speech_recognition_messages.h"
 #include "content/common/swapped_out_messages.h"
 #include "content/common/view_messages.h"
 #include "content/port/browser/render_view_host_delegate_view.h"
 #include "content/port/browser/render_widget_host_view_port.h"
 #include "content/public/browser/browser_accessibility_state.h"
@@ skipping 778 matching lines @@
 
 bool RenderViewHostImpl::SuddenTerminationAllowed() const {
   return sudden_termination_allowed_ ||
       GetProcess()->SuddenTerminationAllowed();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // 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.
   // We still want to handle certain ACKs to keep our state consistent.
   if (is_swapped_out_) {
     if (!content::SwappedOutMessages::CanHandleWhileSwappedOut(msg)) {
       // If this is a synchronous message and we decided not to handle it,
       // we must send an error reply, or else the renderer will be stuck
       // and won't respond to future requests.
       if (msg.is_sync()) {
@@ skipping 1080 matching lines @@
   // can cause navigations to be ignored in OnMsgNavigate.
   is_waiting_for_beforeunload_ack_ = false;
   is_waiting_for_unload_ack_ = false;
 }
 
 void RenderViewHostImpl::ClearPowerSaveBlockers() {
   STLDeleteValues(&power_save_blockers_);
 }
 
 }  // namespace content