Make PostMessageToJavaScript use new WebKit API instead of script.

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 caee008cfd14a9c74ae594029d8de13febcc81d9..a720b8b955aba2f49e4770527e296ef55589b2c0 100644
--- a/webkit/plugins/ppapi/message_channel.cc
+++ b/webkit/plugins/ppapi/message_channel.cc
@@ -12,15 +12,21 @@
 #include "base/message_loop.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebDOMMessageEvent.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebNode.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginContainer.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebSerializedScriptValue.h"
 #include "ppapi/shared_impl/var.h"
+#include "v8/include/v8.h"
 #include "webkit/plugins/ppapi/npapi_glue.h"
 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 using ppapi::StringVar;
 using WebKit::WebBindings;
+using WebKit::WebDOMMessageEvent;
+using WebKit::WebSerializedScriptValue;
 
 namespace webkit {
 
@@ -42,50 +48,46 @@ bool IdentifierIsPostMessage(NPIdentifier identifier) {
   return WebBindings::getStringIdentifier(kPostMessage) == identifier;
 }
 
-// Converts the given PP_Var to an NPVariant, returning true on success.
-// False means that the given variant is invalid. In this case, the result
-// NPVariant will be set to a void one.
-//
-// The contents of the PP_Var will NOT be copied, so you need to ensure that
-// the PP_Var remains valid while the resultant NPVariant is in use.
-//
-// Note:  This is largely copied from var.cc so that we don't depend on code
-//        which will be removed.  TODO(dmichael) remove this comment when var
-//        is removed.
-bool PPVarToNPVariantNoCopy(PP_Var var, NPVariant* result) {
+// Converts the given PP_Var to a v8::Value, returning true on success.
+// False means that the given variant is invalid. In this case, |result| will
+// be set to an empty handle.
+bool PPVarToV8Value(PP_Var var, v8::Handle<v8::Value>* result) {
   switch (var.type) {
     case PP_VARTYPE_UNDEFINED:
-      VOID_TO_NPVARIANT(*result);
+      *result = v8::Undefined();
       break;
     case PP_VARTYPE_NULL:
-      NULL_TO_NPVARIANT(*result);
+      *result = v8::Null();
       break;
     case PP_VARTYPE_BOOL:
-      BOOLEAN_TO_NPVARIANT(var.value.as_bool, *result);
+      *result = (var.value.as_bool == PP_TRUE) ? v8::True() : v8::False();
       break;
     case PP_VARTYPE_INT32:
-      INT32_TO_NPVARIANT(var.value.as_int, *result);
+      *result = v8::Integer::New(var.value.as_int);
       break;
     case PP_VARTYPE_DOUBLE:
-      DOUBLE_TO_NPVARIANT(var.value.as_double, *result);
+      *result = v8::Number::New(var.value.as_double);
       break;
     case PP_VARTYPE_STRING: {
       StringVar* string = StringVar::FromPPVar(var);
       if (!string) {
-        VOID_TO_NPVARIANT(*result);
+        result->Clear();
         return false;
       }
       const std::string& value = string->value();
-      STRINGN_TO_NPVARIANT(value.c_str(), value.size(), *result);
+      // TODO(dmichael): We should consider caching the V8 string in the host-
+      // side StringVar, so that we only have to convert/copy once if a
+      // string is sent more than once.
+      *result = v8::String::New(value.c_str(), value.size());
       break;
     }
     case PP_VARTYPE_OBJECT:
       // Objects are not currently supported.
       NOTIMPLEMENTED();
-      VOID_TO_NPVARIANT(*result);
+      result->Clear();
       return false;
     default:
-      VOID_TO_NPVARIANT(*result);
+      result->Clear();
       return false;
   }
   return true;
@@ -286,8 +288,6 @@ MessageChannel::MessageChannel(PluginInstance* instance)
       passthrough_object_(NULL),
       np_object_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
-  VOID_TO_NPVARIANT(onmessage_invoker_);
-
   // 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);
@@ -296,93 +296,50 @@ MessageChannel::MessageChannel(PluginInstance* instance)
   np_object_->message_channel = this;
 }
 
-bool MessageChannel::EvaluateOnMessageInvoker() {
-  // If we've already evaluated the function, just return.
-  if (NPVARIANT_IS_OBJECT(onmessage_invoker_))
-    return true;
-
-  // This is the javascript code that we invoke to create and dispatch a
-  // message event.
-  const char invoke_onmessage_js[] =
-      "(function(window, module_instance, message_data) {"
-      "  if (module_instance) {"
-      "    var message_event = new MessageEvent('message', "
-      "                                         { data: message_data });"
-      "    module_instance.dispatchEvent(message_event);"
-      "  }"
-      "})";
-  // Note that we purposely omit |origin| and |source|. The |origin| is only
-  // specified for cross-document and server-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)
-
-  NPString function_string = { invoke_onmessage_js,
-                               sizeof(invoke_onmessage_js)-1 };
-  // Get the current frame to pass to the evaluate function.
-  WebKit::WebFrame* frame =
-      instance_->container()->element().document().frame();
-  // Evaluate the function and obtain an NPVariant pointing to it.
-  if (!WebBindings::evaluate(NULL, frame->windowObject(), &function_string,
-                             &onmessage_invoker_)) {
-    // If it fails, do nothing.
-    return false;
+void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
+  // Serialize the message data.
+  v8::HandleScope scope;
+  v8::Handle<v8::Value> v8_val;
+  if (!PPVarToV8Value(message_data, &v8_val)) {
+    NOTREACHED();
+    return;
   }
-  DCHECK(NPVARIANT_IS_OBJECT(onmessage_invoker_));
-  return true;
-}
 
-void MessageChannel::PostMessageToJavaScript(PP_Var message_data) {
-  // Make a copy of the message data for the Task we will run.
-  PP_Var var_copy(CopyPPVar(message_data));
+  WebSerializedScriptValue serialized_val =
+      WebSerializedScriptValue::serialize(v8_val);
 
   MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&MessageChannel::PostMessageToJavaScriptImpl,
                  weak_ptr_factory_.GetWeakPtr(),
-                 var_copy));
+                 serialized_val));
 }
 
