From 8275d865da70a68992560db2a5d65bf1b9fa8f56 Mon Sep 17 00:00:00 2001 From: "japhet@chromium.org" Date: Wed, 18 Nov 2009 22:29:33 +0000 Subject: If an NP_* function is called on an out of process plugin, save enough info to send an NPN_SetException back to the correct renderer if necessary. BUG=26764 TEST=none Review URL: http://codereview.chromium.org/375005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32419 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/common/plugin_messages_internal.h | 6 +++--- chrome/plugin/npobject_proxy.cc | 15 -------------- chrome/plugin/npobject_proxy.h | 3 --- chrome/plugin/npobject_stub.cc | 10 --------- chrome/plugin/npobject_stub.h | 1 - chrome/plugin/npobject_util.cc | 13 +++++++++--- chrome/plugin/plugin_channel_base.cc | 14 ++++++++++++- chrome/plugin/plugin_channel_base.h | 4 ++++ chrome/renderer/plugin_channel_host.cc | 13 ++++++++++++ chrome/renderer/plugin_channel_host.h | 3 +++ chrome/test/data/npapi/npobject_set_exception.html | 24 ++++++++++++++++++++++ chrome/test/ui/npapi_uitest.cc | 12 +++++++++++ 12 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 chrome/test/data/npapi/npobject_set_exception.html diff --git a/chrome/common/plugin_messages_internal.h b/chrome/common/plugin_messages_internal.h index f9a5bbf..84f9cd3 100644 --- a/chrome/common/plugin_messages_internal.h +++ b/chrome/common/plugin_messages_internal.h @@ -355,6 +355,9 @@ IPC_BEGIN_MESSAGES(PluginHost) int /* resource_id */, bool /* defer */) + IPC_SYNC_MESSAGE_CONTROL1_0(PluginHostMsg_SetException, + std::string /* message */) + IPC_END_MESSAGES(PluginHost) //----------------------------------------------------------------------------- @@ -410,7 +413,4 @@ IPC_BEGIN_MESSAGES(NPObject) NPVariant_Param /* result_param */, bool /* result */) - IPC_SYNC_MESSAGE_ROUTED1_0(NPObjectMsg_SetException, - std::string /* message */) - IPC_END_MESSAGES(NPObject) diff --git a/chrome/plugin/npobject_proxy.cc b/chrome/plugin/npobject_proxy.cc index 01cc4c7..a55442b 100644 --- a/chrome/plugin/npobject_proxy.cc +++ b/chrome/plugin/npobject_proxy.cc @@ -484,18 +484,3 @@ bool NPObjectProxy::NPNEvaluate(NPP npp, result_param, channel.get(), result_var, containing_window, page_url); return true; } - -void NPObjectProxy::NPNSetException(NPObject *obj, - const NPUTF8 *message) { - NPObjectProxy* proxy = GetProxy(obj); - if (!proxy) { - return; - } - - NPVariant_Param result_param; - std::string message_str(message); - - proxy->Send(new NPObjectMsg_SetException(proxy->route_id(), message_str)); - // Send may delete proxy. - proxy = NULL; -} diff --git a/chrome/plugin/npobject_proxy.h b/chrome/plugin/npobject_proxy.h index 2b8966d..c5f3e597 100644 --- a/chrome/plugin/npobject_proxy.h +++ b/chrome/plugin/npobject_proxy.h @@ -81,9 +81,6 @@ class NPObjectProxy : public IPC::Channel::Listener, NPString *script, NPVariant *result); - static void NPNSetException(NPObject *obj, - const NPUTF8 *message); - static bool NPInvokePrivate(NPP npp, NPObject *obj, bool is_default, diff --git a/chrome/plugin/npobject_stub.cc b/chrome/plugin/npobject_stub.cc index 4fe7e77..a23f2dc 100644 --- a/chrome/plugin/npobject_stub.cc +++ b/chrome/plugin/npobject_stub.cc @@ -81,7 +81,6 @@ void NPObjectStub::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(NPObjectMsg_Enumeration, OnEnumeration); IPC_MESSAGE_HANDLER_DELAY_REPLY(NPObjectMsg_Construct, OnConstruct); IPC_MESSAGE_HANDLER_DELAY_REPLY(NPObjectMsg_Evaluate, OnEvaluate); - IPC_MESSAGE_HANDLER(NPObjectMsg_SetException, OnSetException); IPC_MESSAGE_UNHANDLED_ERROR() IPC_END_MESSAGE_MAP() } @@ -372,12 +371,3 @@ void NPObjectStub::OnEvaluate(const std::string& script, NPObjectMsg_Evaluate::WriteReplyParams(reply_msg, result_param, return_value); local_channel->Send(reply_msg); } - -void NPObjectStub::OnSetException(const std::string& message) { - if (IsPluginProcess()) { - NOTREACHED() << "Should only be called on NPObjects in the renderer"; - return; - } - - WebBindings::setException(npobject_, message.c_str()); -} diff --git a/chrome/plugin/npobject_stub.h b/chrome/plugin/npobject_stub.h index fbf54c2..a2e2022 100644 --- a/chrome/plugin/npobject_stub.h +++ b/chrome/plugin/npobject_stub.h @@ -73,7 +73,6 @@ class NPObjectStub : public IPC::Channel::Listener, IPC::Message* reply_msg); void OnEvaluate(const std::string& script, bool popups_allowed, IPC::Message* reply_msg); - void OnSetException(const std::string& message); private: NPObject* npobject_; diff --git a/chrome/plugin/npobject_util.cc b/chrome/plugin/npobject_util.cc index 7594225..229a984 100644 --- a/chrome/plugin/npobject_util.cc +++ b/chrome/plugin/npobject_util.cc @@ -84,9 +84,16 @@ static bool NPN_EvaluatePatch(NPP npp, } -static void NPN_SetExceptionPatch(NPObject *obj, - const NPUTF8 *message) { - return NPObjectProxy::NPNSetException(obj, message); +static void NPN_SetExceptionPatch(NPObject *obj, const NPUTF8 *message) { + std::string message_str(message); + if (IsPluginProcess()) { + PluginChannelBase* renderer_channel = + PluginChannelBase::GetCurrentChannel(); + if (renderer_channel) + renderer_channel->Send(new PluginHostMsg_SetException(message_str)); + } else { + WebBindings::setException(obj, message_str.c_str()); + } } static bool NPN_EnumeratePatch(NPP npp, NPObject *obj, diff --git a/chrome/plugin/plugin_channel_base.cc b/chrome/plugin/plugin_channel_base.cc index f4bcac9..7da4560 100644 --- a/chrome/plugin/plugin_channel_base.cc +++ b/chrome/plugin/plugin_channel_base.cc @@ -4,7 +4,10 @@ #include "chrome/plugin/plugin_channel_base.h" +#include + #include "base/hash_tables.h" +#include "base/lazy_instance.h" #include "chrome/common/child_process.h" #include "ipc/ipc_sync_message.h" @@ -17,6 +20,8 @@ typedef base::hash_map > static PluginChannelMap g_plugin_channels_; +static base::LazyInstance > > + lazy_plugin_channel_stack_(base::LINKER_INITIALIZED); PluginChannelBase* PluginChannelBase::GetChannel( const std::string& channel_name, IPC::Channel::Mode mode, @@ -67,6 +72,10 @@ PluginChannelBase::PluginChannelBase() PluginChannelBase::~PluginChannelBase() { } +PluginChannelBase* PluginChannelBase::GetCurrentChannel() { + return lazy_plugin_channel_stack_.Pointer()->top(); +} + void PluginChannelBase::CleanupChannels() { // Make a copy of the references as we can't iterate the map since items will // be removed from it as we clean them up. @@ -115,7 +124,8 @@ int PluginChannelBase::Count() { void PluginChannelBase::OnMessageReceived(const IPC::Message& message) { // This call might cause us to be deleted, so keep an extra reference to // ourself so that we can send the reply and decrement back in_dispatch_. - scoped_refptr me(this); + lazy_plugin_channel_stack_.Pointer()->push( + scoped_refptr(this)); if (message.is_sync()) in_sync_dispatch_++; @@ -133,6 +143,8 @@ void PluginChannelBase::OnMessageReceived(const IPC::Message& message) { } if (message.is_sync()) in_sync_dispatch_--; + + lazy_plugin_channel_stack_.Pointer()->pop(); } void PluginChannelBase::OnChannelConnected(int32 peer_pid) { diff --git a/chrome/plugin/plugin_channel_base.h b/chrome/plugin/plugin_channel_base.h index 38bb87b..f4aab85 100644 --- a/chrome/plugin/plugin_channel_base.h +++ b/chrome/plugin/plugin_channel_base.h @@ -49,6 +49,10 @@ class PluginChannelBase : public IPC::Channel::Listener, return channel_valid_; } + // Returns the most recent PluginChannelBase to have received a message + // in this process. + static PluginChannelBase* GetCurrentChannel(); + static void CleanupChannels(); protected: diff --git a/chrome/renderer/plugin_channel_host.cc b/chrome/renderer/plugin_channel_host.cc index e010c56..aa409ce 100644 --- a/chrome/renderer/plugin_channel_host.cc +++ b/chrome/renderer/plugin_channel_host.cc @@ -6,6 +6,8 @@ #include "chrome/common/plugin_messages.h" +#include "third_party/WebKit/WebKit/chromium/public/WebBindings.h" + // A simple MessageFilter that will ignore all messages and respond to sync // messages with an error when is_listening_ is false. class IsListeningFilter : public IPC::ChannelProxy::MessageFilter { @@ -107,6 +109,17 @@ void PluginChannelHost::RemoveRoute(int route_id) { PluginChannelBase::RemoveRoute(route_id); } +void PluginChannelHost::OnControlMessageReceived(const IPC::Message& message) { + IPC_BEGIN_MESSAGE_MAP(PluginChannelHost, message) + IPC_MESSAGE_HANDLER(PluginHostMsg_SetException, OnSetException) + IPC_MESSAGE_UNHANDLED_ERROR() + IPC_END_MESSAGE_MAP() +} + +void PluginChannelHost::OnSetException(const std::string& message) { + WebKit::WebBindings::setException(NULL, message.c_str()); +} + void PluginChannelHost::OnChannelError() { PluginChannelBase::OnChannelError(); diff --git a/chrome/renderer/plugin_channel_host.h b/chrome/renderer/plugin_channel_host.h index 53289b9..18989d4 100644 --- a/chrome/renderer/plugin_channel_host.h +++ b/chrome/renderer/plugin_channel_host.h @@ -42,6 +42,9 @@ class PluginChannelHost : public PluginChannelBase { static PluginChannelBase* ClassFactory() { return new PluginChannelHost(); } + void OnControlMessageReceived(const IPC::Message& message); + void OnSetException(const std::string& message); + // Keep track of all the registered WebPluginDelegeProxies to // inform about OnChannelError typedef base::hash_map ProxyMap; diff --git a/chrome/test/data/npapi/npobject_set_exception.html b/chrome/test/data/npapi/npobject_set_exception.html new file mode 100644 index 0000000..4e78e69e --- /dev/null +++ b/chrome/test/data/npapi/npobject_set_exception.html @@ -0,0 +1,24 @@ + + +Set Exception Test + + + + + +

Test that if NPN_SetException is called by an out of process plugin, the + exception is sent to the proper renderer.

+ +
+ +
+ + + \ No newline at end of file diff --git a/chrome/test/ui/npapi_uitest.cc b/chrome/test/ui/npapi_uitest.cc index e68bf7f..fb43930 100644 --- a/chrome/test/ui/npapi_uitest.cc +++ b/chrome/test/ui/npapi_uitest.cc @@ -393,3 +393,15 @@ TEST_F(NPAPITester, NPObjectReleasedOnDestruction) { scoped_refptr tab_proxy(window_proxy->GetTab(0)); tab_proxy->Close(true); } + +// Test that a dialog is properly created when a plugin throws an +// exception. Should be run for in and out of process plugins, but +// the more interesting case is out of process, where we must route +// the exception to the correct renderer. +TEST_F(NPAPITester, NPObjectSetException) { + GURL url = GetTestUrl(L"npapi", L"npobject_set_exception.html"); + NavigateToURL(url); + WaitForFinish("npobject_set_exception", "1", url, + kTestCompleteCookie, kTestCompleteSuccess, + kShortWaitTimeout); +} -- cgit v1.1