diff options
29 files changed, 398 insertions, 142 deletions
diff --git a/content/ppapi_plugin/ppapi_thread.cc b/content/ppapi_plugin/ppapi_thread.cc index 3277f1b..154826c 100644 --- a/content/ppapi_plugin/ppapi_thread.cc +++ b/content/ppapi_plugin/ppapi_thread.cc @@ -147,6 +147,8 @@ void PpapiThread::Unregister(uint32 plugin_dispatcher_id) { } void PpapiThread::OnMsgLoadPlugin(const FilePath& path) { + SavePluginName(path); + std::string error; base::ScopedNativeLibrary library(base::LoadNativeLibrary(path, &error)); @@ -319,3 +321,8 @@ bool PpapiThread::SetupRendererChannel(base::ProcessHandle host_process_handle, // lifetime of the attached channel. return true; } + +void PpapiThread::SavePluginName(const FilePath& path) { + ppapi::proxy::PluginGlobals::Get()->set_plugin_name( + path.BaseName().AsUTF8Unsafe()); +} diff --git a/content/ppapi_plugin/ppapi_thread.h b/content/ppapi_plugin/ppapi_thread.h index bff66db..0784dd3 100644 --- a/content/ppapi_plugin/ppapi_thread.h +++ b/content/ppapi_plugin/ppapi_thread.h @@ -64,6 +64,9 @@ class PpapiThread : public ChildThread, int renderer_id, IPC::ChannelHandle* handle); + // Sets up the name of the plugin for logging using the given path. + void SavePluginName(const FilePath& path); + // True if running in a broker process rather than a normal plugin process. bool is_broker_; diff --git a/ppapi/proxy/host_dispatcher.cc b/ppapi/proxy/host_dispatcher.cc index efd1f56..fc7686f 100644 --- a/ppapi/proxy/host_dispatcher.cc +++ b/ppapi/proxy/host_dispatcher.cc @@ -14,6 +14,7 @@ #include "ppapi/proxy/interface_list.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/resource_creation_proxy.h" +#include "ppapi/shared_impl/ppapi_globals.h" namespace ppapi { namespace proxy { @@ -174,6 +175,14 @@ bool HostDispatcher::OnMessageReceived(const IPC::Message& msg) { BoolRestorer restorer(&allow_plugin_reentrancy_); allow_plugin_reentrancy_ = false; + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(HostDispatcher, msg) + IPC_MESSAGE_HANDLER(PpapiHostMsg_LogWithSource, OnHostMsgLogWithSource) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + + if (handled) + return true; return Dispatcher::OnMessageReceived(msg); } @@ -215,6 +224,19 @@ void HostDispatcher::OnInvalidMessageReceived() { // received. } +void HostDispatcher::OnHostMsgLogWithSource(PP_Instance instance, + int int_log_level, + const std::string& source, + const std::string& value) { + PP_LogLevel_Dev level = static_cast<PP_LogLevel_Dev>(int_log_level); + if (instance) { + PpapiGlobals::Get()->LogWithSource(instance, level, source, value); + } else { + PpapiGlobals::Get()->BroadcastLogWithSource(pp_module_, level, + source, value); + } +} + // ScopedModuleReference ------------------------------------------------------- ScopedModuleReference::ScopedModuleReference(Dispatcher* dispatcher) { diff --git a/ppapi/proxy/host_dispatcher.h b/ppapi/proxy/host_dispatcher.h index 31de206..1bacdfa 100644 --- a/ppapi/proxy/host_dispatcher.h +++ b/ppapi/proxy/host_dispatcher.h @@ -85,6 +85,11 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { virtual void OnInvalidMessageReceived(); private: + void OnHostMsgLogWithSource(PP_Instance instance, + int int_log_level, + const std::string& source, + const std::string& value); + PP_Module pp_module_; // Maps interface name to whether that interface is supported. If an interface diff --git a/ppapi/proxy/plugin_dispatcher.cc b/ppapi/proxy/plugin_dispatcher.cc index b036978..efe0803 100644 --- a/ppapi/proxy/plugin_dispatcher.cc +++ b/ppapi/proxy/plugin_dispatcher.cc @@ -40,6 +40,9 @@ namespace { typedef std::map<PP_Instance, PluginDispatcher*> InstanceToDispatcherMap; InstanceToDispatcherMap* g_instance_to_dispatcher = NULL; +typedef std::set<PluginDispatcher*> DispatcherSet; +DispatcherSet* g_live_dispatchers = NULL; + } // namespace InstanceData::InstanceData() @@ -60,11 +63,21 @@ PluginDispatcher::PluginDispatcher(base::ProcessHandle remote_process_handle, received_preferences_(false), plugin_dispatcher_id_(0) { SetSerializationRules(new PluginVarSerializationRules); + + if (!g_live_dispatchers) + g_live_dispatchers = new DispatcherSet; + g_live_dispatchers->insert(this); } PluginDispatcher::~PluginDispatcher() { if (plugin_delegate_) plugin_delegate_->Unregister(plugin_dispatcher_id_); + + g_live_dispatchers->erase(this); + if (g_live_dispatchers->empty()) { + delete g_live_dispatchers; + g_live_dispatchers = NULL; + } } // static @@ -88,6 +101,33 @@ const void* PluginDispatcher::GetBrowserInterface(const char* interface_name) { return InterfaceList::GetInstance()->GetInterfaceForPPB(interface_name); } +// static +void PluginDispatcher::LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { + if (!g_live_dispatchers || !g_instance_to_dispatcher) + return; + + if (instance) { + InstanceToDispatcherMap::iterator found = + g_instance_to_dispatcher->find(instance); + if (found != g_instance_to_dispatcher->end()) { + // Send just to this specific dispatcher. + found->second->Send(new PpapiHostMsg_LogWithSource( + instance, static_cast<int>(level), source, value)); + return; + } + } + + // Instance 0 or invalid, send to all dispatchers. + for (DispatcherSet::iterator i = g_live_dispatchers->begin(); + i != g_live_dispatchers->end(); ++i) { + (*i)->Send(new PpapiHostMsg_LogWithSource( + instance, static_cast<int>(level), source, value)); + } +} + const void* PluginDispatcher::GetPluginInterface( const std::string& interface_name) { InterfaceMap::iterator found = plugin_interfaces_.find(interface_name); diff --git a/ppapi/proxy/plugin_dispatcher.h b/ppapi/proxy/plugin_dispatcher.h index 947ab0a..55806b4 100644 --- a/ppapi/proxy/plugin_dispatcher.h +++ b/ppapi/proxy/plugin_dispatcher.h @@ -13,6 +13,7 @@ #include "base/memory/scoped_ptr.h" #include "base/process.h" #include "build/build_config.h" +#include "ppapi/c/dev/ppb_console_dev.h" #include "ppapi/c/pp_rect.h" #include "ppapi/c/pp_instance.h" #include "ppapi/proxy/dispatcher.h" @@ -80,6 +81,14 @@ class PPAPI_PROXY_EXPORT PluginDispatcher : public Dispatcher { // a browser interface. static const void* GetBrowserInterface(const char* interface_name); + // Logs the given log message to the given instance, or, if the instance is + // invalid, to all instances associated with all dispatchers. Used for + // global log messages. + static void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value); + const void* GetPluginInterface(const std::string& interface_name); // You must call this function before anything else. Returns true on success. diff --git a/ppapi/proxy/plugin_globals.cc b/ppapi/proxy/plugin_globals.cc index 360f221..5218dc4 100644 --- a/ppapi/proxy/plugin_globals.cc +++ b/ppapi/proxy/plugin_globals.cc @@ -5,6 +5,7 @@ #include "ppapi/proxy/plugin_globals.h" #include "ppapi/proxy/plugin_dispatcher.h" +#include "ppapi/thunk/enter.h" namespace ppapi { namespace proxy { @@ -66,6 +67,24 @@ base::Lock* PluginGlobals::GetProxyLock() { #endif } +void PluginGlobals::LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { + const std::string& fixed_up_source = source.empty() ? plugin_name_ : source; + PluginDispatcher::LogWithSource(instance, level, fixed_up_source, value); +} + +void PluginGlobals::BroadcastLogWithSource(PP_Module /* module */, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { + // Since we have only one module in a plugin process, broadcast is always + // the same as "send to everybody" which is what the dispatcher implements + // for the "instance = 0" case. + LogWithSource(0, level, source, value); +} + bool PluginGlobals::IsPluginGlobals() const { return true; } diff --git a/ppapi/proxy/plugin_globals.h b/ppapi/proxy/plugin_globals.h index 0b47616..1cf3661 100644 --- a/ppapi/proxy/plugin_globals.h +++ b/ppapi/proxy/plugin_globals.h @@ -41,6 +41,14 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { ApiID id) OVERRIDE; virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE; virtual base::Lock* GetProxyLock() OVERRIDE; + virtual void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; + virtual void BroadcastLogWithSource(PP_Module module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; // Getters for the plugin-specific versions. PluginResourceTracker* plugin_resource_tracker() { @@ -58,6 +66,10 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { plugin_proxy_delegate_ = d; } + // The embedder should call this function when the name of the plugin module + // is known. This will be used for error logging. + void set_plugin_name(const std::string& name) { plugin_name_ = name; } + private: // PpapiGlobals overrides. virtual bool IsPluginGlobals() const OVERRIDE; @@ -70,6 +82,10 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { scoped_refptr<CallbackTracker> callback_tracker_; base::Lock proxy_lock_; + // Name of the plugin used for error logging. This will be empty until + // SetPluginName is called. + std::string plugin_name_; + DISALLOW_COPY_AND_ASSIGN(PluginGlobals); }; diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h index ef5ee81..6947f49 100644 --- a/ppapi/proxy/ppapi_messages.h +++ b/ppapi/proxy/ppapi_messages.h @@ -511,6 +511,13 @@ IPC_MESSAGE_ROUTED2(PpapiMsg_PPPVideoDecoder_NotifyError, IPC_MESSAGE_CONTROL1(PpapiHostMsg_ChannelCreated, IPC::ChannelHandle /* handle */) +// Logs the given message to the console of all instances. +IPC_MESSAGE_CONTROL4(PpapiHostMsg_LogWithSource, + PP_Instance /* instance */, + int /* log_level */, + std::string /* source */, + std::string /* value */) + // PPB_Audio. IPC_SYNC_MESSAGE_ROUTED3_1(PpapiHostMsg_PPBAudio_Create, PP_Instance /* instance_id */, @@ -909,15 +916,6 @@ IPC_SYNC_MESSAGE_ROUTED2_2(PpapiHostMsg_PPBInstance_ExecuteScript, IPC_SYNC_MESSAGE_ROUTED1_1(PpapiHostMsg_PPBInstance_GetDefaultCharSet, PP_Instance /* instance */, ppapi::proxy::SerializedVar /* result */) -IPC_MESSAGE_ROUTED3(PpapiHostMsg_PPBInstance_Log, - PP_Instance /* instance */, - int /* log_level */, - ppapi::proxy::SerializedVar /* value */) -IPC_MESSAGE_ROUTED4(PpapiHostMsg_PPBInstance_LogWithSource, - PP_Instance /* instance */, - int /* log_level */, - ppapi::proxy::SerializedVar /* source */, - ppapi::proxy::SerializedVar /* value */) IPC_SYNC_MESSAGE_ROUTED2_1(PpapiHostMsg_PPBInstance_SetFullscreen, PP_Instance /* instance */, PP_Bool /* fullscreen */, diff --git a/ppapi/proxy/ppb_graphics_2d_proxy.cc b/ppapi/proxy/ppb_graphics_2d_proxy.cc index 0788361..b719a6b 100644 --- a/ppapi/proxy/ppb_graphics_2d_proxy.cc +++ b/ppapi/proxy/ppb_graphics_2d_proxy.cc @@ -91,8 +91,11 @@ void Graphics2D::PaintImageData(PP_Resource image_data, const PP_Rect* src_rect) { Resource* image_object = PpapiGlobals::Get()->GetResourceTracker()->GetResource(image_data); - if (!image_object || pp_instance() != image_object->pp_instance()) + if (!image_object || pp_instance() != image_object->pp_instance()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.PaintImageData: Bad image resource."); return; + } PP_Rect dummy; memset(&dummy, 0, sizeof(PP_Rect)); @@ -113,8 +116,11 @@ void Graphics2D::Scroll(const PP_Rect* clip_rect, void Graphics2D::ReplaceContents(PP_Resource image_data) { Resource* image_object = PpapiGlobals::Get()->GetResourceTracker()->GetResource(image_data); - if (!image_object || pp_instance() != image_object->pp_instance()) + if (!image_object || pp_instance() != image_object->pp_instance()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.PaintImageData: Bad image resource."); return; + } GetDispatcher()->Send(new PpapiHostMsg_PPBGraphics2D_ReplaceContents( kApiID, host_resource(), image_object->host_resource())); diff --git a/ppapi/proxy/ppb_instance_proxy.cc b/ppapi/proxy/ppb_instance_proxy.cc index 1a71966..62335c2 100644 --- a/ppapi/proxy/ppb_instance_proxy.cc +++ b/ppapi/proxy/ppb_instance_proxy.cc @@ -84,10 +84,6 @@ bool PPB_Instance_Proxy::OnMessageReceived(const IPC::Message& msg) { OnHostMsgExecuteScript) IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBInstance_GetDefaultCharSet, OnHostMsgGetDefaultCharSet) - IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBInstance_Log, - OnHostMsgLog) - IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBInstance_LogWithSource, - OnHostMsgLogWithSource) IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBInstance_PostMessage, OnHostMsgPostMessage) IPC_MESSAGE_HANDLER(PpapiHostMsg_PPBInstance_FlashSetFullscreen, @@ -198,24 +194,6 @@ PP_Var PPB_Instance_Proxy::GetDefaultCharSet(PP_Instance instance) { return result.Return(dispatcher); } -void PPB_Instance_Proxy::Log(PP_Instance instance, - int log_level, - PP_Var value) { - dispatcher()->Send(new PpapiHostMsg_PPBInstance_Log( - API_ID_PPB_INSTANCE, instance, static_cast<int>(log_level), - SerializedVarSendInput(dispatcher(), value))); -} - -void PPB_Instance_Proxy::LogWithSource(PP_Instance instance, - int log_level, - PP_Var source, - PP_Var value) { - dispatcher()->Send(new PpapiHostMsg_PPBInstance_LogWithSource( - API_ID_PPB_INSTANCE, instance, static_cast<int>(log_level), - SerializedVarSendInput(dispatcher(), source), - SerializedVarSendInput(dispatcher(), value))); -} - void PPB_Instance_Proxy::NumberOfFindResultsChanged(PP_Instance instance, int32_t total, PP_Bool final_result) { @@ -461,27 +439,6 @@ void PPB_Instance_Proxy::OnHostMsgGetDefaultCharSet( result.Return(dispatcher(), enter.functions()->GetDefaultCharSet(instance)); } -void PPB_Instance_Proxy::OnHostMsgLog(PP_Instance instance, - int log_level, - SerializedVarReceiveInput value) { - EnterInstanceNoLock enter(instance, false); - if (enter.succeeded()) - enter.functions()->Log(instance, log_level, value.Get(dispatcher())); -} - -void PPB_Instance_Proxy::OnHostMsgLogWithSource( - PP_Instance instance, - int log_level, - SerializedVarReceiveInput source, - SerializedVarReceiveInput value) { - EnterInstanceNoLock enter(instance, false); - if (enter.succeeded()) { - enter.functions()->LogWithSource(instance, log_level, - source.Get(dispatcher()), - value.Get(dispatcher())); - } -} - void PPB_Instance_Proxy::OnHostMsgSetFullscreen(PP_Instance instance, PP_Bool fullscreen, PP_Bool* result) { diff --git a/ppapi/proxy/ppb_instance_proxy.h b/ppapi/proxy/ppb_instance_proxy.h index 0ce2562..71107e4 100644 --- a/ppapi/proxy/ppb_instance_proxy.h +++ b/ppapi/proxy/ppb_instance_proxy.h @@ -29,8 +29,7 @@ class SerializedVarOutParam; class SerializedVarReturnValue; class PPB_Instance_Proxy : public InterfaceProxy, - public PPB_Instance_Shared, - public thunk::PPB_Instance_FunctionAPI { + public PPB_Instance_Shared { public: PPB_Instance_Proxy(Dispatcher* dispatcher); virtual ~PPB_Instance_Proxy(); @@ -54,13 +53,6 @@ class PPB_Instance_Proxy : public InterfaceProxy, PP_Var script, PP_Var* exception) OVERRIDE; virtual PP_Var GetDefaultCharSet(PP_Instance instance) OVERRIDE; - virtual void Log(PP_Instance instance, - int log_level, - PP_Var value) OVERRIDE; - virtual void LogWithSource(PP_Instance instance, - int log_level, - PP_Var source, - PP_Var value) OVERRIDE; virtual void NumberOfFindResultsChanged(PP_Instance instance, int32_t total, PP_Bool final_result) OVERRIDE; @@ -122,13 +114,6 @@ class PPB_Instance_Proxy : public InterfaceProxy, SerializedVarReturnValue result); void OnHostMsgGetDefaultCharSet(PP_Instance instance, SerializedVarReturnValue result); - void OnHostMsgLog(PP_Instance instance, - int log_level, - SerializedVarReceiveInput value); - void OnHostMsgLogWithSource(PP_Instance instance, - int log_level, - SerializedVarReceiveInput source, - SerializedVarReceiveInput value); void OnHostMsgSetFullscreen(PP_Instance instance, PP_Bool fullscreen, PP_Bool* result); diff --git a/ppapi/proxy/ppb_url_loader_proxy.cc b/ppapi/proxy/ppb_url_loader_proxy.cc index de74318..c896050 100644 --- a/ppapi/proxy/ppb_url_loader_proxy.cc +++ b/ppapi/proxy/ppb_url_loader_proxy.cc @@ -179,8 +179,13 @@ PPB_URLLoader_API* URLLoader::AsPPB_URLLoader_API() { int32_t URLLoader::Open(PP_Resource request_id, PP_CompletionCallback callback) { EnterResourceNoLock<thunk::PPB_URLRequestInfo_API> enter(request_id, true); - if (enter.failed()) + if (enter.failed()) { + Log(PP_LOGLEVEL_ERROR, "PPB_URLLoader.Open: The URL you're requesting is " + " on a different security origin than your plugin. To request " + " cross-origin resources, see " + " PP_URLREQUESTPROPERTY_ALLOWCROSSORIGINREQUESTS."); return PP_ERROR_BADRESOURCE; + } if (TrackedCallback::IsPending(current_callback_)) return PP_ERROR_INPROGRESS; diff --git a/ppapi/shared_impl/ppapi_globals.h b/ppapi/shared_impl/ppapi_globals.h index 737c3a2..d83e9c6 100644 --- a/ppapi/shared_impl/ppapi_globals.h +++ b/ppapi/shared_impl/ppapi_globals.h @@ -5,8 +5,11 @@ #ifndef PPAPI_SHARED_IMPL_PPAPI_GLOBALS_H_ #define PPAPI_SHARED_IMPL_PPAPI_GLOBALS_H_ +#include <string> + #include "base/basictypes.h" #include "base/threading/thread_local.h" // For testing purposes only. +#include "ppapi/c/dev/ppb_console_dev.h" #include "ppapi/c/pp_instance.h" #include "ppapi/c/pp_module.h" #include "ppapi/shared_impl/api_id.h" @@ -64,6 +67,26 @@ class PPAPI_SHARED_EXPORT PpapiGlobals { PP_Instance instance) = 0; virtual base::Lock* GetProxyLock() = 0; + // Logs the given string to the JS console. If "source" is empty, the name of + // the current module will be used, if it can be determined. + virtual void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) = 0; + + // Like LogWithSource but broadcasts the log to all instances of the given + // module. The module may be 0 to specify that all consoles possibly + // associated with the calling code should be notified. This allows us to + // log errors for things like bad resource IDs where we may not have an + // associated instance. + // + // Note that in the plugin process, the module parameter is ignored since + // there is only one possible one. + virtual void BroadcastLogWithSource(PP_Module module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) = 0; + // Returns the function object corresponding to the given ID, or NULL if // there isn't one. virtual FunctionGroupBase* GetFunctionAPI(PP_Instance inst, ApiID id) = 0; diff --git a/ppapi/shared_impl/ppb_instance_shared.cc b/ppapi/shared_impl/ppb_instance_shared.cc index a2416ce..7e931c3 100644 --- a/ppapi/shared_impl/ppb_instance_shared.cc +++ b/ppapi/shared_impl/ppb_instance_shared.cc @@ -4,14 +4,38 @@ #include "ppapi/shared_impl/ppb_instance_shared.h" +#include <string> + #include "ppapi/c/pp_errors.h" #include "ppapi/c/ppb_input_event.h" +#include "ppapi/shared_impl/ppapi_globals.h" +#include "ppapi/shared_impl/var.h" namespace ppapi { PPB_Instance_Shared::~PPB_Instance_Shared() { } +void PPB_Instance_Shared::Log(PP_Instance instance, + PP_LogLevel_Dev level, + PP_Var value) { + LogWithSource(instance, level, PP_MakeUndefined(), value); +} + +void PPB_Instance_Shared::LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + PP_Var source, + PP_Var value) { + // The source defaults to empty if it's not a string. The PpapiGlobals + // implementation will convert the empty string to the module name if + // possible. + std::string source_str; + if (source.type == PP_VARTYPE_STRING) + source_str = Var::PPVarToLogString(source); + std::string value_str = Var::PPVarToLogString(value); + PpapiGlobals::Get()->LogWithSource(instance, level, source_str, value_str); +} + int32_t PPB_Instance_Shared::ValidateRequestInputEvents( bool is_filtering, uint32_t event_classes) { diff --git a/ppapi/shared_impl/ppb_instance_shared.h b/ppapi/shared_impl/ppb_instance_shared.h index 353a822..2842160 100644 --- a/ppapi/shared_impl/ppb_instance_shared.h +++ b/ppapi/shared_impl/ppb_instance_shared.h @@ -5,15 +5,27 @@ #ifndef PPAPI_SHARED_IMPL_PPB_INSTANCE_SHARED_H_ #define PPAPI_SHARED_IMPL_PPB_INSTANCE_SHARED_H_ +#include "base/compiler_specific.h" #include "ppapi/c/pp_stdint.h" #include "ppapi/shared_impl/ppapi_shared_export.h" +#include "ppapi/thunk/ppb_instance_api.h" namespace ppapi { -class PPAPI_SHARED_EXPORT PPB_Instance_Shared { +class PPAPI_SHARED_EXPORT PPB_Instance_Shared + : NON_EXPORTED_BASE(public thunk::PPB_Instance_FunctionAPI) { public: virtual ~PPB_Instance_Shared(); + // Implementation of some shared PPB_Instance_FunctionAPI functions. + virtual void Log(PP_Instance instance, + PP_LogLevel_Dev log_level, + PP_Var value) OVERRIDE; + virtual void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev log_level, + PP_Var source, + PP_Var value) OVERRIDE; + // Error checks the given resquest to Request[Filtering]InputEvents. Returns // PP_OK if the given classes are all valid, PP_ERROR_NOTSUPPORTED if not. int32_t ValidateRequestInputEvents(bool is_filtering, uint32_t event_classes); diff --git a/ppapi/shared_impl/resource.cc b/ppapi/shared_impl/resource.cc index 4e1f2a8..35add15 100644 --- a/ppapi/shared_impl/resource.cc +++ b/ppapi/shared_impl/resource.cc @@ -45,6 +45,11 @@ void Resource::InstanceWasDeleted() { host_resource_ = HostResource(); } +void Resource::Log(PP_LogLevel_Dev level, const std::string& message) { + PpapiGlobals::Get()->LogWithSource(pp_instance(), level, std::string(), + message); +} + #define DEFINE_TYPE_GETTER(RESOURCE) \ thunk::RESOURCE* Resource::As##RESOURCE() { return NULL; } FOR_ALL_PPAPI_RESOURCE_APIS(DEFINE_TYPE_GETTER) diff --git a/ppapi/shared_impl/resource.h b/ppapi/shared_impl/resource.h index 1c268b0..19775c3 100644 --- a/ppapi/shared_impl/resource.h +++ b/ppapi/shared_impl/resource.h @@ -7,10 +7,13 @@ #include <stddef.h> // For NULL. +#include <string> + #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "ppapi/c/pp_instance.h" #include "ppapi/c/pp_resource.h" +#include "ppapi/c/dev/ppb_console_dev.h" #include "ppapi/shared_impl/host_resource.h" // All resource types should be added here. This implements our hand-rolled @@ -125,6 +128,10 @@ class PPAPI_SHARED_EXPORT Resource : public base::RefCounted<Resource> { // Template-based dynamic casting. See specializations below. template <typename T> T* GetAs() { return NULL; } + protected: + // Logs a message to the console from this resource. + void Log(PP_LogLevel_Dev level, const std::string& message); + private: // See the getters above. PP_Resource pp_resource_; diff --git a/ppapi/shared_impl/test_globals.cc b/ppapi/shared_impl/test_globals.cc index f661998..102dc11 100644 --- a/ppapi/shared_impl/test_globals.cc +++ b/ppapi/shared_impl/test_globals.cc @@ -44,4 +44,16 @@ base::Lock* TestGlobals::GetProxyLock() { return NULL; } +void TestGlobals::LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { +} + +void TestGlobals::BroadcastLogWithSource(PP_Module module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { +} + } // namespace ppapi diff --git a/ppapi/shared_impl/test_globals.h b/ppapi/shared_impl/test_globals.h index 9af6664..ae6f4b2 100644 --- a/ppapi/shared_impl/test_globals.h +++ b/ppapi/shared_impl/test_globals.h @@ -39,6 +39,14 @@ class TestGlobals : public PpapiGlobals { ApiID id) OVERRIDE; virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE; virtual base::Lock* GetProxyLock() OVERRIDE; + virtual void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; + virtual void BroadcastLogWithSource(PP_Module module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; private: ResourceTracker resource_tracker_; diff --git a/ppapi/thunk/enter.cc b/ppapi/thunk/enter.cc index 42f4a0e..43e80fe 100644 --- a/ppapi/thunk/enter.cc +++ b/ppapi/thunk/enter.cc @@ -7,7 +7,9 @@ #include "base/bind.h" #include "base/logging.h" #include "base/message_loop.h" +#include "base/stringprintf.h" #include "ppapi/c/pp_errors.h" +#include "ppapi/shared_impl/ppapi_globals.h" #include "ppapi/thunk/ppb_instance_api.h" #include "ppapi/thunk/resource_creation_api.h" @@ -74,15 +76,32 @@ Resource* EnterBase::GetResource(PP_Resource resource) const { return PpapiGlobals::Get()->GetResourceTracker()->GetResource(resource); } -void EnterBase::SetStateForResourceError(PP_Resource /* pp_resource */, - Resource* /* resource_base */, +void EnterBase::SetStateForResourceError(PP_Resource pp_resource, + Resource* resource_base, void* object, - bool /* report_error */) { + bool report_error) { if (object) return; // Everything worked. retval_ = PP_ERROR_BADRESOURCE; - // TODO(brettw) log the error. + + // We choose to silently ignore the error when the pp_resource is null + // because this is a pretty common case and we don't want to have lots + // of errors in the log. This should be an obvious case to debug. + if (report_error && pp_resource) { + std::string message; + if (resource_base) { + message = base::StringPrintf( + "0x%X is not the correct type for this function.", + pp_resource); + } else { + message = base::StringPrintf( + "0x%X is not a valid resource ID.", + pp_resource); + } + PpapiGlobals::Get()->BroadcastLogWithSource(0, PP_LOGLEVEL_ERROR, + std::string(), message); + } } } // namespace subtle diff --git a/ppapi/thunk/ppb_instance_api.h b/ppapi/thunk/ppb_instance_api.h index 013f61b..6d0339a 100644 --- a/ppapi/thunk/ppb_instance_api.h +++ b/ppapi/thunk/ppb_instance_api.h @@ -5,6 +5,7 @@ #ifndef PPAPI_THUNK_INSTANCE_API_H_ #define PPAPI_THUNK_INSTANCE_API_H_ +#include "ppapi/c/dev/ppb_console_dev.h" #include "ppapi/c/dev/ppb_gamepad_dev.h" #include "ppapi/c/dev/ppb_url_util_dev.h" #include "ppapi/c/pp_completion_callback.h" @@ -47,10 +48,10 @@ class PPB_Instance_FunctionAPI { // Console. virtual void Log(PP_Instance instance, - int log_level, + PP_LogLevel_Dev log_level, PP_Var value) = 0; virtual void LogWithSource(PP_Instance instance, - int log_level, + PP_LogLevel_Dev log_level, PP_Var source, PP_Var value) = 0; diff --git a/webkit/plugins/ppapi/host_globals.cc b/webkit/plugins/ppapi/host_globals.cc index 7705dcc..a8cc9e3 100644 --- a/webkit/plugins/ppapi/host_globals.cc +++ b/webkit/plugins/ppapi/host_globals.cc @@ -8,9 +8,16 @@ #include "base/logging.h" #include "base/rand_util.h" +#include "base/utf_string_conversions.h" #include "ppapi/shared_impl/api_id.h" #include "ppapi/shared_impl/function_group_base.h" #include "ppapi/shared_impl/id_assignment.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebConsoleMessage.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginContainer.h" #include "webkit/plugins/ppapi/plugin_module.h" #include "webkit/plugins/ppapi/ppapi_plugin_instance.h" #include "webkit/plugins/ppapi/ppb_cursor_control_impl.h" @@ -22,10 +29,53 @@ using ppapi::CheckIdType; using ppapi::MakeTypedId; using ppapi::PPIdType; +using WebKit::WebConsoleMessage; +using WebKit::WebString; namespace webkit { namespace ppapi { +namespace { + +typedef std::set<WebKit::WebPluginContainer*> ContainerSet; + +// Adds all WebPluginContainers associated with the given module to the set. +void GetAllContainersForModule(PluginModule* module, + ContainerSet* containers) { + const PluginModule::PluginInstanceSet& instances = + module->GetAllInstances(); + for (PluginModule::PluginInstanceSet::const_iterator i = instances.begin(); + i != instances.end(); ++i) + containers->insert((*i)->container()); +} + +WebConsoleMessage::Level LogLevelToWebLogLevel(PP_LogLevel_Dev level) { + switch (level) { + case PP_LOGLEVEL_TIP: + return WebConsoleMessage::LevelTip; + case PP_LOGLEVEL_LOG: + return WebConsoleMessage::LevelLog; + case PP_LOGLEVEL_WARNING: + return WebConsoleMessage::LevelWarning; + case PP_LOGLEVEL_ERROR: + default: + return WebConsoleMessage::LevelError; + } +} + +WebConsoleMessage MakeLogMessage(PP_LogLevel_Dev level, + const std::string& source, + const std::string& message) { + std::string result = source; + if (!result.empty()) + result.append(": "); + result.append(message); + return WebConsoleMessage(LogLevelToWebLogLevel(level), + WebString(UTF8ToUTF16(result))); +} + +} // namespace + struct HostGlobals::InstanceData { InstanceData() : instance(0) {} @@ -126,6 +176,44 @@ base::Lock* HostGlobals::GetProxyLock() { return NULL; } +void HostGlobals::LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { + PluginInstance* instance_object = HostGlobals::Get()->GetInstance(instance); + if (instance_object) { + instance_object->container()->element().document().frame()-> + addMessageToConsole(MakeLogMessage(level, source, value)); + } else { + BroadcastLogWithSource(0, level, source, value); + } +} + +void HostGlobals::BroadcastLogWithSource(PP_Module pp_module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) { + // Get the unique containers associated with the broadcast. This prevents us + // from sending the same message to the same console when there are two + // instances on the page. + ContainerSet containers; + PluginModule* module = GetModule(pp_module); + if (module) { + GetAllContainersForModule(module, &containers); + } else { + // Unknown module, get containers for all modules. + for (ModuleMap::const_iterator i = module_map_.begin(); + i != module_map_.end(); ++i) { + GetAllContainersForModule(i->second, &containers); + } + } + + WebConsoleMessage message = MakeLogMessage(level, source, value); + for (ContainerSet::iterator i = containers.begin(); + i != containers.end(); ++i) + (*i)->element().document().frame()->addMessageToConsole(message); +} + PP_Module HostGlobals::AddModule(PluginModule* module) { #ifndef NDEBUG // Make sure we're not adding one more than once. diff --git a/webkit/plugins/ppapi/host_globals.h b/webkit/plugins/ppapi/host_globals.h index d0926bc..bbdf04d 100644 --- a/webkit/plugins/ppapi/host_globals.h +++ b/webkit/plugins/ppapi/host_globals.h @@ -43,6 +43,14 @@ class HostGlobals : public ::ppapi::PpapiGlobals { ::ppapi::ApiID id) OVERRIDE; virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE; virtual base::Lock* GetProxyLock() OVERRIDE; + virtual void LogWithSource(PP_Instance instance, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; + virtual void BroadcastLogWithSource(PP_Module module, + PP_LogLevel_Dev level, + const std::string& source, + const std::string& value) OVERRIDE; HostVarTracker* host_var_tracker() { return &host_var_tracker_; diff --git a/webkit/plugins/ppapi/plugin_module.h b/webkit/plugins/ppapi/plugin_module.h index 928f8ac..301f542 100644 --- a/webkit/plugins/ppapi/plugin_module.h +++ b/webkit/plugins/ppapi/plugin_module.h @@ -60,6 +60,8 @@ class WEBKIT_PLUGINS_EXPORT PluginModule : PPP_ShutdownModuleFunc shutdown_module; // Optional, may be NULL. }; + typedef std::set<PluginInstance*> PluginInstanceSet; + // You must call one of the Init functions after the constructor to create a // module of the type you desire. // @@ -106,6 +108,8 @@ class WEBKIT_PLUGINS_EXPORT PluginModule : // but the delegate lives only on the plugin instance so we need one of them. PluginInstance* GetSomeInstance() const; + const PluginInstanceSet& GetAllInstances() const { return instances_; } + // Calls the plugin's GetInterface and returns the given interface pointer, // which could be NULL. const void* GetPluginInterface(const char* name) const; @@ -188,7 +192,6 @@ class WEBKIT_PLUGINS_EXPORT PluginModule : // Non-owning pointers to all instances associated with this module. When // there are no more instances, this object should be deleted. - typedef std::set<PluginInstance*> PluginInstanceSet; PluginInstanceSet instances_; PP_Bool (*reserve_instance_id_)(PP_Module, PP_Instance); diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.cc b/webkit/plugins/ppapi/ppapi_plugin_instance.cc index 9e1717b..7622a63 100644 --- a/webkit/plugins/ppapi/ppapi_plugin_instance.cc +++ b/webkit/plugins/ppapi/ppapi_plugin_instance.cc @@ -12,7 +12,6 @@ #include "base/stringprintf.h" #include "base/utf_offset_string_conversions.h" #include "base/utf_string_conversions.h" -#include "ppapi/c/dev/ppb_console_dev.h" #include "ppapi/c/dev/ppb_find_dev.h" #include "ppapi/c/dev/ppb_gamepad_dev.h" #include "ppapi/c/dev/ppb_zoom_dev.h" @@ -41,7 +40,6 @@ #include "third_party/skia/include/core/SkRect.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebCompositionUnderline.h" -#include "third_party/WebKit/Source/WebKit/chromium/public/WebConsoleMessage.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebCursorInfo.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h" @@ -110,7 +108,6 @@ using ppapi::Var; using ppapi::ViewData; using WebKit::WebBindings; using WebKit::WebCanvas; -using WebKit::WebConsoleMessage; using WebKit::WebCursorInfo; using WebKit::WebDocument; using WebKit::WebElement; @@ -1852,49 +1849,6 @@ PP_Var PluginInstance::GetDefaultCharSet(PP_Instance instance) { return StringVar::StringToPPVar(encoding); } -void PluginInstance::Log(PP_Instance instance, - int log_level, - PP_Var value) { - // TODO(brettw) get the plugin name and use it as the source. - LogWithSource(instance, log_level, PP_MakeUndefined(), value); -} - -void PluginInstance::LogWithSource(PP_Instance instance, - int log_level, - PP_Var source, - PP_Var value) { - // Convert the log level, defaulting to error. - WebConsoleMessage::Level web_level; - switch (log_level) { - case PP_LOGLEVEL_TIP: - web_level = WebConsoleMessage::LevelTip; - break; - case PP_LOGLEVEL_LOG: - web_level = WebConsoleMessage::LevelLog; - break; - case PP_LOGLEVEL_WARNING: - web_level = WebConsoleMessage::LevelWarning; - break; - case PP_LOGLEVEL_ERROR: - default: - web_level = WebConsoleMessage::LevelError; - break; - } - - // Format is the "<source>: <value>". The source defaults to the module name - // if the source isn't a string or is empty. - std::string message; - if (source.type == PP_VARTYPE_STRING) - message = Var::PPVarToLogString(source); - if (message.empty()) - message = module()->name(); - message.append(": "); - message.append(Var::PPVarToLogString(value)); - - container()->element().document().frame()->addMessageToConsole( - WebConsoleMessage(web_level, WebString(UTF8ToUTF16(message)))); -} - void PluginInstance::NumberOfFindResultsChanged(PP_Instance instance, int32_t total, PP_Bool final_result) { diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.h b/webkit/plugins/ppapi/ppapi_plugin_instance.h index c8a6332..1287111 100644 --- a/webkit/plugins/ppapi/ppapi_plugin_instance.h +++ b/webkit/plugins/ppapi/ppapi_plugin_instance.h @@ -34,7 +34,6 @@ #include "ppapi/shared_impl/function_group_base.h" #include "ppapi/shared_impl/ppb_instance_shared.h" #include "ppapi/shared_impl/ppb_view_shared.h" -#include "ppapi/thunk/ppb_instance_api.h" #include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebCanvas.h" #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h" @@ -85,7 +84,6 @@ class PPB_URLRequestInfo_Impl; class WEBKIT_PLUGINS_EXPORT PluginInstance : public base::RefCounted<PluginInstance>, public ::ppapi::FunctionGroupBase, - NON_EXPORTED_BASE(public ::ppapi::thunk::PPB_Instance_FunctionAPI), public ::ppapi::PPB_Instance_Shared { public: // Create and return a PluginInstance object which supports the @@ -332,13 +330,6 @@ class WEBKIT_PLUGINS_EXPORT PluginInstance : PP_Var script, PP_Var* exception) OVERRIDE; virtual PP_Var GetDefaultCharSet(PP_Instance instance) OVERRIDE; - virtual void Log(PP_Instance instance, - int log_level, - PP_Var value) OVERRIDE; - virtual void LogWithSource(PP_Instance instance, - int log_level, - PP_Var source, - PP_Var value) OVERRIDE; virtual void NumberOfFindResultsChanged(PP_Instance instance, int32_t total, PP_Bool final_result) OVERRIDE; diff --git a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc index ddfea5f..f76f622 100644 --- a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc +++ b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc @@ -223,8 +223,11 @@ void PPB_Graphics2D_Impl::PaintImageData(PP_Resource image_data, return; EnterResourceNoLock<PPB_ImageData_API> enter(image_data, true); - if (enter.failed()) + if (enter.failed()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.PaintImageData: Bad image resource."); return; + } PPB_ImageData_Impl* image_resource = static_cast<PPB_ImageData_Impl*>(enter.object()); @@ -232,8 +235,11 @@ void PPB_Graphics2D_Impl::PaintImageData(PP_Resource image_data, operation.paint_image = image_resource; if (!ValidateAndConvertRect(src_rect, image_resource->width(), image_resource->height(), - &operation.paint_src_rect)) + &operation.paint_src_rect)) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.PaintImageData: Rectangle is outside bounds."); return; + } // Validate the bitmap position using the previously-validated rect, there // should be no painted area outside of the image. @@ -259,16 +265,22 @@ void PPB_Graphics2D_Impl::Scroll(const PP_Rect* clip_rect, if (!ValidateAndConvertRect(clip_rect, image_data_->width(), image_data_->height(), - &operation.scroll_clip_rect)) + &operation.scroll_clip_rect)) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.Scroll: Rectangle is outside bounds."); return; + } // If we're being asked to scroll by more than the clip rect size, just // ignore this scroll command and say it worked. int32 dx = amount->x; int32 dy = amount->y; if (dx <= -image_data_->width() || dx >= image_data_->width() || - dy <= -image_data_->height() || dy >= image_data_->height()) + dy <= -image_data_->height() || dy >= image_data_->height()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.Scroll: Scroll amount is larger than image size."); return; + } operation.scroll_dx = dx; operation.scroll_dy = dy; @@ -284,12 +296,19 @@ void PPB_Graphics2D_Impl::ReplaceContents(PP_Resource image_data) { static_cast<PPB_ImageData_Impl*>(enter.object()); if (!PPB_ImageData_Impl::IsImageDataFormatSupported( - image_resource->format())) + image_resource->format())) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.ReplaceContents: Image data format is not supported."); return; + } if (image_resource->width() != image_data_->width() || - image_resource->height() != image_data_->height()) + image_resource->height() != image_data_->height()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_Graphics2D.ReplaceContents: Image size doesn't match " + "Graphics2D size."); return; + } QueuedOperation operation(QueuedOperation::REPLACE); operation.replace_image = image_resource; diff --git a/webkit/plugins/ppapi/ppb_url_loader_impl.cc b/webkit/plugins/ppapi/ppb_url_loader_impl.cc index 0e56501..eccdfe5 100644 --- a/webkit/plugins/ppapi/ppb_url_loader_impl.cc +++ b/webkit/plugins/ppapi/ppb_url_loader_impl.cc @@ -103,8 +103,13 @@ int32_t PPB_URLLoader_Impl::Open(PP_Resource request_id, return PP_ERROR_INPROGRESS; EnterResourceNoLock<PPB_URLRequestInfo_API> enter_request(request_id, true); - if (enter_request.failed()) + if (enter_request.failed()) { + Log(PP_LOGLEVEL_ERROR, + "PPB_URLLoader.Open: invalid request resource ID. (Hint to C++ wrapper" + " users: use the ResourceRequest constructor that takes an instance or" + " else the request will be null.)"); return PP_ERROR_BADARGUMENT; + } PPB_URLRequestInfo_Impl* request = static_cast<PPB_URLRequestInfo_Impl*>( enter_request.object()); @@ -112,8 +117,13 @@ int32_t PPB_URLLoader_Impl::Open(PP_Resource request_id, if (rv != PP_OK) return rv; - if (request->RequiresUniversalAccess() && !has_universal_access_) + if (request->RequiresUniversalAccess() && !has_universal_access_) { + Log(PP_LOGLEVEL_ERROR, "PPB_URLLoader.Open: The URL you're requesting is " + " on a different security origin than your plugin. To request " + " cross-origin resources, see " + " PP_URLREQUESTPROPERTY_ALLOWCROSSORIGINREQUESTS."); return PP_ERROR_NOACCESS; + } if (loader_.get()) return PP_ERROR_INPROGRESS; |