Replace NPObject usage in ppapi with gin

content/renderer/pepper/message_channel.cc

 // Copyright (c) 2012 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 "content/renderer/pepper/message_channel.h"
 
 #include <cstdlib>
 #include <string>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "content/public/common/content_client.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/renderer/pepper/host_array_buffer_var.h"
-#include "content/renderer/pepper/npapi_glue.h"
 #include "content/renderer/pepper/pepper_plugin_instance_impl.h"
+#include "content/renderer/pepper/pepper_try_catch.h"
 #include "content/renderer/pepper/plugin_module.h"
+#include "content/renderer/pepper/plugin_object.h"
 #include "content/renderer/pepper/v8_var_converter.h"
+#include "gin/arguments.h"
+#include "gin/converter.h"
+#include "gin/function_template.h"
+#include "gin/object_template_builder.h"
+#include "gin/public/gin_embedders.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/scoped_pp_var.h"
 #include "ppapi/shared_impl/var.h"
 #include "ppapi/shared_impl/var_tracker.h"
 #include "third_party/WebKit/public/web/WebBindings.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebDOMMessageEvent.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebNode.h"
@@ skipping 20 matching lines @@
 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 "
     "an unsupported type.";
 const char kVarToV8ConversionError[] =
     "Failed to convert a PostMessage "
     "argument from a PP_Var to a Javascript value. It may have cycles or be of "
     "an unsupported type.";
 
-// Helper function to get the MessageChannel that is associated with an
-// NPObject*.
-MessageChannel* ToMessageChannel(NPObject* object) {
-  return static_cast<MessageChannel::MessageChannelNPObject*>(object)
-      ->message_channel.get();
-}
-
-NPObject* ToPassThroughObject(NPObject* object) {
-  MessageChannel* channel = ToMessageChannel(object);
-  return channel ? channel->passthrough_object() : NULL;
-}
-
-// Return true iff |identifier| is equal to |string|.
-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;
+bool HasDevPermission() {
   return GetContentClient()->renderer()->IsPluginAllowedToUseDevChannelAPIs();
 }
 
