PPAPI: Refactor MessageChannel to prep for sync postMessage

1) Remove unused CopyPPVar function from MessageChannel
2) Remove mostly-duplicate NPVariantToPPVar from MessageChannel.
3) Separate V8VarConverter::FromV8Value implementation from calling the callback (so I can have a sync path later, and this is shorter anyway).
4) Simplify NaCl in-process to out-of-process transition. Now, we just queue stuff for in-process (nobody but NaCl uses Messaging in-process), and if/when we switch to out-of-process (handing off to a NaCl app) we drain the queues then.
5) Some other renames/tweaks to how MessageChannel queues pending conversions.

BUG=367896
R=raymes@chromium.org

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

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

17 files changed, 278 insertions, 272 deletions

diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index f57f12b..d625a25 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -110,6 +110,10 @@
 #include "ui/base/webui/jstemplate_builder.h"
 #include "widevine_cdm_version.h"  // In SHARED_INTERMEDIATE_DIR.
 
+#if !defined(DISABLE_NACL)
+#include "components/nacl/renderer/nacl_helper.h"
+#endif
+
 #if defined(ENABLE_WEBRTC)
 #include "chrome/renderer/media/webrtc_logging_message_filter.h"
 #endif
@@ -397,6 +401,10 @@ void ChromeContentRendererClient::RenderFrameCreated(
   new PepperHelper(render_frame);
 #endif
 
+#if !defined(DISABLE_NACL)
+  new nacl::NaClHelper(render_frame);
+#endif
+
   // TODO(jam): when the frame tree moves into content and parent() works at
   // RenderFrame construction, simplify this by just checking parent().
   if (render_frame->GetRenderView()->GetMainRenderFrame() != render_frame) {
diff --git a/components/nacl.gyp b/components/nacl.gyp
index 0cbc668e..faeb887 100644
--- a/components/nacl.gyp
+++ b/components/nacl.gyp
@@ -161,6 +161,8 @@
             'nacl/renderer/manifest_downloader.h',
             'nacl/renderer/manifest_service_channel.cc',
             'nacl/renderer/manifest_service_channel.h',
+            'nacl/renderer/nacl_helper.cc',
+            'nacl/renderer/nacl_helper.h',
             'nacl/renderer/json_manifest.cc',
             'nacl/renderer/json_manifest.h',
             'nacl/renderer/nexe_load_manager.cc',
diff --git a/components/nacl/renderer/nacl_helper.cc b/components/nacl/renderer/nacl_helper.cc
new file mode 100644
index 0000000..516096a
--- /dev/null
+++ b/components/nacl/renderer/nacl_helper.cc
@@ -0,0 +1,22 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/nacl/renderer/nacl_helper.h"
+
+#include "content/public/renderer/renderer_ppapi_host.h"
+
+namespace nacl {
+
+NaClHelper::NaClHelper(content::RenderFrame* render_frame)
+    : RenderFrameObserver(render_frame) {}
+
+NaClHelper::~NaClHelper() {}
+
+void NaClHelper::DidCreatePepperPlugin(content::RendererPpapiHost* host) {
+  // The Native Client plugin is a host for external plugins.
+  if (host->GetPluginName() == "Native Client")
+    host->SetToExternalPluginHost();
+}
+
+}  // namespace nacl
diff --git a/components/nacl/renderer/nacl_helper.h b/components/nacl/renderer/nacl_helper.h
new file mode 100644
index 0000000..be3a52d
--- /dev/null
+++ b/components/nacl/renderer/nacl_helper.h
@@ -0,0 +1,31 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_NACL_RENDERER_NACL_HELPER_H_
+#define COMPONENTS_NACL_RENDERER_NACL_HELPER_H_
+
+#include "base/compiler_specific.h"
+#include "content/public/renderer/render_frame_observer.h"
+
+namespace nacl {
+
+// This class listens for Pepper creation events from the RenderFrame. For the
+// NaCl trusted plugin, it configures it as an external plugin host.
+// TODO(dmichael): When the trusted plugin goes away, we need to figure out the
+//                 right event to watch for.
+class NaClHelper : public content::RenderFrameObserver {
+ public:
+  explicit NaClHelper(content::RenderFrame* render_frame);
+  virtual ~NaClHelper();
+
+  // RenderFrameObserver.
+  virtual void DidCreatePepperPlugin(content::RendererPpapiHost* host) OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NaClHelper);
+};
+
+}  // namespace nacl
+
+#endif  // COMPONENTS_NACL_RENDERER_NACL_HELPER_H_
diff --git a/content/public/renderer/renderer_ppapi_host.h b/content/public/renderer/renderer_ppapi_host.h
index 4e253e2..6c843dc 100644
--- a/content/public/renderer/renderer_ppapi_host.h
+++ b/content/public/renderer/renderer_ppapi_host.h
@@ -123,6 +123,23 @@ class RendererPpapiHost {
   // Returns true if the plugin is running in process.
   virtual bool IsRunningInProcess() const = 0;
 
+  virtual std::string GetPluginName() const = 0;
+
+  // Used by the embedder to inform this RendererPpapiHost that the associated
+  // plugin module is a host for "external plugins."
+  //
+  // An embedder may, at the time a plugin module is created, configure it to
+  // be a host for external plugins. Instances of such plugins go through two
+  // two stages of initialization; the first stage initializes a host plugin
+  // instance, which then loads and initializes a child plugin which takes
+  // over control. These are treated as one Pepper Instance, because despite the
+  // two-stage initialization process, the host and child appear to blink as
+  // one plugin instance.
+  //
+  // The host plugin appears as an in-process plugin, while we interact with the
+  // child plugin via the Pepper proxy.
+  virtual void SetToExternalPluginHost() = 0;
+
   // There are times when the renderer needs to create a ResourceHost in the
   // browser. This function does so asynchronously. |nested_msgs| is a list of
   // resource host creation messages and |instance| is the PP_Instance which
diff --git a/content/renderer/pepper/message_channel.cc b/content/renderer/pepper/message_channel.cc
index 81f49e9..12d24ba 100644
--- a/content/renderer/pepper/message_channel.cc
+++ b/content/renderer/pepper/message_channel.cc
@@ -31,6 +31,7 @@
 
 using ppapi::ArrayBufferVar;
 using ppapi::PpapiGlobals;
+using ppapi::ScopedPPVar;
 using ppapi::StringVar;
 using blink::WebBindings;
 using blink::WebElement;
@@ -65,56 +66,9 @@ NPObject* ToPassThroughObject(NPObject* object) {
   return channel ? channel->passthrough_object() : NULL;
 }
 
-// Helper function to determine if a given identifier is equal to kPostMessage.
-bool IdentifierIsPostMessage(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kPostMessage) == identifier;
-}
-
-// Copy a PP_Var in to a PP_Var that is appropriate for sending via postMessage.
-// This currently just copies the value.  For a string Var, the result is a
-// PP_Var with the a copy of |var|'s string contents and a reference count of 1.
-PP_Var CopyPPVar(const PP_Var& var) {
-  switch (var.type) {
-    case PP_VARTYPE_UNDEFINED:
-    case PP_VARTYPE_NULL:
-    case PP_VARTYPE_BOOL:
-    case PP_VARTYPE_INT32:
-    case PP_VARTYPE_DOUBLE:
-      return var;
-    case PP_VARTYPE_STRING: {
-      StringVar* string = StringVar::FromPPVar(var);
-      if (!string)
-        return PP_MakeUndefined();
-      return StringVar::StringToPPVar(string->value());
-    }
-    case PP_VARTYPE_ARRAY_BUFFER: {
-      ArrayBufferVar* buffer = ArrayBufferVar::FromPPVar(var);
-      if (!buffer)
-        return PP_MakeUndefined();
-      PP_Var new_buffer_var =
-          PpapiGlobals::Get()->GetVarTracker()->MakeArrayBufferPPVar(
-              buffer->ByteLength());
-      DCHECK(new_buffer_var.type == PP_VARTYPE_ARRAY_BUFFER);
-      if (new_buffer_var.type != PP_VARTYPE_ARRAY_BUFFER)
-        return PP_MakeUndefined();
-      ArrayBufferVar* new_buffer = ArrayBufferVar::FromPPVar(new_buffer_var);
-      DCHECK(new_buffer);
-      if (!new_buffer)
-        return PP_MakeUndefined();
-      memcpy(new_buffer->Map(), buffer->Map(), buffer->ByteLength());
-      return new_buffer_var;
-    }
-    case PP_VARTYPE_OBJECT:
-    case PP_VARTYPE_ARRAY:
-    case PP_VARTYPE_DICTIONARY:
-    case PP_VARTYPE_RESOURCE:
-      // These types are not supported by PostMessage in-process. In some rare
-      // cases with the NaCl plugin, they may be sent but they will be dropped
-      // anyway (see crbug.com/318837 for details).
-      return PP_MakeUndefined();
-  }
-  NOTREACHED();
-  return PP_MakeUndefined();
+// Return true iff |identifier| is equal to |string|.
+bool IdentifierIs(NPIdentifier identifier, const char string[]) {
+  return WebBindings::getStringIdentifier(string) == identifier;
 }
 
 //------------------------------------------------------------------------------
@@ -137,8 +91,7 @@ bool MessageChannelHasMethod(NPObject* np_obj, NPIdentifier name) {
   if (!np_obj)
     return false;
 
-  // We only handle a function called postMessage.
-  if (IdentifierIsPostMessage(name))
+  if (IdentifierIs(name, kPostMessage))
     return true;
 
   // Other method names we will pass to the passthrough object, if we have one.
@@ -156,15 +109,15 @@ bool MessageChannelInvoke(NPObject* np_obj,
   if (!np_obj)
     return false;
 
-  // We only handle a function called postMessage.
-  if (IdentifierIsPostMessage(name) && (arg_count == 1)) {
-    MessageChannel* message_channel = ToMessageChannel(np_obj);
-    if (message_channel) {
-      message_channel->NPVariantToPPVar(&args[0]);
-      return true;
-    } else {
-      return false;
-    }
+  MessageChannel* message_channel = ToMessageChannel(np_obj);
+  if (!message_channel)
+    return false;
+
+  // Check to see if we should handle this function ourselves. We only handle
+  // kPostMessage.
+  if (IdentifierIs(name, kPostMessage) && (arg_count == 1)) {
+    message_channel->PostMessageToNative(&args[0]);
+    return true;
   }
   // Other method calls we will pass to the passthrough object, if we have one.
   NPObject* passthrough = ToPassThroughObject(np_obj);
@@ -215,7 +168,7 @@ bool MessageChannelGetProperty(NPObject* np_obj,
     return false;
 
   // Don't allow getting the postMessage function.
-  if (IdentifierIsPostMessage(name))
+  if (IdentifierIs(name, kPostMessage))
     return false;
 
   MessageChannel* message_channel = ToMessageChannel(np_obj);
@@ -238,7 +191,7 @@ bool MessageChannelSetProperty(NPObject* np_obj,
     return false;
 
   // Don't allow setting the postMessage function.
-  if (IdentifierIsPostMessage(name))
+  if (IdentifierIs(name, kPostMessage))
     return false;
 
   // Invoke on the passthrough object, if we have one.
@@ -294,12 +247,21 @@ NPClass message_channel_class = {
 
 // MessageChannel --------------------------------------------------------------
 struct MessageChannel::VarConversionResult {
-  VarConversionResult(const ppapi::ScopedPPVar& r, bool s)
-      : result(r), success(s), conversion_completed(true) {}
-  VarConversionResult() : success(false), conversion_completed(false) {}
-  ppapi::ScopedPPVar result;
-  bool success;
-  bool conversion_completed;
+  VarConversionResult() : success_(false), conversion_completed_(false) {}
+  void ConversionCompleted(const ScopedPPVar& var,
+                           bool success) {
+    conversion_completed_ = true;
+    var_ = var;
+    success_ = success;
+  }
+  const ScopedPPVar& var() const { return var_; }
+  bool success() const { return success_; }
+  bool conversion_completed() const { return conversion_completed_; }
+
+ private:
+  ScopedPPVar var_;
+  bool success_;
+  bool conversion_completed_;
 };
 
 MessageChannel::MessageChannelNPObject::MessageChannelNPObject() {}
@@ -321,61 +283,30 @@ MessageChannel::MessageChannel(PepperPluginInstanceImpl* instance)
   np_object_->message_channel = weak_ptr_factory_.GetWeakPtr();
 }
 
-void MessageChannel::NPVariantToPPVar(const NPVariant* variant) {
-  converted_var_queue_.push_back(VarConversionResult());
-  std::list<VarConversionResult>::iterator result_iterator =
-      --converted_var_queue_.end();
-  switch (variant->type) {
-    case NPVariantType_Void:
-      NPVariantToPPVarComplete(
-          result_iterator, ppapi::ScopedPPVar(PP_MakeUndefined()), true);
-      return;
-    case NPVariantType_Null:
-      NPVariantToPPVarComplete(
-          result_iterator, ppapi::ScopedPPVar(PP_MakeNull()), true);
-      return;
-    case NPVariantType_Bool:
-      NPVariantToPPVarComplete(result_iterator,
-                               ppapi::ScopedPPVar(PP_MakeBool(PP_FromBool(
-                                   NPVARIANT_TO_BOOLEAN(*variant)))),
-                               true);
-      return;
-    case NPVariantType_Int32:
-      NPVariantToPPVarComplete(
-          result_iterator,
-          ppapi::ScopedPPVar(PP_MakeInt32(NPVARIANT_TO_INT32(*variant))),
-          true);
-      return;
-    case NPVariantType_Double:
-      NPVariantToPPVarComplete(
-          result_iterator,
-          ppapi::ScopedPPVar(PP_MakeDouble(NPVARIANT_TO_DOUBLE(*variant))),
-          true);
-      return;
-    case NPVariantType_String:
-      NPVariantToPPVarComplete(
-          result_iterator,
-          ppapi::ScopedPPVar(ppapi::ScopedPPVar::PassRef(),
-                             StringVar::StringToPPVar(
-                                 NPVARIANT_TO_STRING(*variant).UTF8Characters,
-                                 NPVARIANT_TO_STRING(*variant).UTF8Length)),
-          true);
-      return;
-    case NPVariantType_Object: {
-      // Calling WebBindings::toV8Value creates a wrapper around NPVariant so it
-      // shouldn't result in a deep copy.
-      v8::Handle<v8::Value> v8_value = WebBindings::toV8Value(variant);
-      V8VarConverter(instance_->pp_instance())
-          .FromV8Value(v8_value,
-                       v8::Isolate::GetCurrent()->GetCurrentContext(),
-                       base::Bind(&MessageChannel::NPVariantToPPVarComplete,
-                                  weak_ptr_factory_.GetWeakPtr(),
-                                  result_iterator));
-      return;
-    }
+void MessageChannel::EnqueuePluginMessage(const NPVariant* variant) {
+  plugin_message_queue_.push_back(VarConversionResult());
+  if (variant->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.
+
+    // Calling WebBindings::toV8Value creates a wrapper around NPVariant so it
+    // won't result in a deep copy.
+    v8::Handle<v8::Value> v8_value = WebBindings::toV8Value(variant);
+    V8VarConverter v8_var_converter(instance_->pp_instance());
+    v8_var_converter.FromV8Value(
+        v8_value,
+        v8::Isolate::GetCurrent()->GetCurrentContext(),
+        base::Bind(&MessageChannel::FromV8ValueComplete,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   &plugin_message_queue_.back()));
+  } else {
+    plugin_message_queue_.back().ConversionCompleted(
+        ScopedPPVar(ScopedPPVar::PassRef(),
+                    NPVariantToPPVar(instance(), variant)),
+        true);
+    DCHECK(plugin_message_queue_.back().var().get().type != PP_VARTYPE_OBJECT);
   }
-  NPVariantToPPVarComplete(
-      result_iterator, ppapi::ScopedPPVar(PP_MakeUndefined()), false);
 }
 
 void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
@@ -409,82 +340,69 @@ void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
   WebSerializedScriptValue serialized_val =
       WebSerializedScriptValue::serialize(v8_val);
 
-  if (instance_->module()->IsProxied()) {
-    if (early_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);
-    } else {
-      // The proxy sent an asynchronous message, so the plugin is already
-      // unblocked. Therefore, there's no need to PostTask.
-      DCHECK(early_message_queue_.size() == 0);
-      PostMessageToJavaScriptImpl(serialized_val);
-    }
+  if (early_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);
   } else {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&MessageChannel::PostMessageToJavaScriptImpl,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   serialized_val));
+    // The proxy sent an asynchronous message, so the plugin is already
+    // unblocked. Therefore, there's no need to PostTask.
+    DCHECK(early_message_queue_.empty());
+    PostMessageToJavaScriptImpl(serialized_val);
   }
 }
 
-void MessageChannel::StopQueueingJavaScriptMessages() {
+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.
-  early_message_queue_state_ = DRAIN_PENDING;
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&MessageChannel::DrainEarlyMessageQueue,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void MessageChannel::QueueJavaScriptMessages() {
-  if (early_message_queue_state_ == DRAIN_PENDING)
-    early_message_queue_state_ = DRAIN_CANCELLED;
-  else
-    early_message_queue_state_ = QUEUE_MESSAGES;
+void MessageChannel::FromV8ValueComplete(VarConversionResult* result_holder,
+                                         const ScopedPPVar& result,
+                                         bool success) {
+  result_holder->ConversionCompleted(result, success);
+  DrainCompletedPluginMessages();
 }
 
-void MessageChannel::NPVariantToPPVarComplete(
-    const std::list<VarConversionResult>::iterator& result_iterator,
-    const ppapi::ScopedPPVar& result,
-    bool success) {
-  *result_iterator = VarConversionResult(result, success);
-  std::list<VarConversionResult>::iterator it = converted_var_queue_.begin();
-  while (it != converted_var_queue_.end() && it->conversion_completed) {
-    if (it->success) {
-      PostMessageToNative(it->result.get());
+void MessageChannel::DrainCompletedPluginMessages() {
+  if (early_message_queue_state_ == QUEUE_MESSAGES)
+    return;
+
+  while (!plugin_message_queue_.empty() &&
+         plugin_message_queue_.front().conversion_completed()) {
+    const VarConversionResult& front = plugin_message_queue_.front();
+    if (front.success()) {
+      instance_->HandleMessage(front.var());
     } else {
       PpapiGlobals::Get()->LogWithSource(instance()->pp_instance(),
                                          PP_LOGLEVEL_ERROR,
                                          std::string(),
                                          kV8ToVarConversionError);
     }
-
-    converted_var_queue_.erase(it++);
+    plugin_message_queue_.pop_front();
   }
 }
 
 void MessageChannel::DrainEarlyMessageQueue() {
+  DCHECK(early_message_queue_state_ == QUEUE_MESSAGES);
+
   // 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<PepperPluginInstanceImpl> instance_ref(instance_);
-
-  if (early_message_queue_state_ == DRAIN_CANCELLED) {
-    early_message_queue_state_ = QUEUE_MESSAGES;
-    return;
-  }
-  DCHECK(early_message_queue_state_ == DRAIN_PENDING);
-
   while (!early_message_queue_.empty()) {
     PostMessageToJavaScriptImpl(early_message_queue_.front());
     early_message_queue_.pop_front();
   }
   early_message_queue_state_ = SEND_DIRECTLY;
+
+  DrainCompletedPluginMessages();
 }
 
 void MessageChannel::PostMessageToJavaScriptImpl(
@@ -515,30 +433,12 @@ void MessageChannel::PostMessageToJavaScriptImpl(
   //     at least, postMessage on Workers does not provide the origin or source.
   //     TODO(dmichael):  Add origin if we change to a more iframe-like origin
   //                      policy (see crbug.com/81537)
-
   container->element().dispatchEvent(msg_event);
 }
 
-void MessageChannel::PostMessageToNative(PP_Var 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));
-
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&MessageChannel::PostMessageToNativeImpl,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   var_copy));
-  }
-}
-
-void MessageChannel::PostMessageToNativeImpl(PP_Var message_data) {
-  instance_->HandleMessage(message_data);
+void MessageChannel::PostMessageToNative(const NPVariant* message_data) {
+  EnqueuePluginMessage(message_data);
+  DrainCompletedPluginMessages();
 }
 
 MessageChannel::~MessageChannel() {
@@ -565,7 +465,7 @@ void MessageChannel::SetPassthroughObject(NPObject* passthrough) {
 
 bool MessageChannel::GetReadOnlyProperty(NPIdentifier key,
                                          NPVariant* value) const {
-  std::map<NPIdentifier, ppapi::ScopedPPVar>::const_iterator it =
+  std::map<NPIdentifier, ScopedPPVar>::const_iterator it =
       internal_properties_.find(key);
   if (it != internal_properties_.end()) {
     if (value)
@@ -576,7 +476,7 @@ bool MessageChannel::GetReadOnlyProperty(NPIdentifier key,
 }
 
 void MessageChannel::SetReadOnlyProperty(PP_Var key, PP_Var value) {
-  internal_properties_[PPVarToNPIdentifier(key)] = ppapi::ScopedPPVar(value);
+  internal_properties_[PPVarToNPIdentifier(key)] = ScopedPPVar(value);
 }
 
 }  // namespace content
diff --git a/content/renderer/pepper/message_channel.h b/content/renderer/pepper/message_channel.h
index 222d56c..077a68d 100644
--- a/content/renderer/pepper/message_channel.h
+++ b/content/renderer/pepper/message_channel.h
@@ -53,16 +53,13 @@ class MessageChannel {
   explicit MessageChannel(PepperPluginInstanceImpl* instance);
   ~MessageChannel();
 
-  // Converts an NPVariant to a PP_Var. This occurs asynchronously and
-  // NPVariantToPPVarComplete will be called upon completion.
-  void NPVariantToPPVar(const NPVariant* variant);
-
   // Post a message to the onmessage handler for this channel's instance
   // asynchronously.
   void PostMessageToJavaScript(PP_Var message_data);
-  // Post a message to the PPP_Instance HandleMessage function for this
-  // channel's instance.
-  void PostMessageToNative(PP_Var message_data);
+
+  // Post a message to the plugin's HandleMessage function for this channel's
+  // instance.
+  void PostMessageToNative(const NPVariant* message_data);
 
   // 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
@@ -75,11 +72,10 @@ class MessageChannel {
 
   PepperPluginInstanceImpl* instance() { return instance_; }
 
-  // Messages sent to JavaScript are queued by default. After the DOM is
-  // set up for the plugin, users of MessageChannel should call
-  // StopQueueingJavaScriptMessages to start dispatching messages to JavaScript.
-  void QueueJavaScriptMessages();
-  void StopQueueingJavaScriptMessages();
+  // Messages are queued initially. After the PepperPluginInstanceImpl is ready
+  // to send and handle messages, users of MessageChannel should call
+  // Start().
+  void Start();
 
   bool GetReadOnlyProperty(NPIdentifier key, NPVariant* value) const;
   void SetReadOnlyProperty(PP_Var key, PP_Var value);
@@ -88,13 +84,12 @@ class MessageChannel {
   // Struct for storing the result of a NPVariant being converted to a PP_Var.
   struct VarConversionResult;
 
-  // This is called when an NPVariant is finished being converted.
-  // |result_iteartor| is an iterator into |converted_var_queue_| where the
-  // result should be stored.
-  void NPVariantToPPVarComplete(
-      const std::list<VarConversionResult>::iterator& result_iterator,
-      const ppapi::ScopedPPVar& result,
-      bool success);
+  void EnqueuePluginMessage(const NPVariant* variant);
+
+  void FromV8ValueComplete(VarConversionResult* result_holder,
+                           const ppapi::ScopedPPVar& result_var,
+                           bool success);
+  void DrainCompletedPluginMessages();
 
   PepperPluginInstanceImpl* instance_;
 
@@ -118,22 +113,22 @@ class MessageChannel {
 
   void DrainEarlyMessageQueue();
 
-  // TODO(teravest): Remove all the tricky DRAIN_CANCELLED logic once
-  // PluginInstance::ResetAsProxied() is gone.
   std::deque<blink::WebSerializedScriptValue> early_message_queue_;
   enum EarlyMessageQueueState {
     QUEUE_MESSAGES,  // Queue JS messages.
     SEND_DIRECTLY,   // Post JS messages directly.
-    DRAIN_PENDING,   // Drain queue, then transition to DIRECT.
-    DRAIN_CANCELLED  // Preempt drain, go back to QUEUE.
   };
   EarlyMessageQueueState early_message_queue_state_;
 
-  // This queue stores vars that have been converted from NPVariants. Because
-  // conversion can happen asynchronously, the queue stores the var until all
-  // previous vars have been converted before calling PostMessage to ensure that
-  // the order in which messages are processed is preserved.
-  std::list<VarConversionResult> converted_var_queue_;
+  // This queue stores vars that are being sent to the plugin. Because
+  // conversion can happen asynchronously for object types, the queue stores
+  // the var until all previous vars have been converted and sent. This
+  // preserves the order in which JS->plugin messages are processed.
+  //
+  // Note we rely on raw VarConversionResult* pointers remaining valid after
+  // 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<VarConversionResult> plugin_message_queue_;
 
   std::map<NPIdentifier, ppapi::ScopedPPVar> internal_properties_;
 
diff --git a/content/renderer/pepper/mock_renderer_ppapi_host.cc b/content/renderer/pepper/mock_renderer_ppapi_host.cc
index da95d8f..0f7b6e8 100644
--- a/content/renderer/pepper/mock_renderer_ppapi_host.cc
+++ b/content/renderer/pepper/mock_renderer_ppapi_host.cc
@@ -79,6 +79,14 @@ IPC::PlatformFileForTransit MockRendererPpapiHost::ShareHandleWithRemote(
 
 bool MockRendererPpapiHost::IsRunningInProcess() const { return false; }
 
+std::string MockRendererPpapiHost::GetPluginName() const {
+  return std::string();
+}
+
+void MockRendererPpapiHost::SetToExternalPluginHost() {
+  NOTIMPLEMENTED();
+}
+
 void MockRendererPpapiHost::CreateBrowserResourceHosts(
     PP_Instance instance,
     const std::vector<IPC::Message>& nested_msgs,
diff --git a/content/renderer/pepper/mock_renderer_ppapi_host.h b/content/renderer/pepper/mock_renderer_ppapi_host.h
index f1ebbbc..62c9d65 100644
--- a/content/renderer/pepper/mock_renderer_ppapi_host.h
+++ b/content/renderer/pepper/mock_renderer_ppapi_host.h
@@ -52,6 +52,8 @@ class MockRendererPpapiHost : public RendererPpapiHost {
       base::PlatformFile handle,
       bool should_close_source) OVERRIDE;
   virtual bool IsRunningInProcess() const OVERRIDE;
+  virtual std::string GetPluginName() const OVERRIDE;
+  virtual void SetToExternalPluginHost() OVERRIDE;
   virtual void CreateBrowserResourceHosts(
       PP_Instance instance,
       const std::vector<IPC::Message>& nested_msgs,
diff --git a/content/renderer/pepper/pepper_plugin_instance_impl.cc b/content/renderer/pepper/pepper_plugin_instance_impl.cc
index c92e694..8f36f5d 100644
--- a/content/renderer/pepper/pepper_plugin_instance_impl.cc
+++ b/content/renderer/pepper/pepper_plugin_instance_impl.cc
@@ -80,6 +80,7 @@
 #include "ppapi/shared_impl/ppp_instance_combined.h"
 #include "ppapi/shared_impl/resource.h"
 #include "ppapi/shared_impl/scoped_pp_resource.h"
+#include "ppapi/shared_impl/scoped_pp_var.h"
 #include "ppapi/shared_impl/time_conversion.h"
 #include "ppapi/shared_impl/url_request_info_data.h"
 #include "ppapi/shared_impl/var.h"
@@ -146,6 +147,7 @@ using ppapi::PPB_View_Shared;
 using ppapi::PPP_Instance_Combined;
 using ppapi::Resource;
 using ppapi::ScopedPPResource;
+using ppapi::ScopedPPVar;
 using ppapi::StringVar;
 using ppapi::TrackedCallback;
 using ppapi::thunk::EnterResourceNoLock;
@@ -822,8 +824,14 @@ bool PepperPluginInstanceImpl::Initialize(
   scoped_ptr<const char * []> argv_array(StringVectorToArgArray(argv_));
   bool success = PP_ToBool(instance_interface_->DidCreate(
       pp_instance(), argn_.size(), argn_array.get(), argv_array.get()));
-  if (success)
-    message_channel_->StopQueueingJavaScriptMessages();
+  // If this is a plugin that hosts external plugins, we should delay messages
+  // so that the child plugin that's created later will receive all the
+  // messages. (E.g., NaCl trusted plugin starting a child NaCl app.)
+  //
+  // A host for external plugins will call ResetAsProxied later, at which point
+  // we can Start() the message_channel_.
+  if (success && (!module_->renderer_ppapi_host()->IsExternalPluginHost()))
+    message_channel_->Start();
   return success;
 }
 
@@ -1112,11 +1120,11 @@ bool PepperPluginInstanceImpl::HandleInputEvent(
   return rv;
 }
 
-void PepperPluginInstanceImpl::HandleMessage(PP_Var message) {
+void PepperPluginInstanceImpl::HandleMessage(ScopedPPVar message) {
   TRACE_EVENT0("ppapi", "PepperPluginInstanceImpl::HandleMessage");
   ppapi::proxy::HostDispatcher* dispatcher =
       ppapi::proxy::HostDispatcher::GetForInstance(pp_instance());
-  if (!dispatcher || (message.type == PP_VARTYPE_OBJECT)) {
+  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.
     NOTREACHED();
@@ -1125,7 +1133,7 @@ void PepperPluginInstanceImpl::HandleMessage(PP_Var message) {
   dispatcher->Send(new PpapiMsg_PPPMessaging_HandleMessage(
       ppapi::API_ID_PPP_MESSAGING,
       pp_instance(),
-      ppapi::proxy::SerializedVarSendInputShmem(dispatcher, message,
+      ppapi::proxy::SerializedVarSendInputShmem(dispatcher, message.get(),
                                                 pp_instance())));
 }
 
@@ -2737,9 +2745,6 @@ PP_ExternalPluginResult PepperPluginInstanceImpl::ResetAsProxied(
   original_module_ = module_;
   module_ = module;
 
-  // Don't send any messages to the plugin until DidCreate() has finished.
-  message_channel_->QueueJavaScriptMessages();
-
   // For NaCl instances, remember the NaCl plugin instance interface, so we
   // can shut it down by calling its DidDestroy in our Delete() method.
   original_instance_interface_.reset(instance_interface_.release());
@@ -2775,7 +2780,7 @@ PP_ExternalPluginResult PepperPluginInstanceImpl::ResetAsProxied(
   if (!instance_interface_->DidCreate(
           pp_instance(), argn_.size(), argn_array.get(), argv_array.get()))
     return PP_EXTERNAL_PLUGIN_ERROR_INSTANCE;
-  message_channel_->StopQueueingJavaScriptMessages();
+  message_channel_->Start();
 
   // Clear sent_initial_did_change_view_ and cancel any pending DidChangeView
   // event. This way, SendDidChangeView will send the "current" view
diff --git a/content/renderer/pepper/pepper_plugin_instance_impl.h b/content/renderer/pepper/pepper_plugin_instance_impl.h
index f504a6d..68771ef 100644
--- a/content/renderer/pepper/pepper_plugin_instance_impl.h
+++ b/content/renderer/pepper/pepper_plugin_instance_impl.h
@@ -89,6 +89,7 @@ namespace ppapi {
 class Resource;
 struct InputEventData;
 struct PPP_Instance_Combined;
+class ScopedPPVar;
 }
 
 namespace v8 {
@@ -296,8 +297,8 @@ class CONTENT_EXPORT PepperPluginInstanceImpl
   // already in fullscreen mode).
   bool SetFullscreen(bool fullscreen);
 
-  // Implementation of PPP_Messaging.
-  void HandleMessage(PP_Var message);
+  // Send the message on to the plugin.
+  void HandleMessage(ppapi::ScopedPPVar message);
 
   // Returns true if the plugin is processing a user gesture.
   bool IsProcessingUserGesture();
diff --git a/content/renderer/pepper/renderer_ppapi_host_impl.cc b/content/renderer/pepper/renderer_ppapi_host_impl.cc
index 22e7f46..850399a 100644
--- a/content/renderer/pepper/renderer_ppapi_host_impl.cc
+++ b/content/renderer/pepper/renderer_ppapi_host_impl.cc
@@ -41,7 +41,9 @@ RendererPpapiHostImpl::RendererPpapiHostImpl(
     PluginModule* module,
     ppapi::proxy::HostDispatcher* dispatcher,
     const ppapi::PpapiPermissions& permissions)
-    : module_(module), dispatcher_(dispatcher) {
+    : module_(module),
+      dispatcher_(dispatcher),
+      is_external_plugin_host_(false) {
   // Hook the PpapiHost up to the dispatcher for out-of-process communication.
   ppapi_host_.reset(new ppapi::host::PpapiHost(dispatcher, permissions));
   ppapi_host_->AddHostFactoryFilter(scoped_ptr<ppapi::host::HostFactory>(
@@ -54,7 +56,7 @@ RendererPpapiHostImpl::RendererPpapiHostImpl(
 RendererPpapiHostImpl::RendererPpapiHostImpl(
     PluginModule* module,
     const ppapi::PpapiPermissions& permissions)
-    : module_(module), dispatcher_(NULL) {
+    : module_(module), dispatcher_(NULL), is_external_plugin_host_(false) {
   // Hook the host up to the in-process router.
   in_process_router_.reset(new PepperInProcessRouter(this));
   ppapi_host_.reset(new ppapi::host::PpapiHost(
@@ -125,6 +127,10 @@ PepperPluginInstanceImpl* RendererPpapiHostImpl::GetPluginInstanceImpl(
   return GetAndValidateInstance(instance);
 }
 
+bool RendererPpapiHostImpl::IsExternalPluginHost() const {
+  return is_external_plugin_host_;
+}
+
 ppapi::host::PpapiHost* RendererPpapiHostImpl::GetPpapiHost() {
   return ppapi_host_.get();
 }
@@ -227,6 +233,14 @@ bool RendererPpapiHostImpl::IsRunningInProcess() const {
   return is_running_in_process_;
 }
 
+std::string RendererPpapiHostImpl::GetPluginName() const {
+  return module_->name();
+}
+
+void RendererPpapiHostImpl::SetToExternalPluginHost() {
+  is_external_plugin_host_ = true;
+}
+
 void RendererPpapiHostImpl::CreateBrowserResourceHosts(
     PP_Instance instance,
     const std::vector<IPC::Message>& nested_msgs,
diff --git a/content/renderer/pepper/renderer_ppapi_host_impl.h b/content/renderer/pepper/renderer_ppapi_host_impl.h
index 8da1411..a30735d 100644
--- a/content/renderer/pepper/renderer_ppapi_host_impl.h
+++ b/content/renderer/pepper/renderer_ppapi_host_impl.h
@@ -73,6 +73,8 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
 
   PepperPluginInstanceImpl* GetPluginInstanceImpl(PP_Instance instance) const;
 
+  bool IsExternalPluginHost() const;
+
   // RendererPpapiHost implementation.
   virtual ppapi::host::PpapiHost* GetPpapiHost() OVERRIDE;
   virtual bool IsValidInstance(PP_Instance instance) const OVERRIDE;
@@ -94,6 +96,8 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
       base::PlatformFile handle,
       bool should_close_source) OVERRIDE;
   virtual bool IsRunningInProcess() const OVERRIDE;
+  virtual std::string GetPluginName() const OVERRIDE;
+  virtual void SetToExternalPluginHost() OVERRIDE;
   virtual void CreateBrowserResourceHosts(
       PP_Instance instance,
       const std::vector<IPC::Message>& nested_msgs,
@@ -130,6 +134,9 @@ class RendererPpapiHostImpl : public RendererPpapiHost {
   // Whether the plugin is running in process.
   bool is_running_in_process_;
 
+  // Whether this is a host for external plugins.
+  bool is_external_plugin_host_;
+
   DISALLOW_COPY_AND_ASSIGN(RendererPpapiHostImpl);
 };
 
diff --git a/content/renderer/pepper/resource_converter.cc b/content/renderer/pepper/resource_converter.cc
index 62164cf..476e3d3 100644
--- a/content/renderer/pepper/resource_converter.cc
+++ b/content/renderer/pepper/resource_converter.cc
@@ -203,7 +203,7 @@ ResourceConverterImpl::ResourceConverterImpl(PP_Instance instance,
 ResourceConverterImpl::~ResourceConverterImpl() {
   // Verify Flush() was called.
   DCHECK(browser_host_create_messages_.empty());
-  DCHECK(browser_vars.empty());
+  DCHECK(browser_vars_.empty());
 }
 
 bool ResourceConverterImpl::FromV8Value(v8::Handle<v8::Object> val,
@@ -269,9 +269,9 @@ void ResourceConverterImpl::Flush(const base::Callback<void(bool)>& callback) {
   host_->CreateBrowserResourceHosts(
       instance_,
       browser_host_create_messages_,
-      base::Bind(&FlushComplete, callback, browser_vars));
+      base::Bind(&FlushComplete, callback, browser_vars_));
   browser_host_create_messages_.clear();
-  browser_vars.clear();
+  browser_vars_.clear();
 }
 
 bool ResourceConverterImpl::ToV8Value(const PP_Var& var,
@@ -336,7 +336,7 @@ ResourceConverterImpl::CreateResourceVarWithBrowserHost(
   scoped_refptr<HostResourceVar> result =
       CreateResourceVar(pending_renderer_id, create_message);
   browser_host_create_messages_.push_back(browser_host_create_message);
-  browser_vars.push_back(result);
+  browser_vars_.push_back(result);
   return result;
 }
 
diff --git a/content/renderer/pepper/resource_converter.h b/content/renderer/pepper/resource_converter.h
index d399cc5..5b46adf 100644
--- a/content/renderer/pepper/resource_converter.h
+++ b/content/renderer/pepper/resource_converter.h
@@ -95,7 +95,7 @@ class ResourceConverterImpl : public ResourceConverter {
   // conveniently passed to |CreateBrowserResourceHosts|.
   std::vector<IPC::Message> browser_host_create_messages_;
   // A list of the resource vars associated with browser hosts.
-  std::vector<scoped_refptr<HostResourceVar> > browser_vars;
+  std::vector<scoped_refptr<HostResourceVar> > browser_vars_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceConverterImpl);
 };
diff --git a/content/renderer/pepper/v8_var_converter.cc b/content/renderer/pepper/v8_var_converter.cc
index 6dc5d78..bc1a253 100644
--- a/content/renderer/pepper/v8_var_converter.cc
+++ b/content/renderer/pepper/v8_var_converter.cc
@@ -399,6 +399,20 @@ void V8VarConverter::FromV8Value(
     v8::Handle<v8::Value> val,
     v8::Handle<v8::Context> context,
     const base::Callback<void(const ScopedPPVar&, bool)>& callback) {
+  ScopedPPVar result_var;
+  if (FromV8ValueInternal(val, context, &result_var)) {
+    resource_converter_->Flush(base::Bind(callback, result_var));
+  } else {
+    message_loop_proxy_->PostTask(
+        FROM_HERE,
+        base::Bind(callback, result_var, false));
+  }
+}
+
+bool V8VarConverter::FromV8ValueInternal(
+    v8::Handle<v8::Value> val,
+    v8::Handle<v8::Context> context,
+    ppapi::ScopedPPVar* result_var) {
   v8::Context::Scope context_scope(context);
   v8::HandleScope handle_scope(context->GetIsolate());
 
@@ -408,6 +422,7 @@ void V8VarConverter::FromV8Value(
   std::stack<StackEntry<v8::Handle<v8::Value> > > stack;
   stack.push(StackEntry<v8::Handle<v8::Value> >(val));
   ScopedPPVar root;
+  *result_var = PP_MakeUndefined();
   bool is_root = true;
 
   while (!stack.empty()) {
@@ -431,10 +446,7 @@ void V8VarConverter::FromV8Value(
                         &visited_handles,
                         &parent_handles,
                         resource_converter_.get())) {
-      message_loop_proxy_->PostTask(
-          FROM_HERE,
-          base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-      return;
+      return false;
     }
 
     if (is_root) {
@@ -451,21 +463,14 @@ void V8VarConverter::FromV8Value(
       ArrayVar* array_var = ArrayVar::FromPPVar(current_var);
       if (!array_var) {
         NOTREACHED();
-        message_loop_proxy_->PostTask(
-            FROM_HERE,
-            base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-        return;
+        return false;
       }
 
       for (uint32 i = 0; i < v8_array->Length(); ++i) {
         v8::TryCatch try_catch;
         v8::Handle<v8::Value> child_v8 = v8_array->Get(i);
-        if (try_catch.HasCaught()) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
-        }
+        if (try_catch.HasCaught())
+          return false;
 
         if (!v8_array->HasRealIndexedProperty(i))
           continue;
@@ -478,10 +483,7 @@ void V8VarConverter::FromV8Value(
                             &visited_handles,
                             &parent_handles,
                             resource_converter_.get())) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
         if (did_create && child_v8->IsObject())
           stack.push(child_v8);
@@ -496,10 +498,7 @@ void V8VarConverter::FromV8Value(
       DictionaryVar* dict_var = DictionaryVar::FromPPVar(current_var);
       if (!dict_var) {
         NOTREACHED();
-        message_loop_proxy_->PostTask(
-            FROM_HERE,
-            base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-        return;
+        return false;
       }
 
       v8::Handle<v8::Array> property_names(v8_object->GetOwnPropertyNames());
@@ -511,10 +510,7 @@ void V8VarConverter::FromV8Value(
           NOTREACHED() << "Key \"" << *v8::String::Utf8Value(key)
                        << "\" "
                           "is neither a string nor a number";
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
 
         // Skip all callbacks: crbug.com/139933
@@ -525,12 +521,8 @@ void V8VarConverter::FromV8Value(
 
         v8::TryCatch try_catch;
         v8::Handle<v8::Value> child_v8 = v8_object->Get(key);
-        if (try_catch.HasCaught()) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
-        }
+        if (try_catch.HasCaught())
+          return false;
 
         PP_Var child_var;
         if (!GetOrCreateVar(child_v8,
@@ -540,10 +532,7 @@ void V8VarConverter::FromV8Value(
                             &visited_handles,
                             &parent_handles,
                             resource_converter_.get())) {
-          message_loop_proxy_->PostTask(
-              FROM_HERE,
-              base::Bind(callback, ScopedPPVar(PP_MakeUndefined()), false));
-          return;
+          return false;
         }
         if (did_create && child_v8->IsObject())
           stack.push(child_v8);
@@ -554,7 +543,8 @@ void V8VarConverter::FromV8Value(
       }
     }
   }
-  resource_converter_->Flush(base::Bind(callback, root));
+  *result_var = root;
+  return true;
 }
 
 }  // namespace content
diff --git a/content/renderer/pepper/v8_var_converter.h b/content/renderer/pepper/v8_var_converter.h
index 8bb22b7..3423023 100644
--- a/content/renderer/pepper/v8_var_converter.h
+++ b/content/renderer/pepper/v8_var_converter.h
@@ -46,8 +46,12 @@ class CONTENT_EXPORT V8VarConverter {
       v8::Handle<v8::Value> val,
       v8::Handle<v8::Context> context,
       const base::Callback<void(const ppapi::ScopedPPVar&, bool)>& callback);
-
  private:
+  // Returns true on success, false on failure.
+  bool FromV8ValueInternal(v8::Handle<v8::Value> val,
+                           v8::Handle<v8::Context> context,
+                           ppapi::ScopedPPVar* result_var);
+
   // The message loop to run the callback to |FromV8Value| from.
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;