Fix event dispatching for PPB_Messaging::PostMessage. I asked Adam about the "plugin.onmessage = function(x){...}" style vs the 'plugin.addEventListener' style, and it seems that we should prefer supporting the addEventListener style.

This breaks the ability to do "plugin.onmessage =" :-(. I expected dispatchEvent to do it for me, but it does not. To add that support (if desired), I could work on making dispatchEvent do it, or I could fake it out via the script in MessageChannel (Basically have the old path of invoking onmessage directly, plus the new dispatchEvent code). Brett, please focus on PPAPI-specific stuff. Adam, please focus on my event code (especially message_channel.cc and the JavaScript in test_post_message.cc). BUG=None TEST=test_post_message.cc Review URL: http://codereview.chromium.org/6901060 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85348 0039d316-1c4b-4281-b951-d872f2087c98
author: dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2011-05-13 23:11:54 +0000
committer: dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2011-05-13 23:11:54 +0000
commit: 8585fd865df3044b8215e4486698422562712e6c (patch)
tree: f6f2b49818fac61ed3039dc9aa1402b9647c7fed
parent: 5e8d27087b478de2d7136fa9016c4e178e19305e (diff)
download: chromium_src-8585fd865df3044b8215e4486698422562712e6c.zip
chromium_src-8585fd865df3044b8215e4486698422562712e6c.tar.gz
chromium_src-8585fd865df3044b8215e4486698422562712e6c.tar.bz2
5 files changed, 176 insertions, 85 deletions
diff --git a/ppapi/c/ppb_messaging.h b/ppapi/c/ppb_messaging.h
index 0fd1c97..e438346 100644
--- a/ppapi/c/ppb_messaging.h
+++ b/ppapi/c/ppb_messaging.h
@@ -15,7 +15,7 @@
  * @file
  * This file defines the PPB_Messaging interface implemented by the browser.
  * The PPB_Messaging interface contains pointers to functions related to
- * sending messages to the JavaScript onmessage handler on the DOM element
+ * sending messages to JavaScript message event listeners on the DOM element
  * associated with a specific module instance.
  *
  * @addtogroup Interfaces
@@ -24,23 +24,25 @@
 
 /**
  * The PPB_Messaging interface contains pointers to functions related to
- * sending messages to the JavaScript onmessage handler on the DOM element
+ * sending messages to JavaScript message event listeners on the DOM element
  * associated with a specific module instance.
  */
 struct PPB_Messaging {
   /**
-   * @a PostMessage is a pointer to a function which asynchronously invokes the
-   * onmessage handler on the DOM element for the given module instance, if one
-   * exists. This means that a call to @a PostMessage will not block while the
+   * @a PostMessage is a pointer to a function which asynchronously invokes any
+   * listeners for message events on the DOM element for the given module
+   * instance. This means that a call to @a PostMessage will not block while the
    * message is processed.
    *
    * @param message is a PP_Var containing the data to be sent to JavaScript.
    * Currently, it can have an int32_t, double, bool, or string value (objects
    * are not supported.)
    *
-   * The onmessage handler in JavaScript code will receive an object conforming
-   * to the MessageEvent interface. In particular, the value of @a message will
-   * be contained as a property called @a data in the received MessageEvent.
+   * Listeners for message events in JavaScript code will receive an object
+   * conforming to the MessageEvent interface. In particular, the value of @a
+   * message will be contained as a property called @a data in the received
+   * MessageEvent.
+   *
    * This is analogous to listening for messages from Web Workers.
    *
    * See:
@@ -54,9 +56,10 @@ struct PPB_Messaging {
    *   <object id="plugin"
    *           type="application/x-ppapi-postMessage-example"/>
    *   <script type="text/javascript">
-   *     document.getElementById('plugin').onmessage = function(message) {
-   *       alert(message.data);
-   *     }
+   *     var plugin = document.getElementById('plugin');
+   *     plugin.AddEventListener("message",
+   *                             function(message) { alert(message.data); },
+   *                             false);
    *   </script>
    * </body>
    *
diff --git a/ppapi/examples/scripting/post_message.html b/ppapi/examples/scripting/post_message.html
index 883d312..46e0c34 100644
--- a/ppapi/examples/scripting/post_message.html
+++ b/ppapi/examples/scripting/post_message.html
@@ -7,44 +7,45 @@
   -->
 <head>
   <title>postMessage Example</title>
-</head>
-
-<body>
+  <script>
 
-<script type="text/javascript">
-
-function SendString() {
-  plugin = document.getElementById('plugin');
-
-  // If we haven't already done it, set up an 'onmessage' function.  This will
-  // get invoked whenever the plugin calls Instance::PostMessage in C++ (or
-  // PPB_Messaging::PostMessage in C).  In this case, we're expecting a bool to
-  // tell us whether the string we passed was a palindrome.
-  if (!plugin.onmessage) {
-    plugin.onmessage = function(message_event) {
-      if (message_event.data) {
-        alert("The string was a palindrome.");
-      } else {
-        alert("The string was not a palindrome.");
-      }
+  function HandleMessage(message_event) {
+    if (message_event.data) {
+      alert("The string was a palindrome.");
+    } else {
+      alert("The string was not a palindrome.");
     }
   }
 
-  var inputBox = document.getElementById("inputBox");
+  function AddListener() {
+    var plugin = document.getElementById("plugin");
+    plugin.addEventListener("message", HandleMessage, false);
+  }
+  document.addEventListener("DOMContentLoaded", AddListener, false);
+
+  </script>
+</head>
+
+<body>
+  <script>
 
-  // Send the string to the plugin using postMessage.  This results in a call
-  // to Instance::HandleMessage in C++ (or PPP_Messaging::HandleMessage in C).
-  plugin.postMessage(inputBox.value);
-}
+  function SendString() {
+    var plugin = document.getElementById("plugin");
+    var inputBox = document.getElementById("inputBox");
+
+    // Send the string to the plugin using postMessage.  This results in a call
+    // to Instance::HandleMessage in C++ (or PPP_Messaging::HandleMessage in C).
+    plugin.postMessage(inputBox.value);
+  }
 
-</script>
+  </script>
 
-<input type="text" id="inputBox" name="inputBox" value="ablewasiereisawelba"/>
-<p>
-<button onclick='SendString()'>Is Palindrome</button>
-<object id="plugin" type="application/x-ppapi-post-message-example"
-        width="0" height="0"/>
-<hr>
+  <input type="text" id="inputBox" name="inputBox" value="ablewasiereisawelba"/>
+  <p>
+  <button onclick="SendString()">Is Palindrome</button>
+  <object id="plugin" type="application/x-ppapi-post-message-example"
+          width="1" height="1"/>
+  <hr>
 </body>
 </html>
 
diff --git a/ppapi/tests/test_post_message.cc b/ppapi/tests/test_post_message.cc
index abe1733..d8cbb8d 100644
--- a/ppapi/tests/test_post_message.cc
+++ b/ppapi/tests/test_post_message.cc
@@ -4,11 +4,14 @@
 
 #include "ppapi/tests/test_post_message.h"
 
+#include <algorithm>
+
 #include "ppapi/c/dev/ppb_testing_dev.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/cpp/dev/scriptable_object_deprecated.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/var.h"
+#include "ppapi/tests/test_utils.h"
 #include "ppapi/tests/testing_instance.h"
 
 REGISTER_TEST_CASE(PostMessage);
@@ -38,22 +41,40 @@ void TestPostMessage::HandleMessage(const pp::Var& message_data) {
   testing_interface_->QuitMessageLoop(instance_->pp_instance());
 }
 
-bool TestPostMessage::MakeOnMessageEcho(const std::string& expression) {
-  std::string js_code(
-      "document.getElementById('plugin').onmessage = function(message_event) {"
-      "    document.getElementById('plugin').postMessage(");
+bool TestPostMessage::AddEchoingListener(const std::string& expression) {
+  std::string js_code;
+  js_code += "var plugin = document.getElementById('plugin');"
+             "var message_handler = function(message_event) {"
+             "  plugin.postMessage(";
   js_code += expression;
-  js_code += ");}";
+  js_code += "  );"
+             "};"
+             "plugin.addEventListener('message', message_handler);"
+             // Maintain an array of all event listeners, attached to the
+             // plugin. This is so that we can easily remove them later (see
+             // ClearListeners()).
+             "if (!plugin.eventListeners) plugin.eventListeners = [];"
+             "plugin.eventListeners.push(message_handler);";
+  pp::Var exception;
+  instance_->ExecuteScript(js_code, &exception);
+  return exception.is_undefined();
+}
+
+bool TestPostMessage::ClearListeners() {
+  std::string js_code(
+      "var plugin = document.getElementById('plugin');"
+      "while (plugin.eventListeners.length) {"
+      "  plugin.removeEventListener('message', plugin.eventListeners.pop());"
+      "}");
   pp::Var exception;
-  // TODO(dmichael):  Move ExecuteScript to the testing interface.
   instance_->ExecuteScript(js_code, &exception);
   return(exception.is_undefined());
 }
 
 std::string TestPostMessage::TestSendingData() {
-  // Set up the JavaScript onmessage handler to echo the data part of the
+  // Set up the JavaScript message event listener to echo the data part of the
   // message event back to us.
-  ASSERT_TRUE(MakeOnMessageEcho("message_event.data"));
+  ASSERT_TRUE(AddEchoingListener("message_event.data"));
 
   // Test sending a message to JavaScript for each supported type.  The JS sends
   // the data back to us, and we check that they match.
@@ -106,16 +127,18 @@ std::string TestPostMessage::TestSendingData() {
   ASSERT_EQ(message_data_.size(), 1);
   ASSERT_TRUE(message_data_.back().is_null());
 
+  ASSERT_TRUE(ClearListeners());
+
   PASS();
 }
 
 std::string TestPostMessage::TestMessageEvent() {
-  // Set up the JavaScript onmessage handler to pass us some values from the
-  // MessageEvent and make sure they match our expectations.
+  // Set up the JavaScript message event listener to pass us some values from
+  // the MessageEvent and make sure they match our expectations.
 
-  // Have onmessage pass back the type of message_event and make sure it's
+  // Have the listener pass back the type of message_event and make sure it's
   // "object".
-  ASSERT_TRUE(MakeOnMessageEcho("typeof(message_event)"));
+  ASSERT_TRUE(AddEchoingListener("typeof(message_event)"));
   message_data_.clear();
   instance_->PostMessage(pp::Var(kTestInt));
   ASSERT_EQ(message_data_.size(), 0);
@@ -123,9 +146,10 @@ std::string TestPostMessage::TestMessageEvent() {
   ASSERT_EQ(message_data_.size(), 1);
   ASSERT_TRUE(message_data_.back().is_string());
   ASSERT_EQ(message_data_.back().AsString(), "object");
+  ASSERT_TRUE(ClearListeners());
 
   // Make sure all the non-data properties have the expected values.
-  bool success = MakeOnMessageEcho("((message_event.origin == '')"
+  bool success = AddEchoingListener("((message_event.origin == '')"
                                    " && (message_event.lastEventId == '')"
                                    " && (message_event.source == null)"
                                    " && (message_event.ports == null)"
@@ -140,26 +164,57 @@ std::string TestPostMessage::TestMessageEvent() {
   ASSERT_EQ(message_data_.size(), 1);
   ASSERT_TRUE(message_data_.back().is_bool());
   ASSERT_TRUE(message_data_.back().AsBool());
+  ASSERT_TRUE(ClearListeners());
+
+  // Add some event handlers to make sure they receive messages.
+  ASSERT_TRUE(AddEchoingListener("1"));
+  ASSERT_TRUE(AddEchoingListener("2"));
+  ASSERT_TRUE(AddEchoingListener("3"));
+
+  message_data_.clear();
+  instance_->PostMessage(pp::Var(kTestInt));
+  // Make sure we don't get a response in a re-entrant fashion.
+  ASSERT_EQ(message_data_.size(), 0);
+  // We should get 3 messages.
+  testing_interface_->RunMessageLoop(instance_->pp_instance());
+  testing_interface_->RunMessageLoop(instance_->pp_instance());
+  testing_interface_->RunMessageLoop(instance_->pp_instance());
+  ASSERT_EQ(message_data_.size(), 3);
+  // Copy to a vector of doubles and sort; w3c does not specify the order for
+  // event listeners. (Copying is easier than writing an operator< for pp::Var.)
+  //
+  // See http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/events.html.
+  VarVector::iterator iter(message_data_.begin()), the_end(message_data_.end());
+  std::vector<double> double_vec;
+  for (; iter != the_end; ++iter) {
+    ASSERT_TRUE(iter->is_number());
+    double_vec.push_back(iter->AsDouble());
+  }
+  std::sort(double_vec.begin(), double_vec.end());
+  ASSERT_DOUBLE_EQ(double_vec[0], 1.0);
+  ASSERT_DOUBLE_EQ(double_vec[1], 2.0);
+  ASSERT_DOUBLE_EQ(double_vec[2], 3.0);
+
+  ASSERT_TRUE(ClearListeners());
 
   PASS();
 }
 
 std::string TestPostMessage::TestNoHandler() {
-  // Delete the onmessage handler (if it exists)
-  std::string js_code(
-      "if (document.getElementById('plugin').onmessage) {"
-      "  delete document.getElementById('plugin').onmessage;"
-      "}");
-  pp::Var exception;
-  instance_->ExecuteScript(js_code, &exception);
-  ASSERT_TRUE(exception.is_undefined());
+  // Delete any lingering event listeners.
+  ASSERT_TRUE(ClearListeners());
 
-  // Now send a message and make sure we don't get anything back (and that we
-  // don't crash).
+  // Now send a message.  We shouldn't get a response.
   message_data_.clear();
   instance_->PostMessage(pp::Var());
-  testing_interface_->RunMessageLoop(instance_->pp_instance());
-  ASSERT_EQ(message_data_.size(), 0);
+  // Note that at this point, if we call RunMessageLoop, we should hang, because
+  // there should be no call to our HandleMessage function to quit the loop.
+  // Therefore, we will do CallOnMainThread to yield control.  That event should
+  // fire, but we should see no messages when we return.
+  TestCompletionCallback callback(instance_->pp_instance());
+  pp::Module::Get()->core()->CallOnMainThread(0, callback);
+  callback.WaitForResult();
+  ASSERT_TRUE(message_data_.empty());
 
   PASS();
 }
diff --git a/ppapi/tests/test_post_message.h b/ppapi/tests/test_post_message.h
index cf347ec..3b58ba2 100644
--- a/ppapi/tests/test_post_message.h
+++ b/ppapi/tests/test_post_message.h
@@ -23,10 +23,20 @@ class TestPostMessage : public TestCase {
   // the given value to the back of message_data_
   virtual void HandleMessage(const pp::Var& message_data);
 
-  // Set the JavaScript onmessage handler to echo back some expression based on
-  // the message_event by passing it to postMessage.  Returns true on success,
-  // false on failure.
-  bool MakeOnMessageEcho(const std::string& expression);
+  // Add a listener for message events which will echo back the given
+  // JavaScript expression by passing it to postMessage. JavaScript Variables
+  // available to the expression are:
+  //  'plugin' - the DOM element for the test plugin.
+  //  'message_event' - the message event parameter to the listener function.
+  // This also adds the new listener to an array called 'eventListeners' on the
+  // plugin's DOM element. This is used by ClearListeners().
+  // Returns true on success, false on failure.
+  bool AddEchoingListener(const std::string& expression);
+
+  // Clear any listeners that have been added using AddEchoingListener by
+  // calling removeEventListener for each.
+  // Returns true on success, false on failure.
+  bool ClearListeners();
 
   // Test some basic functionality;  make sure we can send data successfully
   // in both directions.
@@ -39,9 +49,11 @@ class TestPostMessage : public TestCase {
   // Test sending a message when no handler exists, make sure nothing happens.
   std::string TestNoHandler();
 
+  typedef std::vector<pp::Var> VarVector;
+
   // This is used to store pp::Var objects we receive via a call to
   // HandleMessage.
-  std::vector<pp::Var> message_data_;
+  VarVector message_data_;
 };
 
 #endif  // PPAPI_TESTS_TEST_POST_MESSAGE_H_
diff --git a/webkit/plugins/ppapi/message_channel.cc b/webkit/plugins/ppapi/message_channel.cc
index b6f01d7..c34886f 100644
--- a/webkit/plugins/ppapi/message_channel.cc
+++ b/webkit/plugins/ppapi/message_channel.cc
@@ -295,25 +295,32 @@ bool MessageChannel::EvaluateOnMessageInvoker() {
   if (NPVARIANT_IS_OBJECT(onmessage_invoker_))
     return true;
 
-  // This is the javascript code that we invoke.  It checks to see if onmessage
-  // exists, and if so, it invokes it.
+  // This is the javascript code that we invoke to create and dispatch a
+  // message event.
   const char invoke_onmessage_js[] =
-      "(function(module_instance, message_data) {"
-      "  if (module_instance &&"  // Only invoke if the instance is valid and
-      "      module_instance.onmessage &&"  // has a function named onmessage.
-      "      typeof(module_instance.onmessage) == 'function') {"
-      "    var message_event = document.createEvent('MessageEvent');"
+      "(function(window, module_instance, message_data) {"
+      "  if (module_instance) {"
+      "    var message_event = window.document.createEvent('MessageEvent');"
       "    message_event.initMessageEvent('message',"  // type
       "                                   false,"  // canBubble
       "                                   false,"  // cancelable
       "                                   message_data,"  // data
-      "                                   '',"  // origin
+      "                                   '',"  // origin [*]
       "                                   '',"  // lastEventId
-      "                                   module_instance,"  // source
+      "                                   null,"  // source [*]
       "                                   []);"  // ports
-      "    module_instance.onmessage(message_event);"
+      "    module_instance.dispatchEvent(message_event);"
       "  }"
       "})";
+  // [*] 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)
+
   NPString function_string = { invoke_onmessage_js,
                                sizeof(invoke_onmessage_js)-1 };
   // Get the current frame to pass to the evaluate function.
@@ -351,13 +358,26 @@ void MessageChannel::PostMessageToJavaScriptImpl(PP_Var message_data) {
 
   NPVariant result_var;
   VOID_TO_NPVARIANT(result_var);
-  NPVariant npvariant_args[2];
+  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[0]);
+                      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[1]))
+  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_),
author	dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2011-05-13 23:11:54 +0000
committer	dmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2011-05-13 23:11:54 +0000
commit	8585fd865df3044b8215e4486698422562712e6c (patch)
tree	f6f2b49818fac61ed3039dc9aa1402b9647c7fed
parent	5e8d27087b478de2d7136fa9016c4e178e19305e (diff)
download	chromium_src-8585fd865df3044b8215e4486698422562712e6c.zip chromium_src-8585fd865df3044b8215e4486698422562712e6c.tar.gz chromium_src-8585fd865df3044b8215e4486698422562712e6c.tar.bz2