-//------------------------------------------------------------------------------
-// Implementations of NPClass functions.  These are here to:
-// - Implement postMessage behavior.
-// - Forward calls to the 'passthrough' object to allow backwards-compatibility
-//   with GetInstanceObject() objects.
-//------------------------------------------------------------------------------
-NPObject* MessageChannelAllocate(NPP npp, NPClass* the_class) {
-  return new MessageChannel::MessageChannelNPObject;
-}
-
-void MessageChannelDeallocate(NPObject* object) {
-  MessageChannel::MessageChannelNPObject* instance =
-      static_cast<MessageChannel::MessageChannelNPObject*>(object);
-  delete instance;
-}
-
-bool MessageChannelHasMethod(NPObject* np_obj, NPIdentifier name) {
-  if (!np_obj)
-    return false;
-
-  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)
-    return WebBindings::hasMethod(NULL, passthrough, name);
-  return false;
-}
-
-bool MessageChannelInvoke(NPObject* np_obj,
-                          NPIdentifier name,
-                          const NPVariant* args,
-                          uint32 arg_count,
-                          NPVariant* result) {
-  if (!np_obj)
-    return false;
-
-  MessageChannel* message_channel = ToMessageChannel(np_obj);
-  if (!message_channel)
-    return false;
-
-  // 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) {
-    return WebBindings::invoke(
-        NULL, passthrough, name, args, arg_count, result);
-  }
-  return false;
-}
-
-bool MessageChannelInvokeDefault(NPObject* np_obj,
-                                 const NPVariant* args,
-                                 uint32 arg_count,
-                                 NPVariant* result) {
-  if (!np_obj)
-    return false;
-
-  // Invoke on the passthrough object, if we have one.
-  NPObject* passthrough = ToPassThroughObject(np_obj);
-  if (passthrough) {
-    return WebBindings::invokeDefault(
-        NULL, passthrough, args, arg_count, result);
-  }
-  return false;
-}
-
-bool MessageChannelHasProperty(NPObject* np_obj, NPIdentifier name) {
-  if (!np_obj)
-    return false;
-
-  MessageChannel* message_channel = ToMessageChannel(np_obj);
-  if (message_channel) {
-    if (message_channel->GetReadOnlyProperty(name, NULL))
-      return true;
-  }
-
-  // Invoke on the passthrough object, if we have one.
-  NPObject* passthrough = ToPassThroughObject(np_obj);
-  if (passthrough)
-    return WebBindings::hasProperty(NULL, passthrough, name);
-  return false;
-}
-
-bool MessageChannelGetProperty(NPObject* np_obj,
-                               NPIdentifier name,
-                               NPVariant* result) {
-  if (!np_obj)
-    return false;
-
-  // 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))
-      return true;
-  }
-
-  // Invoke on the passthrough object, if we have one.
-  NPObject* passthrough = ToPassThroughObject(np_obj);
-  if (passthrough)
-    return WebBindings::getProperty(NULL, passthrough, name, result);
-  return false;
-}
-
-bool MessageChannelSetProperty(NPObject* np_obj,
-                               NPIdentifier name,
-                               const NPVariant* variant) {
-  if (!np_obj)
-    return false;
-
-  // 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)
-    return WebBindings::setProperty(NULL, passthrough, name, variant);
-  return false;
-}
-
-bool MessageChannelEnumerate(NPObject* np_obj,
-                             NPIdentifier** value,
-                             uint32_t* count) {
-  if (!np_obj)
-    return false;
-
-  // Invoke on the passthrough object, if we have one, to enumerate its
-  // properties.
-  NPObject* passthrough = ToPassThroughObject(np_obj);
-  if (passthrough) {
-    bool success = WebBindings::enumerate(NULL, passthrough, value, count);
-    if (success) {
-      // Add postMessage to the list and return it.
-      if (std::numeric_limits<size_t>::max() / sizeof(NPIdentifier) <=
-          static_cast<size_t>(*count) + 1)  // Else, "always false" x64 warning.
-        return false;
-      NPIdentifier* new_array = static_cast<NPIdentifier*>(
-          std::malloc(sizeof(NPIdentifier) * (*count + 1)));
-      std::memcpy(new_array, *value, sizeof(NPIdentifier) * (*count));
-      new_array[*count] = WebBindings::getStringIdentifier(kPostMessage);
-      std::free(*value);
-      *value = new_array;
-      ++(*count);
-      return true;
-    }
-  }
-
-  // Otherwise, build an array that includes only postMessage.
-  *value = static_cast<NPIdentifier*>(malloc(sizeof(NPIdentifier)));
-  (*value)[0] = WebBindings::getStringIdentifier(kPostMessage);
-  *count = 1;
-  return true;
-}
-
-NPClass message_channel_class = {
-    NP_CLASS_STRUCT_VERSION,      &MessageChannelAllocate,
-    &MessageChannelDeallocate,    NULL,
-    &MessageChannelHasMethod,     &MessageChannelInvoke,
-    &MessageChannelInvokeDefault, &MessageChannelHasProperty,
-    &MessageChannelGetProperty,   &MessageChannelSetProperty,
-    NULL,                         &MessageChannelEnumerate, };
-
 }  // namespace
 
 // MessageChannel --------------------------------------------------------------
 struct MessageChannel::VarConversionResult {
   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() {}
+// static
+gin::WrapperInfo MessageChannel::kWrapperInfo = {gin::kEmbedderNativeGin};
 
-MessageChannel::MessageChannelNPObject::~MessageChannelNPObject() {}
-
-MessageChannel::MessageChannel(PepperPluginInstanceImpl* instance)
-    : instance_(instance),
-      passthrough_object_(NULL),
-      np_object_(NULL),
-      early_message_queue_state_(QUEUE_MESSAGES),
-      weak_ptr_factory_(this) {
-  // 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(instance_->instanceNPP(),
-                                            &message_channel_class);
-  DCHECK(obj);
-  np_object_ = static_cast<MessageChannel::MessageChannelNPObject*>(obj);
-  np_object_->message_channel = weak_ptr_factory_.GetWeakPtr();
+// static
+void MessageChannel::Create(PepperPluginInstanceImpl* instance,
+                            v8::Persistent<v8::Object>* result) {
+  v8::HandleScope handle_scope(instance->GetIsolate());
+  v8::Context::Scope context_scope(instance->GetContext());
+  gin::Handle<MessageChannel> message_channel =
+      gin::CreateHandle(instance->GetIsolate(), new MessageChannel(instance));
+  result->Reset(instance->GetIsolate(), message_channel.ToV8()->ToObject());
 }
 
 MessageChannel::~MessageChannel() {
-  WebBindings::releaseObject(np_object_);
-  if (passthrough_object_)
-    WebBindings::releaseObject(passthrough_object_);
+  passthrough_object_.Reset();
 }
 
-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()));
+v8::Local<v8::Value> MessageChannel::GetNamedProperty(
+    v8::Isolate* isolate,
+    const std::string& identifier) {
+  PepperTryCatchV8 try_catch(instance_, false, isolate);
+  if (identifier == kPostMessage) {
+    return gin::CreateFunctionTemplate(isolate,
+        base::Bind(&MessageChannel::PostMessageToNative,
+                   weak_ptr_factory_.GetWeakPtr()))->GetFunction();
+  } else if (identifier == kPostMessageAndAwaitResponse && HasDevPermission()) {
+    return gin::CreateFunctionTemplate(isolate,
+        base::Bind(&MessageChannel::PostBlockingMessageToNative,
+                   weak_ptr_factory_.GetWeakPtr()))->GetFunction();
+  }
+
+  std::map<std::string, ScopedPPVar>::const_iterator it =
+      internal_named_properties_.find(identifier);
+  if (it != internal_named_properties_.end()) {
+    v8::Handle<v8::Value> result = try_catch.ToV8(it->second.get());
+    if (try_catch.ThrowException())
+      return v8::Local<v8::Value>();
+    return result;
+  }
+
+  PluginObject* plugin_object = GetPluginObject(isolate);
+  if (plugin_object)
+    return plugin_object->GetNamedProperty(isolate, identifier);
+  return v8::Local<v8::Value>();
 }
 
