From 3fe4ceee750b2cd130bd402de3d371d8518c3eba Mon Sep 17 00:00:00 2001 From: dmichael Date: Thu, 25 Sep 2014 15:26:33 -0700 Subject: [PATCH] PPAPI: Never re-enter JavaScript for PostMessage. Blocking renderer->plugin messages can be interrupted by any message from the plugin->renderer (even async ones). So while handline a blocking message, such as HandleInputEvent or HandleBlockingMessage, it's currently possible to re-enter JavaScript. This patch makes that impossible by queueing up Plugin->Renderer messages sent via PPB_Messaging::PostMessage while any renderer->plugin sync message is on the stack. BUG=384528 Committed: https://crrev.com/f73075c99b5ba30e8d62dc5f13fdfb210d0fc506 Cr-Commit-Position: refs/heads/master@{#296311} Review URL: https://codereview.chromium.org/589213003 Cr-Commit-Position: refs/heads/master@{#296807} --- content/renderer/pepper/host_dispatcher_wrapper.cc | 11 ++- content/renderer/pepper/host_dispatcher_wrapper.h | 2 +- content/renderer/pepper/message_channel.cc | 85 +++++++++++++++++----- content/renderer/pepper/message_channel.h | 43 +++++++++-- .../renderer/pepper/pepper_hung_plugin_filter.h | 5 +- ppapi/proxy/dispatcher.cc | 5 +- ppapi/proxy/dispatcher.h | 5 +- ppapi/proxy/host_dispatcher.cc | 20 +++-- ppapi/proxy/host_dispatcher.h | 17 +++-- ppapi/proxy/ppapi_proxy_test.cc | 13 +--- ppapi/proxy/ppapi_proxy_test.h | 4 - ppapi/tests/test_message_handler.cc | 7 +- 12 files changed, 147 insertions(+), 70 deletions(-) diff --git a/content/renderer/pepper/host_dispatcher_wrapper.cc b/content/renderer/pepper/host_dispatcher_wrapper.cc index 606d2815c419..dd69789961e7 100644 --- a/content/renderer/pepper/host_dispatcher_wrapper.cc +++ b/content/renderer/pepper/host_dispatcher_wrapper.cc @@ -32,7 +32,7 @@ HostDispatcherWrapper::~HostDispatcherWrapper() {} bool HostDispatcherWrapper::Init(const IPC::ChannelHandle& channel_handle, PP_GetInterface_Func local_get_interface, const ppapi::Preferences& preferences, - PepperHungPluginFilter* filter) { + scoped_refptr filter) { if (channel_handle.name.empty()) return false; @@ -44,7 +44,11 @@ bool HostDispatcherWrapper::Init(const IPC::ChannelHandle& channel_handle, dispatcher_delegate_.reset(new PepperProxyChannelDelegateImpl); dispatcher_.reset(new ppapi::proxy::HostDispatcher( - module_->pp_module(), local_get_interface, filter, permissions_)); + module_->pp_module(), local_get_interface, permissions_)); + // The HungPluginFilter needs to know when we are blocked on a sync message + // to the plugin. Note the filter outlives the dispatcher, so there is no + // need to remove it as an observer. + dispatcher_->AddSyncMessageStatusObserver(filter.get()); if (!dispatcher_->InitHostWithChannel(dispatcher_delegate_.get(), peer_pid_, @@ -55,6 +59,9 @@ bool HostDispatcherWrapper::Init(const IPC::ChannelHandle& channel_handle, dispatcher_delegate_.reset(); return false; } + // HungPluginFilter needs to listen for some messages on the IO thread. + dispatcher_->AddIOThreadMessageFilter(filter); + dispatcher_->channel()->SetRestrictDispatchChannelGroup( kRendererRestrictDispatchGroup_Pepper); return true; diff --git a/content/renderer/pepper/host_dispatcher_wrapper.h b/content/renderer/pepper/host_dispatcher_wrapper.h index e5349178ed2a..c3b58fb87de6 100644 --- a/content/renderer/pepper/host_dispatcher_wrapper.h +++ b/content/renderer/pepper/host_dispatcher_wrapper.h @@ -34,7 +34,7 @@ class HostDispatcherWrapper { bool Init(const IPC::ChannelHandle& channel_handle, PP_GetInterface_Func local_get_interface, const ppapi::Preferences& preferences, - PepperHungPluginFilter* filter); + scoped_refptr filter); // Implements GetInterface for the proxied plugin. const void* GetProxiedInterface(const char* name); diff --git a/content/renderer/pepper/message_channel.cc b/content/renderer/pepper/message_channel.cc index d5bbef9f0c2a..95be20f9b02f 100644 --- a/content/renderer/pepper/message_channel.cc +++ b/content/renderer/pepper/message_channel.cc @@ -104,12 +104,18 @@ MessageChannel* MessageChannel::Create(PepperPluginInstanceImpl* instance, } MessageChannel::~MessageChannel() { + // Note it's unlikely but possible that we outlive the instance, and so we + // might have already unregistered in InstanceDeleted. That's OK, because + // removing an observer that's already removed is a no-op. + UnregisterSyncMessageStatusObserver(); + passthrough_object_.Reset(); if (instance_) instance_->MessageChannelDestroyed(); } void MessageChannel::InstanceDeleted() { + UnregisterSyncMessageStatusObserver(); instance_ = NULL; } @@ -137,27 +143,36 @@ void MessageChannel::PostMessageToJavaScript(PP_Var message_data) { WebSerializedScriptValue serialized_val = WebSerializedScriptValue::serialize(v8_val); - if (early_message_queue_state_ != SEND_DIRECTLY) { + if (js_message_queue_state_ != SEND_DIRECTLY) { // We can't just PostTask here; the messages would arrive out of // order. Instead, we queue them up until we're ready to post // them. - early_message_queue_.push_back(serialized_val); + js_message_queue_.push_back(serialized_val); } else { // The proxy sent an asynchronous message, so the plugin is already // unblocked. Therefore, there's no need to PostTask. - DCHECK(early_message_queue_.empty()); + DCHECK(js_message_queue_.empty()); PostMessageToJavaScriptImpl(serialized_val); } } void MessageChannel::Start() { - // We PostTask here instead of draining the message queue directly - // since we haven't finished initializing the PepperWebPluginImpl yet, so - // the plugin isn't available in the DOM. - base::MessageLoop::current()->PostTask( - FROM_HERE, - base::Bind(&MessageChannel::DrainEarlyMessageQueue, - weak_ptr_factory_.GetWeakPtr())); + DCHECK_EQ(WAITING_TO_START, js_message_queue_state_); + DCHECK_EQ(WAITING_TO_START, plugin_message_queue_state_); + + ppapi::proxy::HostDispatcher* dispatcher = + ppapi::proxy::HostDispatcher::GetForInstance(instance_->pp_instance()); + // The dispatcher is NULL for in-process. + if (dispatcher) + dispatcher->AddSyncMessageStatusObserver(this); + + // We can't drain the JS message queue directly since we haven't finished + // initializing the PepperWebPluginImpl yet, so the plugin isn't available in + // the DOM. + DrainJSMessageQueueSoon(); + + plugin_message_queue_state_ = SEND_DIRECTLY; + DrainCompletedPluginMessages(); } void MessageChannel::SetPassthroughObject(v8::Handle passthrough) { @@ -176,7 +191,9 @@ void MessageChannel::SetReadOnlyProperty(PP_Var key, PP_Var value) { MessageChannel::MessageChannel(PepperPluginInstanceImpl* instance) : gin::NamedPropertyInterceptor(instance->GetIsolate(), this), instance_(instance), - early_message_queue_state_(QUEUE_MESSAGES), + js_message_queue_state_(WAITING_TO_START), + blocking_message_depth_(0), + plugin_message_queue_state_(WAITING_TO_START), weak_ptr_factory_(this) { } @@ -186,6 +203,18 @@ gin::ObjectTemplateBuilder MessageChannel::GetObjectTemplateBuilder( .AddNamedPropertyInterceptor(); } +void MessageChannel::BeginBlockOnSyncMessage() { + js_message_queue_state_ = QUEUE_MESSAGES; + ++blocking_message_depth_; +} + +void MessageChannel::EndBlockOnSyncMessage() { + DCHECK_GT(blocking_message_depth_, 0); + --blocking_message_depth_; + if (!blocking_message_depth_) + DrainJSMessageQueueSoon(); +} + v8::Local MessageChannel::GetNamedProperty( v8::Isolate* isolate, const std::string& identifier) { @@ -286,7 +315,7 @@ void MessageChannel::PostBlockingMessageToNative(gin::Arguments* args) { NOTREACHED(); } - if (early_message_queue_state_ == QUEUE_MESSAGES) { + if (plugin_message_queue_state_ == WAITING_TO_START) { try_catch.ThrowException( "Attempted to call a synchronous method on a plugin that was not " "yet loaded."); @@ -399,7 +428,7 @@ void MessageChannel::FromV8ValueComplete(VarConversionResult* result_holder, void MessageChannel::DrainCompletedPluginMessages() { DCHECK(instance_); - if (early_message_queue_state_ == QUEUE_MESSAGES) + if (plugin_message_queue_state_ == WAITING_TO_START) return; while (!plugin_message_queue_.empty() && @@ -417,22 +446,38 @@ void MessageChannel::DrainCompletedPluginMessages() { } } -void MessageChannel::DrainEarlyMessageQueue() { +void MessageChannel::DrainJSMessageQueue() { if (!instance_) return; - DCHECK(early_message_queue_state_ == QUEUE_MESSAGES); + if (js_message_queue_state_ == SEND_DIRECTLY) + return; // Take a reference on the PluginInstance. This is because JavaScript code // may delete the plugin, which would destroy the PluginInstance and its // corresponding MessageChannel. scoped_refptr instance_ref(instance_); - while (!early_message_queue_.empty()) { - PostMessageToJavaScriptImpl(early_message_queue_.front()); - early_message_queue_.pop_front(); + while (!js_message_queue_.empty()) { + PostMessageToJavaScriptImpl(js_message_queue_.front()); + js_message_queue_.pop_front(); } - early_message_queue_state_ = SEND_DIRECTLY; + js_message_queue_state_ = SEND_DIRECTLY; +} - DrainCompletedPluginMessages(); +void MessageChannel::DrainJSMessageQueueSoon() { + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&MessageChannel::DrainJSMessageQueue, + weak_ptr_factory_.GetWeakPtr())); +} + +void MessageChannel::UnregisterSyncMessageStatusObserver() { + if (!instance_) + return; + ppapi::proxy::HostDispatcher* dispatcher = + ppapi::proxy::HostDispatcher::GetForInstance(instance_->pp_instance()); + // The dispatcher is NULL for in-process. + if (dispatcher) + dispatcher->RemoveSyncMessageStatusObserver(this); } } // namespace content diff --git a/content/renderer/pepper/message_channel.h b/content/renderer/pepper/message_channel.h index 3caef263db38..daf2e53658ba 100644 --- a/content/renderer/pepper/message_channel.h +++ b/content/renderer/pepper/message_channel.h @@ -14,6 +14,7 @@ #include "gin/handle.h" #include "gin/interceptor.h" #include "gin/wrappable.h" +#include "ppapi/proxy/host_dispatcher.h" #include "ppapi/shared_impl/resource.h" #include "third_party/WebKit/public/web/WebSerializedScriptValue.h" #include "v8/include/v8.h" @@ -46,8 +47,10 @@ class PluginObject; // - The message target won't be limited to instance, and should support // either plugin-provided or JS objects. // TODO(dmichael): Add support for separate MessagePorts. -class MessageChannel : public gin::Wrappable, - public gin::NamedPropertyInterceptor { +class MessageChannel : + public gin::Wrappable, + public gin::NamedPropertyInterceptor, + public ppapi::proxy::HostDispatcher::SyncMessageStatusObserver { public: static gin::WrapperInfo kWrapperInfo; @@ -102,6 +105,10 @@ class MessageChannel : public gin::Wrappable, virtual gin::ObjectTemplateBuilder GetObjectTemplateBuilder( v8::Isolate* isolate) OVERRIDE; + // ppapi::proxy::HostDispatcher::SyncMessageStatusObserver + virtual void BeginBlockOnSyncMessage() OVERRIDE; + virtual void EndBlockOnSyncMessage() OVERRIDE; + // Post a message to the plugin's HandleMessage function for this channel's // instance. void PostMessageToNative(gin::Arguments* args); @@ -122,8 +129,18 @@ class MessageChannel : public gin::Wrappable, const ppapi::ScopedPPVar& result_var, bool success); + // Drain the queue of messages that are going to the plugin. All "completed" + // messages at the head of the queue will be sent; any messages awaiting + // conversion as well as messages after that in the queue will not be sent. void DrainCompletedPluginMessages(); - void DrainEarlyMessageQueue(); + // Drain the queue of messages that are going to JavaScript. + void DrainJSMessageQueue(); + // PostTask to call DrainJSMessageQueue() soon. Use this when you want to + // send the messages, but can't immediately (e.g., because the instance is + // not ready or JavaScript is on the stack). + void DrainJSMessageQueueSoon(); + + void UnregisterSyncMessageStatusObserver(); PepperPluginInstanceImpl* instance_; @@ -134,12 +151,21 @@ class MessageChannel : public gin::Wrappable, // scripting. v8::Persistent passthrough_object_; - std::deque early_message_queue_; - enum EarlyMessageQueueState { - QUEUE_MESSAGES, // Queue JS messages. - SEND_DIRECTLY, // Post JS messages directly. + enum MessageQueueState { + WAITING_TO_START, // Waiting for Start() to be called. Queue messages. + QUEUE_MESSAGES, // Queue messages temporarily. + SEND_DIRECTLY, // Post messages directly. }; - EarlyMessageQueueState early_message_queue_state_; + + // This queue stores values being posted to JavaScript. + std::deque js_message_queue_; + MessageQueueState js_message_queue_state_; + // When the renderer is sending a blocking message to the plugin, we will + // queue Plugin->JS messages temporarily to avoid re-entering JavaScript. This + // counts how many blocking renderer->plugin messages are on the stack so that + // we only begin sending messages to JavaScript again when the depth reaches + // zero. + int blocking_message_depth_; // This queue stores vars that are being sent to the plugin. Because // conversion can happen asynchronously for object types, the queue stores @@ -150,6 +176,7 @@ class MessageChannel : public gin::Wrappable, // calls to push_back or pop_front; hence why we're using list. (deque would // probably also work, but is less clearly specified). std::list plugin_message_queue_; + MessageQueueState plugin_message_queue_state_; std::map internal_named_properties_; diff --git a/content/renderer/pepper/pepper_hung_plugin_filter.h b/content/renderer/pepper/pepper_hung_plugin_filter.h index e7457fd11320..bc5d3863d3d0 100644 --- a/content/renderer/pepper/pepper_hung_plugin_filter.h +++ b/content/renderer/pepper/pepper_hung_plugin_filter.h @@ -26,9 +26,10 @@ namespace content { // thread. This is important since when we're blocked on a sync message to a // hung plugin, the main thread is frozen. // -// NOTE: This class is refcounted (via SyncMessageStatusReceiver). +// NOTE: This class is refcounted (via IPC::MessageFilter). class PepperHungPluginFilter - : public ppapi::proxy::HostDispatcher::SyncMessageStatusReceiver { + : public ppapi::proxy::HostDispatcher::SyncMessageStatusObserver, + public IPC::MessageFilter { public: // The |frame_routing_id| is the ID of the render_frame so that this class can // send messages to the browser via that frame's route. The |plugin_child_id| diff --git a/ppapi/proxy/dispatcher.cc b/ppapi/proxy/dispatcher.cc index 786c24d8b074..3c038a3906d8 100644 --- a/ppapi/proxy/dispatcher.cc +++ b/ppapi/proxy/dispatcher.cc @@ -47,11 +47,12 @@ InterfaceProxy* Dispatcher::GetInterfaceProxy(ApiID id) { return proxy; } -void Dispatcher::AddIOThreadMessageFilter(IPC::MessageFilter* filter) { +void Dispatcher::AddIOThreadMessageFilter( + scoped_refptr filter) { // Our filter is refcounted. The channel will call the destruct method on the // filter when the channel is done with it, so the corresponding Release() // happens there. - channel()->AddFilter(filter); + channel()->AddFilter(filter.get()); } bool Dispatcher::OnMessageReceived(const IPC::Message& msg) { diff --git a/ppapi/proxy/dispatcher.h b/ppapi/proxy/dispatcher.h index 68d67fe4d8b9..f37308a871c7 100644 --- a/ppapi/proxy/dispatcher.h +++ b/ppapi/proxy/dispatcher.h @@ -13,6 +13,7 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/tracked_objects.h" +#include "ipc/message_filter.h" #include "ppapi/c/pp_instance.h" #include "ppapi/c/pp_module.h" #include "ppapi/c/ppp.h" @@ -62,8 +63,8 @@ class PPAPI_PROXY_EXPORT Dispatcher : public ProxyChannel { // created so far. InterfaceProxy* GetInterfaceProxy(ApiID id); - // Adds the given filter to the IO thread. Takes ownership of the pointer. - void AddIOThreadMessageFilter(IPC::MessageFilter* filter); + // Adds the given filter to the IO thread. + void AddIOThreadMessageFilter(scoped_refptr filter); // IPC::Listener implementation. virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE; diff --git a/ppapi/proxy/host_dispatcher.cc b/ppapi/proxy/host_dispatcher.cc index 8f3833f5ddff..69de02a6677c 100644 --- a/ppapi/proxy/host_dispatcher.cc +++ b/ppapi/proxy/host_dispatcher.cc @@ -61,10 +61,8 @@ class BoolRestorer { HostDispatcher::HostDispatcher(PP_Module module, PP_GetInterface_Func local_get_interface, - SyncMessageStatusReceiver* sync_status, const PpapiPermissions& permissions) : Dispatcher(local_get_interface, permissions), - sync_status_(sync_status), pp_module_(module), ppb_proxy_(NULL), allow_plugin_reentrancy_(false) { @@ -94,8 +92,6 @@ bool HostDispatcher::InitHostWithChannel( if (!Dispatcher::InitWithChannel(delegate, peer_pid, channel_handle, is_client)) return false; - AddIOThreadMessageFilter(sync_status_.get()); - Send(new PpapiMsg_SetPreferences(preferences)); return true; } @@ -157,9 +153,11 @@ bool HostDispatcher::Send(IPC::Message* msg) { // destroys the plugin module and in turn the dispatcher. ScopedModuleReference scoped_ref(this); - sync_status_->BeginBlockOnSyncMessage(); + FOR_EACH_OBSERVER(SyncMessageStatusObserver, sync_status_observer_list_, + BeginBlockOnSyncMessage()); bool result = Dispatcher::Send(msg); - sync_status_->EndBlockOnSyncMessage(); + FOR_EACH_OBSERVER(SyncMessageStatusObserver, sync_status_observer_list_, + EndBlockOnSyncMessage()); return result; } else { @@ -240,6 +238,16 @@ const void* HostDispatcher::GetProxiedInterface(const std::string& iface_name) { return NULL; } +void HostDispatcher::AddSyncMessageStatusObserver( + SyncMessageStatusObserver* obs) { + sync_status_observer_list_.AddObserver(obs); +} + +void HostDispatcher::RemoveSyncMessageStatusObserver( + SyncMessageStatusObserver* obs) { + sync_status_observer_list_.RemoveObserver(obs); +} + void HostDispatcher::AddFilter(IPC::Listener* listener) { filters_.push_back(listener); } diff --git a/ppapi/proxy/host_dispatcher.h b/ppapi/proxy/host_dispatcher.h index c1ba15feb114..89ca780b678d 100644 --- a/ppapi/proxy/host_dispatcher.h +++ b/ppapi/proxy/host_dispatcher.h @@ -11,6 +11,7 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" +#include "base/observer_list.h" #include "base/process/process.h" #include "ipc/message_filter.h" #include "ppapi/c/pp_instance.h" @@ -27,11 +28,13 @@ namespace proxy { class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { public: // This interface receives notifications about sync messages being sent by - // the dispatcher to the plugin process. It is used to detect a hung plugin. + // the dispatcher to the plugin process. Some parts of Chrome may need to + // know whether we are sending a synchronous message to the plugin; e.g. to + // detect a hung plugin or to avoid re-entering JavaScript. // // Note that there can be nested sync messages, so the begin/end status // actually represents a stack of blocking messages. - class SyncMessageStatusReceiver : public IPC::MessageFilter { + class SyncMessageStatusObserver { public: // Notification that a sync message is about to be sent out. virtual void BeginBlockOnSyncMessage() = 0; @@ -41,7 +44,7 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { virtual void EndBlockOnSyncMessage() = 0; protected: - virtual ~SyncMessageStatusReceiver() {} + virtual ~SyncMessageStatusObserver() {} }; // Constructor for the renderer side. This will take a reference to the @@ -50,7 +53,6 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { // You must call InitHostWithChannel after the constructor. HostDispatcher(PP_Module module, PP_GetInterface_Func local_get_interface, - SyncMessageStatusReceiver* sync_status, const PpapiPermissions& permissions); ~HostDispatcher(); @@ -102,6 +104,9 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { // Returns the proxy interface for talking to the implementation. const PPB_Proxy_Private* ppb_proxy() const { return ppb_proxy_; } + void AddSyncMessageStatusObserver(SyncMessageStatusObserver* obs); + void RemoveSyncMessageStatusObserver(SyncMessageStatusObserver* obs); + void AddFilter(IPC::Listener* listener); protected: @@ -114,8 +119,6 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { const std::string& source, const std::string& value); - scoped_refptr sync_status_; - PP_Module pp_module_; // Maps interface name to whether that interface is supported. If an interface @@ -133,6 +136,8 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher { // ultimately call back into the plugin. bool allow_plugin_reentrancy_; + ObserverList sync_status_observer_list_; + std::vector filters_; DISALLOW_COPY_AND_ASSIGN(HostDispatcher); diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc index 81793d5c1742..a23362615329 100644 --- a/ppapi/proxy/ppapi_proxy_test.cc +++ b/ppapi/proxy/ppapi_proxy_test.cc @@ -12,6 +12,7 @@ #include "base/observer_list.h" #include "base/run_loop.h" #include "ipc/ipc_sync_channel.h" +#include "ipc/message_filter.h" #include "ppapi/c/pp_errors.h" #include "ppapi/c/private/ppb_proxy_private.h" #include "ppapi/proxy/ppapi_messages.h" @@ -407,16 +408,8 @@ void PluginProxyMultiThreadTest::InternalSetUpTestOnSecondaryThread( // HostProxyTestHarness -------------------------------------------------------- -class HostProxyTestHarness::MockSyncMessageStatusReceiver - : public HostDispatcher::SyncMessageStatusReceiver { - public: - virtual void BeginBlockOnSyncMessage() OVERRIDE {} - virtual void EndBlockOnSyncMessage() OVERRIDE {} -}; - HostProxyTestHarness::HostProxyTestHarness(GlobalsConfiguration globals_config) - : globals_config_(globals_config), - status_receiver_(new MockSyncMessageStatusReceiver) { + : globals_config_(globals_config) { } HostProxyTestHarness::~HostProxyTestHarness() { @@ -437,7 +430,6 @@ void HostProxyTestHarness::SetUpHarness() { host_dispatcher_.reset(new HostDispatcher( pp_module(), &MockGetInterface, - status_receiver_.release(), PpapiPermissions::AllPermissions())); host_dispatcher_->InitWithTestSink(&sink()); HostDispatcher::SetForInstance(pp_instance(), host_dispatcher_.get()); @@ -456,7 +448,6 @@ void HostProxyTestHarness::SetUpHarnessWithChannel( host_dispatcher_.reset(new HostDispatcher( pp_module(), &MockGetInterface, - status_receiver_.release(), PpapiPermissions::AllPermissions())); ppapi::Preferences preferences; host_dispatcher_->InitHostWithChannel(&delegate_mock_, diff --git a/ppapi/proxy/ppapi_proxy_test.h b/ppapi/proxy/ppapi_proxy_test.h index 9a2df1c5d2ca..980510563afd 100644 --- a/ppapi/proxy/ppapi_proxy_test.h +++ b/ppapi/proxy/ppapi_proxy_test.h @@ -285,16 +285,12 @@ class HostProxyTestHarness : public ProxyTestHarnessBase { }; private: - class MockSyncMessageStatusReceiver; - void CreateHostGlobals(); GlobalsConfiguration globals_config_; scoped_ptr host_globals_; scoped_ptr host_dispatcher_; DelegateMock delegate_mock_; - - scoped_ptr status_receiver_; }; class HostProxyTest : public HostProxyTestHarness, public testing::Test { diff --git a/ppapi/tests/test_message_handler.cc b/ppapi/tests/test_message_handler.cc index 0483a6b0cd49..c8132e69fadb 100644 --- a/ppapi/tests/test_message_handler.cc +++ b/ppapi/tests/test_message_handler.cc @@ -266,13 +266,8 @@ std::string TestMessageHandler::TestPostMessageAndAwaitResponse() { js_code += values_to_test[i]; js_code += " result: \" + result);\n"; } - // TODO(dmichael): Setting a property uses GetInstanceObject, which sends sync - // message, which can get interrupted with message to eval script, etc. - // FINISHED_WAITING message can therefore jump ahead. This test is - // currently carefully crafted to avoid races by doing all the JS in one call. - // That should be fixed before this API goes to stable. See crbug.com/384528 - js_code += "plugin.postMessage('FINISHED_TEST');\n"; instance_->EvalScript(js_code); + instance_->EvalScript("plugin.postMessage('FINISHED_TEST');\n"); handler.WaitForTestFinishedMessage(); handler.Unregister(); ASSERT_SUBTEST_SUCCESS(handler.WaitForDestroy()); -- 2.11.4.GIT