PPAPI: Make GetProxiedInterface not re-enter the plugin

It's important that we never re-enter the plugin when it is blocked on
synchronous calls to the renderer (unless they are scripting messages, which
have to be re-entrant). This especially breaks assumptions in OpenGL usage.

This required making the plugin side of the PPB_VideoDecoder_Dev proxy check
for the interface prior to sending its synchronous Create message. I audited
all the other uses of GetProxiedInterface from the renderer, and none of the
others happen as a result of a sync plugin->renderer message.

I also renamed the existing "SupportsInterface" message to "IsInterfaceSupported" to make the intent more obvious, and because I liked the SupportsInterface name better for the new usage: Telling the renderer that an interface is supported.

BUG=418651

Review URL: https://codereview.chromium.org/704913002

Cr-Commit-Position: refs/heads/master@{#303247}

9 files changed, 36 insertions, 18 deletions

diff --git a/ppapi/proxy/host_dispatcher.cc b/ppapi/proxy/host_dispatcher.cc
index ee7e919..0e821db 100644
--- a/ppapi/proxy/host_dispatcher.cc
+++ b/ppapi/proxy/host_dispatcher.cc
@@ -224,10 +224,7 @@ const void* HostDispatcher::GetProxiedInterface(const std::string& iface_name) {
     // Need to query. Cache the result so we only do this once.
     bool supported = false;
 
-    bool previous_reentrancy_value = allow_plugin_reentrancy_;
-    allow_plugin_reentrancy_ = true;
-    Send(new PpapiMsg_SupportsInterface(iface_name, &supported));
-    allow_plugin_reentrancy_ = previous_reentrancy_value;
+    Send(new PpapiMsg_IsInterfaceSupported(iface_name, &supported));
 
     std::pair<PluginSupportedMap::iterator, bool> iter_success_pair;
     iter_success_pair = plugin_supported_.insert(
@@ -274,6 +271,11 @@ void HostDispatcher::OnHostMsgLogWithSource(PP_Instance instance,
   }
 }
 
+void HostDispatcher::OnHostMsgPluginSupportsInterface(
+    const std::string& interface_name) {
+  plugin_supported_[interface_name] = true;
+}
+
 // ScopedModuleReference -------------------------------------------------------
 
 ScopedModuleReference::ScopedModuleReference(Dispatcher* dispatcher)
diff --git a/ppapi/proxy/host_dispatcher.h b/ppapi/proxy/host_dispatcher.h
index 345f8ea..0f503ff 100644
--- a/ppapi/proxy/host_dispatcher.h
+++ b/ppapi/proxy/host_dispatcher.h
@@ -122,6 +122,7 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher {
                               int int_log_level,
                               const std::string& source,
                               const std::string& value);
+  void OnHostMsgPluginSupportsInterface(const std::string& interface_name);
 
   void RemoveSyncMessageStatusObserver(SyncMessageStatusObserver* obs);
 
diff --git a/ppapi/proxy/plugin_dispatcher.cc b/ppapi/proxy/plugin_dispatcher.cc
index ad3e7a0..c3e96e2 100644
--- a/ppapi/proxy/plugin_dispatcher.cc
+++ b/ppapi/proxy/plugin_dispatcher.cc
@@ -223,7 +223,8 @@ bool PluginDispatcher::OnMessageReceived(const IPC::Message& msg) {
     // Handle some plugin-specific control messages.
     bool handled = true;
     IPC_BEGIN_MESSAGE_MAP(PluginDispatcher, msg)
-      IPC_MESSAGE_HANDLER(PpapiMsg_SupportsInterface, OnMsgSupportsInterface)
+      IPC_MESSAGE_HANDLER(PpapiMsg_IsInterfaceSupported,
+                          OnMsgIsInterfaceSupported)
       IPC_MESSAGE_HANDLER(PpapiMsg_SetPreferences, OnMsgSetPreferences)
       IPC_MESSAGE_UNHANDLED(handled = false);
     IPC_END_MESSAGE_MAP()
@@ -297,7 +298,7 @@ void PluginDispatcher::ForceFreeAllInstances() {
   }
 }
 
-void PluginDispatcher::OnMsgSupportsInterface(
+void PluginDispatcher::OnMsgIsInterfaceSupported(
     const std::string& interface_name,
     bool* result) {
   *result = !!GetPluginInterface(interface_name);
diff --git a/ppapi/proxy/plugin_dispatcher.h b/ppapi/proxy/plugin_dispatcher.h
index f8e22f6..2dc526f 100644
--- a/ppapi/proxy/plugin_dispatcher.h
+++ b/ppapi/proxy/plugin_dispatcher.h
@@ -175,7 +175,8 @@ class PPAPI_PROXY_EXPORT PluginDispatcher
   void ForceFreeAllInstances();
 
   // IPC message handlers.
-  void OnMsgSupportsInterface(const std::string& interface_name, bool* result);
+  void OnMsgIsInterfaceSupported(
+      const std::string& interface_name, bool* result);
   void OnMsgSetPreferences(const Preferences& prefs);
 
   virtual bool SendMessage(IPC::Message* msg);
diff --git a/ppapi/proxy/plugin_dispatcher_unittest.cc b/ppapi/proxy/plugin_dispatcher_unittest.cc
index 821ecad..83a5cf4 100644
--- a/ppapi/proxy/plugin_dispatcher_unittest.cc
+++ b/ppapi/proxy/plugin_dispatcher_unittest.cc
@@ -62,10 +62,10 @@ TEST_F(PluginDispatcherTest, SupportsInterface) {
   RegisterTestInterface(PPP_INSTANCE_INTERFACE, &dummy_ppp_instance_interface);
 
   // Sending a request for a random interface should fail.
-  EXPECT_FALSE(SupportsInterface("Random interface"));
+  EXPECT_FALSE(IsInterfaceSupported("Random interface"));
 
   // Sending a request for a supported PPP interface should succeed.
-  EXPECT_TRUE(SupportsInterface(PPP_INSTANCE_INTERFACE));
+  EXPECT_TRUE(IsInterfaceSupported(PPP_INSTANCE_INTERFACE));
 }
 
 TEST_F(PluginDispatcherTest, PPBCreation) {
diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h
index 484e6c7..135786c 100644
--- a/ppapi/proxy/ppapi_messages.h
+++ b/ppapi/proxy/ppapi_messages.h
@@ -486,9 +486,13 @@ IPC_MESSAGE_CONTROL1(PpapiMsg_SetPreferences,
 
 // Sent in both directions to see if the other side supports the given
 // interface.
-IPC_SYNC_MESSAGE_CONTROL1_1(PpapiMsg_SupportsInterface,
+IPC_SYNC_MESSAGE_CONTROL1_1(PpapiMsg_IsInterfaceSupported,
                             std::string /* interface_name */,
                             bool /* result */)
+// Sent by the plugin side of the proxy to inform the renderer that a given
+// plugin interface (PPP_*) is supported.
+IPC_MESSAGE_CONTROL1(PpapiHostMsg_PluginSupportsInterface,
+                     std::string /* interface_name */)
 
 IPC_MESSAGE_CONTROL1(PpapiHostMsg_LogInterfaceUsage,
                      int /* interface_hash */)
@@ -1180,9 +1184,9 @@ IPC_SYNC_MESSAGE_ROUTED3_1(
 IPC_SYNC_MESSAGE_ROUTED1_1(PpapiHostMsg_PPBTesting_GetLiveObjectsForInstance,
                            PP_Instance /* instance */,
                            uint32 /* result */)
-IPC_SYNC_MESSAGE_ROUTED2_0(PpapiHostMsg_PPBTesting_SimulateInputEvent,
-                           PP_Instance /* instance */,
-                           ppapi::InputEventData /* input_event */)
+IPC_MESSAGE_ROUTED2(PpapiHostMsg_PPBTesting_SimulateInputEvent,
+                    PP_Instance /* instance */,
+                    ppapi::InputEventData /* input_event */)
 IPC_SYNC_MESSAGE_ROUTED1_0(
     PpapiHostMsg_PPBTesting_SetMinimumArrayBufferSizeForShmem,
     uint32_t /* threshold */)
diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc
index 2c0b136..b75f4b8 100644
--- a/ppapi/proxy/ppapi_proxy_test.cc
+++ b/ppapi/proxy/ppapi_proxy_test.cc
@@ -122,13 +122,13 @@ void ProxyTestHarnessBase::RegisterTestInterface(const char* name,
   registered_interfaces_[name] = test_interface;
 }
 
-bool ProxyTestHarnessBase::SupportsInterface(const char* name) {
+bool ProxyTestHarnessBase::IsInterfaceSupported(const char* name) {
   sink().ClearMessages();
 
   // IPC doesn't actually write to this when we send a message manually
   // not actually using IPC.
   bool unused_result = false;
-  PpapiMsg_SupportsInterface msg(name, &unused_result);
+  PpapiMsg_IsInterfaceSupported msg(name, &unused_result);
   GetDispatcher()->OnMessageReceived(msg);
 
   const IPC::Message* reply_msg =
@@ -137,8 +137,8 @@ bool ProxyTestHarnessBase::SupportsInterface(const char* name) {
   if (!reply_msg)
     return false;
 
-  TupleTypes<PpapiMsg_SupportsInterface::ReplyParam>::ValueTuple reply_data;
-  EXPECT_TRUE(PpapiMsg_SupportsInterface::ReadReplyParam(
+  TupleTypes<PpapiMsg_IsInterfaceSupported::ReplyParam>::ValueTuple reply_data;
+  EXPECT_TRUE(PpapiMsg_IsInterfaceSupported::ReadReplyParam(
       reply_msg, &reply_data));
 
   sink().ClearMessages();
diff --git a/ppapi/proxy/ppapi_proxy_test.h b/ppapi/proxy/ppapi_proxy_test.h
index ce9ec65..c0c11cd 100644
--- a/ppapi/proxy/ppapi_proxy_test.h
+++ b/ppapi/proxy/ppapi_proxy_test.h
@@ -76,7 +76,7 @@ class ProxyTestHarnessBase {
 
   // Sends a "supports interface" message to the current dispatcher and returns
   // true if it's supported. This is just for the convenience of tests.
-  bool SupportsInterface(const char* name);
+  bool IsInterfaceSupported(const char* name);
 
  private:
   // Destination for IPC messages sent by the test.
diff --git a/ppapi/proxy/ppb_video_decoder_proxy.cc b/ppapi/proxy/ppb_video_decoder_proxy.cc
index 7699c84..8913c01 100644
--- a/ppapi/proxy/ppb_video_decoder_proxy.cc
+++ b/ppapi/proxy/ppb_video_decoder_proxy.cc
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
+#include "ppapi/c/dev/ppp_video_decoder_dev.h"
 #include "ppapi/proxy/enter_proxy.h"
 #include "ppapi/proxy/plugin_dispatcher.h"
 #include "ppapi/proxy/ppapi_messages.h"
@@ -195,6 +196,14 @@ PP_Resource PPB_VideoDecoder_Proxy::CreateProxyResource(
   if (!dispatcher->preferences().is_accelerated_video_decode_enabled)
     return 0;
 
+  // We must get the plugin interface now, prior to doing Create synchronously.
+  // Otherwise, the browser will try to get the interface via a re-entrant
+  // sync message back to us, which would deadlock.
+  const std::string if_name = PPP_VIDEODECODER_DEV_INTERFACE_0_11;
+  if (!dispatcher->GetPluginInterface(if_name))
+    return 0;
+  dispatcher->Send(new PpapiHostMsg_PluginSupportsInterface(if_name));
+
   EnterResourceNoLock<PPB_Graphics3D_API> enter_context(graphics_context,
                                                         true);
   if (enter_context.failed())