PPAPI: Implement synchronous postMessage

BUG=367896

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278102 0039d316-1c4b-4281-b951-d872f2087c98

chrome/test/ppapi/ppapi_browsertest.cc

@@ -62,11 +62,25 @@ using content::RenderViewHost;
 #if defined(DISABLE_NACL)
 
 #define TEST_PPAPI_NACL(test_name)
+#define TEST_PPAPI_NACL_NO_PNACL(test_name)
 #define TEST_PPAPI_NACL_DISALLOWED_SOCKETS(test_name)
 #define TEST_PPAPI_NACL_WITH_SSL_SERVER(test_name)
 
 #else
 
+// TODO(dmichael): Remove this macro, crbug.com/384539
+#define TEST_PPAPI_NACL_NO_PNACL(test_name) \
+    IN_PROC_BROWSER_TEST_F(PPAPINaClNewlibTest, test_name) { \
+      RunTestViaHTTP(STRIP_PREFIXES(test_name)); \
+    } \
+    IN_PROC_BROWSER_TEST_F(PPAPINaClGLibcTest, MAYBE_GLIBC(test_name)) { \
+      RunTestViaHTTP(STRIP_PREFIXES(test_name)); \
+    } \
+    IN_PROC_BROWSER_TEST_F(PPAPINaClPNaClNonSfiTest, \
+                           MAYBE_PNACL_NONSFI(test_name)) { \
+      RunTestViaHTTP(STRIP_PREFIXES(test_name)); \
+    }
+
 // NaCl based PPAPI tests
 #define TEST_PPAPI_NACL(test_name) \
     IN_PROC_BROWSER_TEST_F(PPAPINaClNewlibTest, test_name) { \
@@ -1219,6 +1233,10 @@ TEST_PPAPI_NACL(VideoSource)
 // Printing doesn't work in content_browsertests.
 TEST_PPAPI_OUT_OF_PROCESS(Printing)
 
+// TODO(dmichael): Make this work on PNaCl and remove the macro.
+//                 crbug.com/384539
+TEST_PPAPI_NACL_NO_PNACL(MessageHandler)
+
 TEST_PPAPI_NACL(MessageLoop_Basics)
 TEST_PPAPI_NACL(MessageLoop_Post)
 

content/renderer/pepper/message_channel.cc

@@ -45,6 +45,7 @@ namespace content {
 namespace {
 
 const char kPostMessage[] = "postMessage";
+const char kPostMessageAndAwaitResponse[] = "postMessageAndAwaitResponse";
 const char kV8ToVarConversionError[] =
     "Failed to convert a PostMessage "
     "argument from a JavaScript value to a PP_Var. It may have cycles or be of "
@@ -71,6 +72,14 @@ bool IdentifierIs(NPIdentifier identifier, const char string[]) {
   return WebBindings::getStringIdentifier(string) == identifier;
 }
 
+bool HasDevChannelPermission(NPObject* channel_object) {
+  MessageChannel* channel = ToMessageChannel(channel_object);
+  if (!channel)
+    return false;
+  return channel->instance()->module()->permissions().HasPermission(
+      ppapi::PERMISSION_DEV_CHANNEL);
+}
+
 //------------------------------------------------------------------------------
 // Implementations of NPClass functions.  These are here to:
 // - Implement postMessage behavior.
@@ -93,7 +102,10 @@ bool MessageChannelHasMethod(NPObject* np_obj, NPIdentifier name) {
 
   if (IdentifierIs(name, kPostMessage))
     return true;
-
+  if (IdentifierIs(name, kPostMessageAndAwaitResponse) &&
+      HasDevChannelPermission(np_obj)) {
+    return true;
+  }
   // Other method names we will pass to the passthrough object, if we have one.
   NPObject* passthrough = ToPassThroughObject(np_obj);
   if (passthrough)
@@ -113,12 +125,17 @@ bool MessageChannelInvoke(NPObject* np_obj,
   if (!message_channel)
     return false;
 
-  // Check to see if we should handle this function ourselves. We only handle
-  // kPostMessage.
+  // Check to see if we should handle this function ourselves.
   if (IdentifierIs(name, kPostMessage) && (arg_count == 1)) {
     message_channel->PostMessageToNative(&args[0]);
     return true;
+  } else if (IdentifierIs(name, kPostMessageAndAwaitResponse) &&
+             (arg_count == 1) &&
+             HasDevChannelPermission(np_obj)) {
+    message_channel->PostBlockingMessageToNative(&args[0], result);
+    return true;
   }
+
   // Other method calls we will pass to the passthrough object, if we have one.
   NPObject* passthrough = ToPassThroughObject(np_obj);
   if (passthrough) {
@@ -167,10 +184,13 @@ bool MessageChannelGetProperty(NPObject* np_obj,
   if (!np_obj)
     return false;
 
-  // Don't allow getting the postMessage function.
+  // Don't allow getting the postMessage functions.
   if (IdentifierIs(name, kPostMessage))
     return false;
-
+  if (IdentifierIs(name, kPostMessageAndAwaitResponse) &&
+      HasDevChannelPermission(np_obj)) {
+     return false;
+  }
   MessageChannel* message_channel = ToMessageChannel(np_obj);
   if (message_channel) {
     if (message_channel->GetReadOnlyProperty(name, result))
@@ -190,10 +210,13 @@ bool MessageChannelSetProperty(NPObject* np_obj,
   if (!np_obj)
     return false;
 
-  // Don't allow setting the postMessage function.
+  // Don't allow setting the postMessage functions.
   if (IdentifierIs(name, kPostMessage))
     return false;
-
+  if (IdentifierIs(name, kPostMessageAndAwaitResponse) &&
+      HasDevChannelPermission(np_obj)) {
+    return false;
+  }
   // Invoke on the passthrough object, if we have one.
   NPObject* passthrough = ToPassThroughObject(np_obj);
   if (passthrough)
@@ -447,6 +470,80 @@ void MessageChannel::PostMessageToNative(const NPVariant* message_data) {
   DrainCompletedPluginMessages();
 }
 
+void MessageChannel::PostBlockingMessageToNative(const NPVariant* message_data,
+                                                 NPVariant* np_result) {
+  if (early_message_queue_state_ == QUEUE_MESSAGES) {
+    WebBindings::setException(
+        np_object_,
+        "Attempted to call a synchronous method on a plugin that was not "
+        "yet loaded.");
+    return;
+  }
+
+  // If the queue of messages to the plugin is non-empty, we're still waiting on
+  // pending Var conversions. This means at some point in the past, JavaScript
+  // called postMessage (the async one) and passed us something with a browser-
+  // side host (e.g., FileSystem) and we haven't gotten a response from the
+  // browser yet. We can't currently support sending a sync message if the
+  // plugin does this, because it will break the ordering of the messages
+  // arriving at the plugin.
+  // TODO(dmichael): Fix this.
+  // See https://code.google.com/p/chromium/issues/detail?id=367896#c4
+  if (!plugin_message_queue_.empty()) {
+    WebBindings::setException(
+        np_object_,
+        "Failed to convert parameter synchronously, because a prior "
+        "call to postMessage contained a type which required asynchronous "
+        "transfer which has not completed. Not all types are supported yet by "
+        "postMessageAndAwaitResponse. See crbug.com/367896.");
+    return;
+  }
+  ScopedPPVar param;
+  if (message_data->type == NPVariantType_Object) {
+    // Convert NPVariantType_Object in to an appropriate PP_Var like Dictionary,
+    // Array, etc. Note NPVariantToVar would convert to an "Object" PP_Var,
+    // which we don't support for Messaging.
+    v8::Handle<v8::Value> v8_value = WebBindings::toV8Value(message_data);
+    V8VarConverter v8_var_converter(instance_->pp_instance());
+    bool success = v8_var_converter.FromV8ValueSync(
+        v8_value,
+        v8::Isolate::GetCurrent()->GetCurrentContext(),
+        &param);
+    if (!success) {
+      WebBindings::setException(
+          np_object_,
+          "Failed to convert the given parameter to a PP_Var to send to "
+          "the plugin.");
+      return;
+    }
+  } else {
+    param = ScopedPPVar(ScopedPPVar::PassRef(),
+                        NPVariantToPPVar(instance(), message_data));
+  }
+  ScopedPPVar pp_result;
+  bool was_handled = instance_->HandleBlockingMessage(param, &pp_result);
+  if (!was_handled) {
+    WebBindings::setException(
+        np_object_,
+        "The plugin has not registered a handler for synchronous messages. "
+        "See the documentation for PPB_Messaging::RegisterMessageHandler "
+        "and PPP_MessageHandler.");
+    return;
+  }
+  v8::Handle<v8::Value> v8_val;
+  if (!V8VarConverter(instance_->pp_instance()).ToV8Value(
+          pp_result.get(),
+          v8::Isolate::GetCurrent()->GetCurrentContext(),
+          &v8_val)) {
+    WebBindings::setException(
+        np_object_,
+        "Failed to convert the plugin's result to a JavaScript type.");
+    return;
+  }
+  // Success! Convert the result to an NPVariant.
+  WebBindings::toNPVariant(v8_val, NULL, np_result);
+}
+
 MessageChannel::~MessageChannel() {
   WebBindings::releaseObject(np_object_);
   if (passthrough_object_)

content/renderer/pepper/message_channel.h

@@ -60,6 +60,10 @@ class MessageChannel {
   // Post a message to the plugin's HandleMessage function for this channel's
   // instance.
   void PostMessageToNative(const NPVariant* message_data);
+  // Post a message to the plugin's HandleBlocking Message function for this
+  // channel's instance synchronously, and return a result.
+  void PostBlockingMessageToNative(const NPVariant* message_data,
+                                   NPVariant* np_result);
 
   // Return the NPObject* to which we should forward any calls which aren't
   // related to postMessage.  Note that this can be NULL;  it only gets set if

content/renderer/pepper/pepper_plugin_instance_impl.cc

@@ -1168,8 +1168,8 @@ void PepperPluginInstanceImpl::HandleMessage(ScopedPPVar message) {
   ppapi::proxy::HostDispatcher* dispatcher =
       ppapi::proxy::HostDispatcher::GetForInstance(pp_instance());
   if (!dispatcher || (message.get().type == PP_VARTYPE_OBJECT)) {
-    // The dispatcher should always be valid, and the browser should never send
-    // an 'object' var over PPP_Messaging.
+    // The dispatcher should always be valid, and MessageChannel should never
+    // send an 'object' var over PPP_Messaging.
     NOTREACHED();
     return;
   }
@@ -1180,6 +1180,32 @@ void PepperPluginInstanceImpl::HandleMessage(ScopedPPVar message) {
                                                 pp_instance())));
 }
 
+bool PepperPluginInstanceImpl::HandleBlockingMessage(ScopedPPVar message,
+                                                     ScopedPPVar* result) {
+  TRACE_EVENT0("ppapi", "PepperPluginInstanceImpl::HandleBlockingMessage");
+  ppapi::proxy::HostDispatcher* dispatcher =
+      ppapi::proxy::HostDispatcher::GetForInstance(pp_instance());
+  if (!dispatcher || (message.get().type == PP_VARTYPE_OBJECT)) {
+    // The dispatcher should always be valid, and MessageChannel should never
+    // send an 'object' var over PPP_Messaging.
+    NOTREACHED();
+    return false;
+  }
+  ppapi::proxy::ReceiveSerializedVarReturnValue msg_reply;
+  bool was_handled = false;
+  dispatcher->Send(new PpapiMsg_PPPMessageHandler_HandleBlockingMessage(
+      ppapi::API_ID_PPP_MESSAGING,
+      pp_instance(),
+      ppapi::proxy::SerializedVarSendInputShmem(dispatcher, message.get(),
+                                                pp_instance()),
+      &msg_reply,
+      &was_handled));
+  *result = ScopedPPVar(ScopedPPVar::PassRef(), msg_reply.Return(dispatcher));
+  TRACE_EVENT0("ppapi",
+               "PepperPluginInstanceImpl::HandleBlockingMessage return.");
+  return was_handled;
+}
+
 PP_Var PepperPluginInstanceImpl::GetInstanceObject() {
   // Keep a reference on the stack. See NOTE above.
   scoped_refptr<PepperPluginInstanceImpl> ref(this);

content/renderer/pepper/pepper_plugin_instance_impl.h

@@ -303,6 +303,12 @@ class CONTENT_EXPORT PepperPluginInstanceImpl
   // Send the message on to the plugin.
   void HandleMessage(ppapi::ScopedPPVar message);
 
+  // Send the message synchronously to the plugin, and get a result. Returns
+  // true if the plugin handled the message, false if it didn't. The plugin
+  // won't handle the message if it has not registered a PPP_MessageHandler.
+  bool HandleBlockingMessage(ppapi::ScopedPPVar message,
+                             ppapi::ScopedPPVar* result);
+
   // Returns true if the plugin is processing a user gesture.
   bool IsProcessingUserGesture();
 

content/renderer/pepper/v8_var_converter.cc

@@ -411,6 +411,18 @@ V8VarConverter::VarResult V8VarConverter::FromV8Value(
   return result;
 }
 
+bool V8VarConverter::FromV8ValueSync(
+    v8::Handle<v8::Value> val,
+    v8::Handle<v8::Context> context,
+    ppapi::ScopedPPVar* result_var) {
+  bool success = FromV8ValueInternal(val, context, result_var);
+  if (!success || resource_converter_->NeedsFlush()) {
+    resource_converter_->Reset();
+    return false;
+  }
+  return true;
+}
+
 bool V8VarConverter::FromV8ValueInternal(
     v8::Handle<v8::Value> val,
     v8::Handle<v8::Context> context,

content/renderer/pepper/v8_var_converter.h

@@ -62,6 +62,9 @@ class CONTENT_EXPORT V8VarConverter {
       v8::Handle<v8::Value> val,
       v8::Handle<v8::Context> context,
       const base::Callback<void(const ppapi::ScopedPPVar&, bool)>& callback);
+  bool FromV8ValueSync(v8::Handle<v8::Value> val,
+                       v8::Handle<v8::Context> context,
+                       ppapi::ScopedPPVar* result_var);
  private:
   // Returns true on success, false on failure.
   bool FromV8ValueInternal(v8::Handle<v8::Value> val,

content/test/ppapi/ppapi_browsertest.cc

@@ -108,6 +108,8 @@ TEST_PPAPI_OUT_OF_PROCESS(MediaStreamVideoTrack)
 TEST_PPAPI_IN_PROCESS(Memory)
 TEST_PPAPI_OUT_OF_PROCESS(Memory)
 
+TEST_PPAPI_OUT_OF_PROCESS(MessageHandler)
+
 TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Basics)
 TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Post)
 

ppapi/api/ppb_messaging.idl

@@ -86,9 +86,6 @@ interface PPB_Messaging {
   void PostMessage([in] PP_Instance instance, [in] PP_Var message);
 
   /**
-   * <strong>Note:</strong> This function is not yet implemented. Please use
-   * PPB_Messaging_1_0.
-   *
    * Registers a handler for receiving messages from JavaScript. If a handler
    * is registered this way, it will replace PPP_Messaging, and all messages
    * sent from JavaScript via postMessage and postMessageAndAwaitResponse will
@@ -99,6 +96,12 @@ interface PPB_Messaging {
    * <code>message_loop</code> is attached, when <code>message_loop</code> is
    * run. It is illegal to pass the main thread message loop;
    * RegisterMessageHandler will return PP_ERROR_WRONG_THREAD in that case.
+   * If you quit <code>message_loop</code> before calling Unregister(),
+   * the browser will not be able to call functions in the plugin's message
+   * handler any more. That could mean missing some messages or could cause a
+   * leak if you depend on Destroy() to free hander data. So you should,
+   * whenever possible, Unregister() the handler prior to quitting its event
+   * loop.
    *
    * Attempting to register a message handler when one is already registered
    * will cause the current MessageHandler to be unregistered and replaced. In
@@ -123,9 +126,6 @@ interface PPB_Messaging {
                                  [in] PPP_MessageHandler handler,
                                  [in] PP_Resource message_loop);
   /**
-   * <strong>Note:</strong> This function is not yet implemented. Please use
-   * PPB_Messaging_1_0.
-   *
    * Unregisters the current message handler for <code>instance</code> if one
    * is registered. After this call, the message handler (if one was
    * registered) will have "Destroy" called on it and will receive no further

ppapi/api/ppp_message_handler.idl

@@ -37,7 +37,7 @@ interface PPP_MessageHandler {
    * postMessage().
    */
   void HandleMessage([in] PP_Instance instance,
-                     [in] mem_t user_data,
+                     [inout] mem_t user_data,
                      [in] PP_Var message);
   /**
    * Invoked as a result of JavaScript invoking postMessageAndAwaitResponse()

ppapi/c/ppb_messaging.h

@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From ppb_messaging.idl modified Mon Jun  2 11:00:28 2014. */
+/* From ppb_messaging.idl modified Fri Jun 13 15:28:26 2014. */
 
 #ifndef PPAPI_C_PPB_MESSAGING_H_
 #define PPAPI_C_PPB_MESSAGING_H_
@@ -100,9 +100,6 @@ struct PPB_Messaging_1_1 { /* dev */
    */
   void (*PostMessage)(PP_Instance instance, struct PP_Var message);
   /**
-   * <strong>Note:</strong> This function is not yet implemented. Please use
-   * PPB_Messaging_1_0.
-   *
    * Registers a handler for receiving messages from JavaScript. If a handler
    * is registered this way, it will replace PPP_Messaging, and all messages
    * sent from JavaScript via postMessage and postMessageAndAwaitResponse will
@@ -113,6 +110,12 @@ struct PPB_Messaging_1_1 { /* dev */
    * <code>message_loop</code> is attached, when <code>message_loop</code> is
    * run. It is illegal to pass the main thread message loop;
    * RegisterMessageHandler will return PP_ERROR_WRONG_THREAD in that case.
+   * If you quit <code>message_loop</code> before calling Unregister(),
+   * the browser will not be able to call functions in the plugin's message
+   * handler any more. That could mean missing some messages or could cause a
+   * leak if you depend on Destroy() to free hander data. So you should,
+   * whenever possible, Unregister() the handler prior to quitting its event
+   * loop.
    *
    * Attempting to register a message handler when one is already registered
    * will cause the current MessageHandler to be unregistered and replaced. In
@@ -137,9 +140,6 @@ struct PPB_Messaging_1_1 { /* dev */
       const struct PPP_MessageHandler_0_1* handler,
       PP_Resource message_loop);
   /**
-   * <strong>Note:</strong> This function is not yet implemented. Please use
-   * PPB_Messaging_1_0.
-   *
    * Unregisters the current message handler for <code>instance</code> if one
    * is registered. After this call, the message handler (if one was
    * registered) will have "Destroy" called on it and will receive no further