-void MessageChannel::PostMessageToJavaScriptImpl(PP_Var message_data) {
-  // Make sure we have our function for invoking onmessage on JavaScript.
-  bool success = EvaluateOnMessageInvoker();
-  DCHECK(success);
-  if (!success)
-    return;
-
+void MessageChannel::PostMessageToJavaScriptImpl(
+    const WebSerializedScriptValue& message_data) {
   DCHECK(instance_);
 
-  NPVariant result_var;
-  VOID_TO_NPVARIANT(result_var);
-  NPVariant npvariant_args[3];
-  // Get the frame so we can get the window object.
-  WebKit::WebFrame* frame =
-      instance_->container()->element().document().frame();
-  if (!frame)
-    return;
-
-  OBJECT_TO_NPVARIANT(frame->windowObject(), npvariant_args[0]);
-  OBJECT_TO_NPVARIANT(instance_->container()->scriptableObjectForElement(),
-                      npvariant_args[1]);
-  // Convert message to an NPVariant without copying. At this point, the data
-  // has already been copied.
-  if (!PPVarToNPVariantNoCopy(message_data, &npvariant_args[2])) {
-    // We couldn't create an NPVariant, so we can't invoke the method.  Thus,
-    // WebBindings::invokeDefault does not take ownership of these variants, so
-    // we must release our references to them explicitly.
-    WebBindings::releaseVariantValue(&npvariant_args[0]);
-    WebBindings::releaseVariantValue(&npvariant_args[1]);
-    return;
-  }
-
-  WebBindings::invokeDefault(NULL,
-                             NPVARIANT_TO_OBJECT(onmessage_invoker_),
-                             npvariant_args,
-                             sizeof(npvariant_args)/sizeof(*npvariant_args),
-                             &result_var);
+  const WebKit::WebElement& element = instance_->container()->element();

[piman, 2011/11/16 22:17:10]

container() can be NULL after the plugin has been removed from the DOM (but the
instance is not destroyed yet). I could see getting into that case if one sends
PostMessage from DidDestroyInstance. Well, to be safe, I think we should
(silently?) fail if container is NULL.

[dmichael (off chromium), 2011/11/16 23:17:31]

Done. Agree it should be silent if it's a real possibility.

It's a little scary to me if we can lose the container and still have the
PluginInstance; this is bound to bite us in other places. Here, I was careful to
ensure that if the PluginInstance goes away, its MessageChannel goes away, and
all tasks will not fire (due to using a WeakPtr in MessageChannel). I thought
that was 'safe enough'. :-(

[piman, 2011/11/16 23:37:26]

It is expected because of re-entrancy cases. I guess they may only happen with
synchronous scriting, but the case is real: browser calls plugin which calls
back browser to remove self from the DOM.
The instance can't go away because it's on the stack. But the container has to
go away because the plugin isn't in the DOM any more.
PluginInstance is ref-counted, so you can't have any control over its lifetime,
however the WebPluginContainer is not (and is reset in PluginInstance::Delete),
so unless you control all references to PluginInstance, you can't guarantee that
it won't outlive the container.

+
+  WebKit::WebDOMEvent event = element.document().createEvent("MessageEvent");

[piman, 2011/11/16 22:17:10]

For consistency, mind 'using' this and the other WebKit types?

[dmichael (off chromium), 2011/11/16 23:17:31]

Done.

+  WebDOMMessageEvent& msg_event(static_cast<WebDOMMessageEvent&>(event));

[piman, 2011/11/16 22:17:10]

WebDOMMessageEvent msg_event = event.to<WebDOMMessageEvent>()
At the C++ level, event is not a WebDOMMessageEvent, so that cast is technically
invalid (even though it works, it is fragile to changes to WebDOMMessageEvent).

[dmichael (off chromium), 2011/11/16 23:17:31]

Thanks for catching that. I felt a little dirty when I wrote that line, but I
knew it would work and oddly didn't notice the to<> function.

Done.

+  msg_event.initMessageEvent("message",  // type
+                             false,  // canBubble
+                             false,  // cancelable
+                             message_data,  // data
+                             "",  // origin [*]
+                             NULL,  // source [*]
+                             "");  // lastEventId
+  // [*] Note that the |origin| is only specified for cross-document and server-
+  //     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)
+
+  instance_->container()->element().dispatchEvent(msg_event);
 }
 
 void MessageChannel::PostMessageToNative(PP_Var message_data) {
@@ -403,7 +360,6 @@ MessageChannel::~MessageChannel() {
   WebBindings::releaseObject(np_object_);
   if (passthrough_object_)
     WebBindings::releaseObject(passthrough_object_);
-  WebBindings::releaseVariantValue(&onmessage_invoker_);
 }
 
 void MessageChannel::SetPassthroughObject(NPObject* passthrough) {