PPAPI: Make Messaging not PostTask and copy when out-of-process.

webkit/plugins/ppapi/message_channel.cc

Index: webkit/plugins/ppapi/message_channel.cc
diff --git a/webkit/plugins/ppapi/message_channel.cc b/webkit/plugins/ppapi/message_channel.cc
index 646b822201ca6e640262c3cd96ee892d2f8a82c3..5d5ff8620c7e7f2de1045a1b6a436dca10677682 100644
--- a/webkit/plugins/ppapi/message_channel.cc
+++ b/webkit/plugins/ppapi/message_channel.cc
@@ -24,6 +24,7 @@
 #include "v8/include/v8.h"
 #include "webkit/plugins/ppapi/host_array_buffer_var.h"
 #include "webkit/plugins/ppapi/npapi_glue.h"
+#include "webkit/plugins/ppapi/plugin_module.h"
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 using ppapi::ArrayBufferVar;
@@ -330,7 +331,8 @@ MessageChannel::MessageChannel(PluginInstance* instance)
     : instance_(instance),
       passthrough_object_(NULL),
       np_object_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
+      did_create_finished_(false) {
   // Now create an NPObject for receiving calls to postMessage. This sets the
   // reference count to 1.  We release it in the destructor.
   NPObject* obj = WebBindings::createObject(NULL, &message_channel_class);
@@ -358,11 +360,40 @@ void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
   WebSerializedScriptValue serialized_val =
       WebSerializedScriptValue::serialize(v8_val);
 
+  if (instance_->module()->IsProxied()) {
+    // The proxy sent an asynchronous message, so the plugin is already
+    //  unblocked. Therefore, there's no need to PostTask.

[dmichael (off chromium), 2012/12/10 22:02:52]

This comment probably belongs in the first branch of the if below now

[teravest, 2012/12/10 22:30:59]

Done.

+    if (did_create_finished_) {
+      DCHECK(early_message_queue_.size() == 0);
+      PostMessageToJavaScriptImpl(serialized_val);
+    } else {
+      early_message_queue_.push_back(serialized_val);

[dmichael (off chromium), 2012/12/10 22:02:52]

Probably worth a brief comment why we have to defer sending (here or on the
member in the header)

And a comment here about why we can't just PostTask

[teravest, 2012/12/10 22:30:59]

Done.

+    }
+  } else {
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&MessageChannel::PostMessageToJavaScriptImpl,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   serialized_val));
+  }
+}
+
+void MessageChannel::DidCreateFinished() {
+  // We PostTask here instead of draining the message queue directly
+  // since we haven't finished initializing the WebPluginImpl yet, so
+  // the plugin isn't available in the DOM.
   MessageLoop::current()->PostTask(
       FROM_HERE,
-      base::Bind(&MessageChannel::PostMessageToJavaScriptImpl,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 serialized_val));
+      base::Bind(&MessageChannel::DrainEarlyMessageQueue,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void MessageChannel::DrainEarlyMessageQueue() {

[dmichael (off chromium), 2012/12/10 22:02:52]

Might be worth doing:
DCHECK(!did_create_finished_);

[teravest, 2012/12/10 22:30:59]

Done.

+  while (!early_message_queue_.empty()) {
+    PostMessageToJavaScriptImpl(early_message_queue_.front());
+    early_message_queue_.pop_front();

[dmichael (off chromium), 2012/12/10 22:02:52]

If there's any chance of re-entry, you'd probably want to do something more
like:
WebSerializedScriptValue val = early_message_queue_.front();
early_message_queue_.pop_front();
PostMessageToJavaScriptImpl(val);

...it's unlikely, but it actually could happen for trusted in-process plugins,
if they do some sync scripting call in their JavaScript message event handler.

[teravest, 2012/12/10 22:30:59]

I think it's safe as is...

On 2012/12/10 22:02:52, dmichael wrote:

[dmichael (off chromium), 2012/12/10 22:49:47]

Yes, I wasn't thinking straight; the problem I was imagining would happen if
there was re-entry in to *this* method, which should never happen.

+  }
+  did_create_finished_ = true;
 }
 
 void MessageChannel::PostMessageToJavaScriptImpl(
@@ -398,13 +429,20 @@ void MessageChannel::PostMessageToJavaScriptImpl(
 }
 
 void MessageChannel::PostMessageToNative(PP_Var message_data) {
-  // Make a copy of the message data for the Task we will run.
-  PP_Var var_copy(CopyPPVar(message_data));
+  if (instance_->module()->IsProxied()) {
+    // In the proxied case, the copy will happen via serializiation, and the
+    // message is asynchronous. Therefore there's no need to copy the Var, nor
+    // to PostTask.
+    PostMessageToNativeImpl(message_data);
+  } else {
+    // Make a copy of the message data for the Task we will run.
+    PP_Var var_copy(CopyPPVar(message_data));
 
-  MessageLoop::current()->PostTask(FROM_HERE,
-      base::Bind(&MessageChannel::PostMessageToNativeImpl,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 var_copy));
+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&MessageChannel::PostMessageToNativeImpl,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   var_copy));
+  }
 }
 
 void MessageChannel::PostMessageToNativeImpl(PP_Var message_data) {