-void MessageChannel::SetPassthroughObject(NPObject* passthrough) {
-  // Retain the passthrough object; We need to ensure it lives as long as this
-  // MessageChannel.
-  if (passthrough)
-    WebBindings::retainObject(passthrough);
-
-  // If we had a passthrough set already, release it. Note that we retain the
-  // incoming passthrough object first, so that we behave correctly if anyone
-  // invokes:
-  //   SetPassthroughObject(passthrough_object());
-  if (passthrough_object_)
-    WebBindings::releaseObject(passthrough_object_);
-
-  passthrough_object_ = passthrough;
-}
-
-bool MessageChannel::GetReadOnlyProperty(NPIdentifier key,
-                                         NPVariant* value) const {
-  std::map<NPIdentifier, ScopedPPVar>::const_iterator it =
-      internal_properties_.find(key);
-  if (it != internal_properties_.end()) {
-    if (value)
-      return PPVarToNPVariant(it->second.get(), value);
+bool MessageChannel::SetNamedProperty(v8::Isolate* isolate,
+                                      const std::string& identifier,
+                                      v8::Local<v8::Value> value) {
+  PepperTryCatchV8 try_catch(instance_, false, isolate);
+  if (identifier == kPostMessage ||
+      (identifier == kPostMessageAndAwaitResponse && HasDevPermission())) {
+    try_catch.ThrowException("Cannot set properties with the name postMessage"
+                             "or postMessageAndAwaitResponse");
     return true;
   }
+

[dmichael (off chromium), 2014/08/21 22:24:24]

Might be good to add a comment w/TODO...
// We don't forward this to the passthrough object; no plugins use
// that feature.
// TODO(raymes): Remove SetProperty support from PPP_Class.

[raymes, 2014/08/22 08:28:40]

Done.

   return false;
 }
 
-void MessageChannel::SetReadOnlyProperty(PP_Var key, PP_Var value) {
-  internal_properties_[PPVarToNPIdentifier(key)] = ScopedPPVar(value);
+std::vector<std::string> MessageChannel::EnumerateNamedProperties(
+    v8::Isolate* isolate) {
+  std::vector<std::string> result;
+  PluginObject* plugin_object = GetPluginObject(isolate);
+  if (plugin_object)
+    result = plugin_object->EnumerateNamedProperties(isolate);
+  result.push_back(kPostMessage);
+  if (HasDevPermission())
+    result.push_back(kPostMessageAndAwaitResponse);
+  return result;
+}
+
+v8::Local<v8::Value> MessageChannel::GetIndexedProperty(v8::Isolate* isolate,
+                                                        uint32_t index) {
+  PepperTryCatchV8 try_catch(instance_, false, isolate);
+  std::map<uint32_t, ScopedPPVar>::const_iterator it =
+      internal_indexed_properties_.find(index);

[dmichael (off chromium), 2014/08/21 22:24:24]

The "SetReadonlyProperty" stuff is only used by the NaCl integration, and it
only uses string keys. You can get rid of internal_indexed_properties_ and
DCHECK or NOTREACHED if the key is not a string in SetReadOnlyProperty.

(It would probably make sense to change that API to use std::string, but Justin
or I can do that later; you don't have to mess with it here).

[raymes, 2014/08/22 08:28:40]

Thank you! Done.

+  if (it != internal_indexed_properties_.end()) {
+    v8::Handle<v8::Value> result = try_catch.ToV8(it->second.get());
+    if (try_catch.ThrowException())
+      return v8::Local<v8::Value>();
+    return result;
+  }
+
+  PluginObject* plugin_object = GetPluginObject(isolate);
+  if (plugin_object)
+    return plugin_object->GetIndexedProperty(isolate, index);
+  return v8::Local<v8::Value>();
+}
+
+std::vector<uint32_t> MessageChannel::EnumerateIndexedProperties(
+    v8::Isolate* isolate) {
+  PluginObject* plugin_object = GetPluginObject(isolate);
+  if (plugin_object)
+    return plugin_object->EnumerateIndexedProperties(isolate);
+  return std::vector<uint32_t>();
 }
 
 void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
   v8::HandleScope scope(v8::Isolate::GetCurrent());
 
   // Because V8 is probably not on the stack for Native->JS calls, we need to
   // enter the appropriate context for the plugin.
-  WebPluginContainer* container = instance_->container();
-  // It's possible that container() is NULL if the plugin has been removed from
-  // the DOM (but the PluginInstance is not destroyed yet).
-  if (!container)
+  v8::Local<v8::Context> context = instance_->GetContext();
+  if (context.IsEmpty())
     return;
 
-  v8::Local<v8::Context> context =
-      container->element().document().frame()->mainWorldScriptContext();
-  // If the page is being destroyed, the context may be empty.
-  if (context.IsEmpty())
-    return;
   v8::Context::Scope context_scope(context);
 
   v8::Handle<v8::Value> v8_val;
+  // TODO(raymes): Possibly change this to use TryCatch to do the conversion and
+  // throw an exception if necessary.

[dmichael (off chromium), 2014/08/21 22:24:24]

We get here as a result of async IPC from the plugin; there's no place for the
exception to go. You can add a comment to that effect if you like, but there's
no need for a TODO.

[raymes, 2014/08/22 08:28:40]

Ah...I forgot about this. Done!

   if (!V8VarConverter(instance_->pp_instance())
            .ToV8Value(message_data, context, &v8_val)) {
     PpapiGlobals::Get()->LogWithSource(instance_->pp_instance(),
                                        PP_LOGLEVEL_ERROR,
                                        std::string(),
                                        kVarToV8ConversionError);
     return;
   }
 
   WebSerializedScriptValue serialized_val =
       WebSerializedScriptValue::serialize(v8_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 {
     // 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::PostMessageToNative(const NPVariant* message_data) {
+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()));
+}
+
+void MessageChannel::SetPassthroughObject(v8::Handle<v8::Object> passthrough) {
+  passthrough_object_.Reset(instance_->GetIsolate(), passthrough);
+}
+
+void MessageChannel::SetReadOnlyProperty(PP_Var key, PP_Var value) {
+  StringVar* key_string = StringVar::FromPPVar(key);
+  if (key.type == PP_VARTYPE_INT32 && key.value.as_int >= 0) {
+    internal_indexed_properties_[key.value.as_int] = ScopedPPVar(value);
+  } else if (key_string) {
+    internal_named_properties_[key_string->value()] = ScopedPPVar(value);
+  } else {
+    NOTREACHED();
+  }
+}
+
+MessageChannel::MessageChannel(PepperPluginInstanceImpl* instance)
+    : gin::NamedPropertyInterceptor(instance->GetIsolate(), this),
+      gin::IndexedPropertyInterceptor(instance->GetIsolate(), this),
+      instance_(instance),
+      early_message_queue_state_(QUEUE_MESSAGES),
+      weak_ptr_factory_(this) {
+}
+
+gin::ObjectTemplateBuilder MessageChannel::GetObjectTemplateBuilder(
+    v8::Isolate* isolate) {
+  return Wrappable<MessageChannel>::GetObjectTemplateBuilder(isolate)
+      .AddNamedPropertyInterceptor()
+      .AddIndexedPropertyInterceptor();
+}
+
+void MessageChannel::PostMessageToNative(gin::Arguments* args) {
+  if (args->Length() != 1) {
+    // TODO(raymes): Consider throwing an exception here. We don't now for
+    // backward compatibility.
+    return;
+  }
+
+  v8::Handle<v8::Value> message_data;
+  if (!args->GetNext(&message_data)) {
+    NOTREACHED();
+  }
+
   EnqueuePluginMessage(message_data);
   DrainCompletedPluginMessages();
 }
 
-void MessageChannel::PostBlockingMessageToNative(const NPVariant* message_data,
-                                                 NPVariant* np_result) {
+void MessageChannel::PostBlockingMessageToNative(gin::Arguments* args) {
+  PepperTryCatchV8 try_catch(instance_, false, args->isolate());
+  if (args->Length() != 1) {
+    try_catch.ThrowException(
+        "postMessageAndAwaitResponse requires one argument");
+    return;
+  }
+
+  v8::Handle<v8::Value> message_data;
+  if (!args->GetNext(&message_data)) {
+    NOTREACHED();
+  }
+
   if (early_message_queue_state_ == QUEUE_MESSAGES) {
-    WebBindings::setException(
-        np_object_,
+    try_catch.ThrowException(
         "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_,
+    try_catch.ThrowException(
         "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 param = try_catch.FromV8(message_data);

[dmichael (off chromium), 2014/08/21 22:24:24]

It feels kind of wrong that the "convert_objects" parameter is so far away from
the place where it's meaningful. I don't think it's obvious to a reader at all
what "true" means up there, and it doesn't make it obvious at this point that we
don't convert "Object" PP_Vars.

WDYT of moving the param to "FromV8" and "ToV8" instead of having it in the
constructor? The code might then look like...
// Messaging doesn't support "Object" PP_Vars.
const bool kConvertObjects = false;
ScopedPPVar param = try_catch.FromV8(message_data, kConvertObjects);

[raymes, 2014/08/22 08:28:40]

I think you're right, it is quite confusing. At the same time it makes the code
a bit messier at the conversion sites. How about this compromise:

-I removed the parameter altogether from PepperTryCatchVar because it was always
being set to true.
-I changed the constructor param to be the enum you suggested in V8VarConverter.

I understand your argument though, so let me know if you still prefer me to push
it out of the constructor :)

[dmichael (off chromium), 2014/08/22 16:28:46]

This works for me, seems like a good compromise. It's pretty understandable now,
and moving it to the conversion calls would be unnecessarily verbose.

+  if (try_catch.ThrowException())
+    return;
+
   ScopedPPVar pp_result;
   bool was_handled = instance_->HandleBlockingMessage(param, &pp_result);
   if (!was_handled) {
-    WebBindings::setException(
-        np_object_,
+    try_catch.ThrowException(
         "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.");
+  v8::Handle<v8::Value> v8_result = try_catch.ToV8(pp_result.get());
+  if (try_catch.ThrowException())
     return;
-  }
-  // Success! Convert the result to an NPVariant.
-  WebBindings::toNPVariant(v8_val, NULL, np_result);
+
+  args->Return(v8_result);
 }
 
 void MessageChannel::PostMessageToJavaScriptImpl(
     const WebSerializedScriptValue& message_data) {
   DCHECK(instance_);
 
   WebPluginContainer* container = instance_->container();
   // It's possible that container() is NULL if the plugin has been removed from
   // the DOM (but the PluginInstance is not destroyed yet).
   if (!container)
@@ skipping 13 matching lines @@
   //     sent messages, while |source| is only specified for cross-document
   //     messages:
   //      http://www.whatwg.org/specs/web-apps/current-work/multipage/comms.html
   //     This currently behaves like Web Workers. On Firefox, Chrome, and Safari
   //     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::EnqueuePluginMessage(const NPVariant* variant) {
+PluginObject* MessageChannel::GetPluginObject(v8::Isolate* isolate) {
+  return PluginObject::FromV8Object(isolate,
+      v8::Local<v8::Object>::New(isolate, passthrough_object_));
+}
+
+void MessageChannel::EnqueuePluginMessage(v8::Handle<v8::Value> v8_value) {
   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());
-    V8VarConverter::VarResult conversion_result =
-        v8_var_converter.FromV8Value(
-            v8_value,
-            v8::Isolate::GetCurrent()->GetCurrentContext(),
-            base::Bind(&MessageChannel::FromV8ValueComplete,
-                       weak_ptr_factory_.GetWeakPtr(),
-                       &plugin_message_queue_.back()));
-    if (conversion_result.completed_synchronously) {
-      plugin_message_queue_.back().ConversionCompleted(
-          conversion_result.var,
-          conversion_result.success);
-    }
-  } else {
+  // 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.
+  // TODO(raymes): Possibly change this to use TryCatch to do the conversion and
+  // throw an exception if necessary.
+  V8VarConverter v8_var_converter(instance_->pp_instance());
+  V8VarConverter::VarResult conversion_result =
+      v8_var_converter.FromV8Value(

[dmichael (off chromium), 2014/08/21 22:24:24]

Looking at this code, it wasn't clear to me what told the V8VarConverter not to
make a Object PP_Var. You could use an enum and change it up to look like:
V8VarConverter v8_var_conterter(instance_->pp_instance(),
                                V8VarConverter::kDisallowObjectVars);
vs.
V8VarConverter v8_var_conterter(instance_->pp_instance(),
                                V8VarConverter::kAllowObjectVars);

The overloaded constructor, where one has a bool param and the other effectively
defaults the bool to false is kind of confusing and seems counter to usual
Google/Chromium style.

[raymes, 2014/08/22 08:28:40]

I agree, I like your suggestion I think it's much clearer. Thanks!

+          v8_value,
+          v8::Isolate::GetCurrent()->GetCurrentContext(),
+          base::Bind(&MessageChannel::FromV8ValueComplete,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     &plugin_message_queue_.back()));
+  if (conversion_result.completed_synchronously) {
     plugin_message_queue_.back().ConversionCompleted(
-        ScopedPPVar(ScopedPPVar::PassRef(),
-                    NPVariantToPPVar(instance(), variant)),
-        true);
-    DCHECK(plugin_message_queue_.back().var().get().type != PP_VARTYPE_OBJECT);
+        conversion_result.var,
+        conversion_result.success);
   }
 }
 
 void MessageChannel::FromV8ValueComplete(VarConversionResult* result_holder,
                                          const ScopedPPVar& result,
                                          bool success) {
   result_holder->ConversionCompleted(result, success);
   DrainCompletedPluginMessages();
 }
 
@@ skipping 26 matching lines @@
   while (!early_message_queue_.empty()) {
     PostMessageToJavaScriptImpl(early_message_queue_.front());
     early_message_queue_.pop_front();
   }
   early_message_queue_state_ = SEND_DIRECTLY;
 
   DrainCompletedPluginMessages();
 }
 
 }  // namespace content