Don't send PP_Vars/V8 values with cycles across PostMessage

ppapi/tests/test_post_message.cc

Index: ppapi/tests/test_post_message.cc
diff --git a/ppapi/tests/test_post_message.cc b/ppapi/tests/test_post_message.cc
index 8c67ee5a7c1b8907c89d6d86708ba8145cccba1f..74e648d89ca8c0f1bca114698c7bab4e8833e1e2 100644
--- a/ppapi/tests/test_post_message.cc
+++ b/ppapi/tests/test_post_message.cc
@@ -238,6 +238,16 @@ bool TestPostMessage::AddEchoingListener(const std::string& expression) {
   return true;
 }
 
+bool TestPostMessage::PostMessageFromJavaScript(const std::string& func) {
+  std::string js_code;
+  js_code += "var plugin = document.getElementById('plugin');"
+             "plugin.postMessage(";
+  js_code += func + "()";
+  js_code += "                      );";
+  instance_->EvalScript(js_code);
+  return true;
+}
+
 bool TestPostMessage::ClearListeners() {
   std::string js_code;
   js_code += "var plugin = document.getElementById('plugin');"
@@ -523,6 +533,7 @@ std::string TestPostMessage::TestSendingComplexVar() {
   // if we didn't run the 'SendInInit' test. All tests other than 'SendInInit'
   // should start with these.
   WaitForMessages();
+  message_data_.clear();
   ASSERT_TRUE(ClearListeners());
 
   pp::Var string(kTestString);
@@ -531,7 +542,6 @@ std::string TestPostMessage::TestSendingComplexVar() {
   dictionary.Set(pp::Var("bar"), string);
   dictionary.Set(pp::Var("abc"), pp::Var(kTestInt));
   dictionary.Set(pp::Var("def"), pp::Var());
-  dictionary.Set(pp::Var("dictionary"), dictionary);  // Self-reference.
 
   // Reference to array.
   pp::VarArray_Dev array;
@@ -544,15 +554,9 @@ std::string TestPostMessage::TestSendingComplexVar() {
   dictionary.Set(pp::Var("array-ref1"), array);
   dictionary.Set(pp::Var("array-ref2"), array);
 
-  // Set up a (dictionary -> array -> dictionary) cycle.
-  pp::VarArray_Dev array2;
-  array2.Set(0, dictionary);
-  dictionary.Set(pp::Var("array2"), array2);
-
   // Set up the JavaScript message event listener to echo the data part of the
   // message event back to us.
   ASSERT_TRUE(AddEchoingListener("message_event.data"));
-  message_data_.clear();
   instance_->PostMessage(dictionary);
   // PostMessage is asynchronous, so we should not receive a response yet.
   ASSERT_EQ(message_data_.size(), 0);
@@ -561,12 +565,40 @@ std::string TestPostMessage::TestSendingComplexVar() {
   pp::VarDictionary_Dev result(message_data_.back());
   ASSERT_TRUE(VarsEqual(dictionary, message_data_.back()));
 
+  WaitForMessages();
+  message_data_.clear();
+  ASSERT_TRUE(ClearListeners());
+
+  // Set up a (dictionary -> array -> dictionary) cycle. Cycles shouldn't be
+  // transmitted.
+  pp::VarArray_Dev array2;
+  array2.Set(0, dictionary);
+  dictionary.Set(pp::Var("array2"), array2);
+
+  ASSERT_TRUE(AddEchoingListener("message_event.data"));
+  instance_->PostMessage(dictionary);
+  instance_->PostMessage(pp::Var());
+  // PostMessage is asynchronous, so we should not receive a response yet.
+  ASSERT_EQ(message_data_.size(), 0);
+  ASSERT_EQ(WaitForMessages(), 1);
+  ASSERT_TRUE(message_data_.back().is_undefined());
+
   // Break the cycles.
-  dictionary.Delete(pp::Var("dictionary"));
   dictionary.Delete(pp::Var("array2"));
-  result.Delete(pp::Var("dictionary"));
-  result.Delete(pp::Var("array2"));
 
+  WaitForMessages();
+  message_data_.clear();
+  ASSERT_TRUE(ClearListeners());
+
+  // Test sending a cycle from JavaScript to the plugin.
+  ASSERT_TRUE(AddEchoingListener("message_event.data"));
+  PostMessageFromJavaScript("function() { var x = []; x[0] = x; return x; }");
+  instance_->PostMessage(pp::Var());
+  ASSERT_EQ(message_data_.size(), 0);
+  ASSERT_EQ(WaitForMessages(), 1);

[dmichael (off chromium), 2013/06/05 17:05:47]

Shouldn't we expect 0 here now?

[raymes, 2013/06/05 22:48:26]

Yeah - I thought that WaitForMessages() wouldn't work if we weren't expecting a
message so I posted an undefined var right after, but it seems to work fine.

[dmichael (off chromium), 2013/06/06 16:32:30]

It sends its own special message (that's echoed back) that goes in the queue
behind whatever messages you're really waiting on, and quits the message loop
when that one arrives. That's how it knows how many messages you got.

+  ASSERT_TRUE(message_data_.back().is_undefined());
+
+  WaitForMessages();
   message_data_.clear();
   ASSERT_TRUE(ClearListeners());