Hook up V8<->Var conversion in the pepper message channel

ppapi/tests/test_post_message.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 "ppapi/tests/test_post_message.h"
 
 #include <algorithm>
+#include <map>
 #include <sstream>
 
 #include "ppapi/c/dev/ppb_testing_dev.h"
 #include "ppapi/c/pp_var.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/var.h"
 #include "ppapi/cpp/var_array_buffer.h"
 #include "ppapi/tests/pp_thread.h"
 #include "ppapi/tests/test_utils.h"
 #include "ppapi/tests/testing_instance.h"
@@ skipping 33 matching lines @@
 };
 
 void InvokePostMessageThreadFunc(void* user_data) {
   InvokePostMessageThreadArg* arg =
       static_cast<InvokePostMessageThreadArg*>(user_data);
   for (int32_t i = 0; i < kMessagesToSendPerThread; ++i)
     arg->instance->PostMessage(arg->value_to_send);
   delete arg;
 }
 
+// TODO(raymes): Consider putting something like this into pp::Var.
+bool VarsEqual(const pp::Var& expected,
+               const pp::Var& actual,
+               std::map<int64_t, int64_t>* visited_ids) {
+  if (expected.pp_var().type != actual.pp_var().type) {
+    if (!expected.is_number() && !actual.is_number())
+      return false;
+  }
+  // TODO(raymes): Implement a pp::Var::IsRefCounted() function.
+  if (expected.pp_var().type > PP_VARTYPE_DOUBLE) {
+    std::map<int64_t, int64_t>::const_iterator it =
+        visited_ids->find(expected.pp_var().value.as_id);
+    if (it != visited_ids->end()) {
+      if (it->second == actual.pp_var().value.as_id)
+        return true;
+      return false;
+    }
+    (*visited_ids)[expected.pp_var().value.as_id] = actual.pp_var().value.as_id;

[dmichael (off chromium), 2013/05/09 20:21:12]

random trivia: You can do this with one lookup using map::insert. If there is no
matching item in the map, it will not insert. The return is a pair with an
iterator and a bool...  the bool tells you whether the insert happened, the
iterator is always where the item with the given key is in the map.

Nice when performance is more important than readability. Usually what you have
is more readable, IMHO, so don't change unless you want to.

[raymes, 2013/05/13 17:09:43]

Cool thanks - yeah I didn't know (or I knew and forgot). I used this in the
other CL already to add a DCHECK :)

+  }
+
+  if (expected.is_number()) {
+    return fabs(expected.AsDouble() - actual.AsDouble()) < 1.0e-4;
+  } else if (expected.is_array()) {
+    pp::VarArray_Dev expected_array(expected);
+    pp::VarArray_Dev actual_array(actual);
+    if (expected_array.GetLength() != actual_array.GetLength())
+      return false;
+    for (uint32_t i = 0; i < expected_array.GetLength(); ++i) {
+      if (!VarsEqual(expected_array.Get(i), actual_array.Get(i), visited_ids))
+        return false;
+    }
+    return true;
+  } else if (expected.is_dictionary()) {
+    pp::VarDictionary_Dev expected_dict(expected);
+    pp::VarDictionary_Dev actual_dict(actual);
+    if (expected_dict.GetKeys().GetLength() !=
+        actual_dict.GetKeys().GetLength()) {
+      return false;
+    }
+    for (uint32_t i = 0; i < expected_dict.GetKeys().GetLength(); ++i) {
+      pp::Var key = expected_dict.GetKeys().Get(i);

[dmichael (off chromium), 2013/05/09 20:21:12]

Don't you also need to compare the value of the key? Otherwise, you might say
that the following are equivalent:
{ "a": 1, "b": 2 }
{ "c": 1, "d": 2 }

[raymes, 2013/05/13 17:09:43]

Well it's not iterating through the values here, but the keys. But I should
explicitly check that the other dictionary contains that key (otherwise Get()
will just return PP_VARTYPE_UNDEFINED which may cause a false positive here if
unlucky).

[dmichael (off chromium), 2013/05/13 18:02:26]

Sorry, I was misreading, as if you were using the i-th key in both cases.

+      if (!VarsEqual(expected_dict.Get(key), actual_dict.Get(key), visited_ids))
+        return false;
+    }
+    return true;
+  } else {
+    return expected == actual;
+  }
+}
+
+bool VarsEqual(const pp::Var& expected,
+               const pp::Var& actual) {
+  std::map<int64_t, int64_t> visited_ids;
+  return VarsEqual(expected, actual, &visited_ids);
+}
+
 class ScopedArrayBufferSizeSetter {
  public:
   ScopedArrayBufferSizeSetter(const PPB_Testing_Dev* interface,
                               PP_Instance instance,
                               uint32_t threshold)
      : interface_(interface),
        instance_(instance) {
     interface_->SetMinimumArrayBufferSizeForShmem(instance_, threshold);
   }
   ~ScopedArrayBufferSizeSetter() {
@@ skipping 57 matching lines @@
 
   return success;
 }
 
 void TestPostMessage::RunTests(const std::string& filter) {
   // Note: SendInInit must be first, because it expects to receive a message
   // that was sent in Init above.
   RUN_TEST(SendInInit, filter);
   RUN_TEST(SendingData, filter);
   RUN_TEST(SendingArrayBuffer, filter);
+  RUN_TEST(SendingArray, filter);
+  RUN_TEST(SendingDictionary, filter);
+  RUN_TEST(SendingComplexVar, filter);
   RUN_TEST(MessageEvent, filter);
   RUN_TEST(NoHandler, filter);
   RUN_TEST(ExtraParam, filter);
   if (testing_interface_->IsOutOfProcess())
     RUN_TEST(NonMainThread, filter);
 }
 
 void TestPostMessage::HandleMessage(const pp::Var& message_data) {
   if (message_data.is_string() &&
       (message_data.AsString() == FINISHED_WAITING_MESSAGE))
@@ skipping 205 matching lines @@
     for (size_t i = 0; i < test_data.ByteLength(); ++i)
       ASSERT_EQ(buff[i], received_buff[i]);
 
     message_data_.clear();
     ASSERT_TRUE(ClearListeners());
   }
 
   PASS();
 }
 
+std::string TestPostMessage::TestSendingArray() {
+  // Clean up after previous tests. This also swallows the message sent by Init
+  // if we didn't run the 'SendInInit' test. All tests other than 'SendInInit'
+  // should start with these.
+  WaitForMessages();
+  ASSERT_TRUE(ClearListeners());
+
+  pp::VarArray_Dev array;
+  array.Set(0, pp::Var(kTestBool));
+  array.Set(1, pp::Var(kTestString));
+  // Purposely leave index 2 empty.
+  array.Set(3, pp::Var(kTestInt));
+  array.Set(4, pp::Var(kTestDouble));
+
+  std::stringstream ss;
+  ss << array.GetLength();

[dmichael (off chromium), 2013/05/09 20:21:12]

How about:
std::string length_as_string(ss.str());
just for readability's sake

[raymes, 2013/05/13 17:09:43]

Done.

+
+  // Have the listener test some properties of the Array.
+  std::vector<std::string> properties_to_check;
+  properties_to_check.push_back(
+      "message_event.data.constructor.name === 'Array'");
+  properties_to_check.push_back(
+      std::string("message_event.data.length === ") + ss.str());
+  for (std::vector<std::string>::iterator iter = properties_to_check.begin();
+       iter != properties_to_check.end();
+       ++iter) {
+    ASSERT_TRUE(AddEchoingListener(*iter));
+    message_data_.clear();
+    instance_->PostMessage(array);
+    ASSERT_EQ(message_data_.size(), 0);
+    ASSERT_EQ(WaitForMessages(), 1);
+    ASSERT_TRUE(message_data_.back().is_bool());
+    if (!message_data_.back().AsBool())
+      return std::string("Failed: ") + *iter + ", size: " + ss.str();
+    ASSERT_TRUE(message_data_.back().AsBool());
+    ASSERT_TRUE(ClearListeners());

[dmichael (off chromium), 2013/05/09 20:21:12]

I wonder how hard it would be to wrap this property-checking stuff in a
function, since we're using it in 3 places now? Then replace this here with...

ASSERT_SUBTEST_SUCCESS(
    CheckMessageProperties(properties_to_check));

Or something

[raymes, 2013/05/13 17:09:43]

Done.

+  }
+
+  // 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(array);
+  // 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_array());
+  ASSERT_TRUE(VarsEqual(array, message_data_.back()));
+
+  message_data_.clear();
+  ASSERT_TRUE(ClearListeners());
+
+  PASS();
+}
+
+std::string TestPostMessage::TestSendingDictionary() {
+  // Clean up after previous tests. This also swallows the message sent by Init
+  // if we didn't run the 'SendInInit' test. All tests other than 'SendInInit'
+  // should start with these.
+  WaitForMessages();
+  ASSERT_TRUE(ClearListeners());
+
+  pp::VarDictionary_Dev dictionary;
+  dictionary.Set(pp::Var("foo"), pp::Var(kTestBool));
+  dictionary.Set(pp::Var("bar"), pp::Var(kTestString));
+  dictionary.Set(pp::Var("abc"), pp::Var(kTestInt));
+  dictionary.Set(pp::Var("def"), pp::Var());
+
+  std::stringstream ss;
+  ss << dictionary.GetKeys().GetLength();
+
+  // Have the listener test some properties of the Dictionary.
+  std::vector<std::string> properties_to_check;
+  properties_to_check.push_back(
+      "message_event.data.constructor.name === 'Object'");
+  properties_to_check.push_back(
+      std::string("Object.keys(message_event.data).length === ") + ss.str());
+  for (std::vector<std::string>::iterator iter = properties_to_check.begin();
+       iter != properties_to_check.end();
+       ++iter) {
+    ASSERT_TRUE(AddEchoingListener(*iter));
+    message_data_.clear();
+    instance_->PostMessage(dictionary);
+    ASSERT_EQ(message_data_.size(), 0);
+    ASSERT_EQ(WaitForMessages(), 1);
+    ASSERT_TRUE(message_data_.back().is_bool());
+    if (!message_data_.back().AsBool())
+      return std::string("Failed: ") + *iter + ", size: " + ss.str();
+    ASSERT_TRUE(message_data_.back().AsBool());
+    ASSERT_TRUE(ClearListeners());
+  }
+
+  // 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);
+  ASSERT_EQ(WaitForMessages(), 1);
+  ASSERT_TRUE(message_data_.back().is_dictionary());
+  ASSERT_TRUE(VarsEqual(dictionary, message_data_.back()));
+
+  message_data_.clear();
+  ASSERT_TRUE(ClearListeners());
+
+  PASS();
+}
+
+std::string TestPostMessage::TestSendingComplexVar() {
+  // Clean up after previous tests. This also swallows the message sent by Init
+  // if we didn't run the 'SendInInit' test. All tests other than 'SendInInit'
+  // should start with these.
+  WaitForMessages();
+  ASSERT_TRUE(ClearListeners());
+
+  pp::VarDictionary_Dev dictionary;
+  dictionary.Set(pp::Var("foo"), pp::Var(kTestBool));
+  dictionary.Set(pp::Var("bar"), pp::Var(kTestString));
+  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;
+  array.Set(0, pp::Var(kTestBool));
+  array.Set(1, pp::Var(kTestString));
+  // Purposely leave index 2 empty.

[dmichael (off chromium), 2013/05/09 20:21:12]

This doesn't actually leave index 2 empty in our implementation; it sticks an
Undefined in, right? Granted, it's a subtle difference.

[raymes, 2013/05/13 17:09:43]

Right - I updated the comment to clarify.

+  array.Set(3, pp::Var(kTestInt));
+  array.Set(4, pp::Var(kTestDouble));
+
+  dictionary.Set(pp::Var("array1"), array);
+  dictionary.Set(pp::Var("array2"), array);
+
+  // Set up a (dictionary -> dictionary2 -> dictionary) cycle.
+  pp::VarDictionary_Dev dictionary2;
+  dictionary2.Set(pp::Var("dictionary"), dictionary);
+  dictionary.Set(pp::Var("dictionary2"), dictionary2);

[dmichael (off chromium), 2013/05/09 20:21:12]

Can we involve an array in a cycle too? And maybe have a situation where one
child refers to another child of the same parent?
  A
 / \
v   v
B-->C

[raymes, 2013/05/13 17:09:43]

Done - I modified it to add these 2 cases. I didn't think it was necessary to be
too thorough here because the unittests should cover the important corner cases.

+
+  // 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);
+  ASSERT_EQ(WaitForMessages(), 1);
+  ASSERT_TRUE(message_data_.back().is_dictionary());
+  pp::VarDictionary_Dev result(message_data_.back());
+  ASSERT_TRUE(VarsEqual(dictionary, message_data_.back()));
+
+  // Break the cycles.
+  dictionary.Delete(pp::Var("dictionary"));
+  dictionary.Delete(pp::Var("dictionary2"));
+  result.Delete(pp::Var("dictionary"));
+  result.Delete(pp::Var("dictionary2"));
+
+  message_data_.clear();
+  ASSERT_TRUE(ClearListeners());
+
+  PASS();
+}
+
 std::string TestPostMessage::TestMessageEvent() {
   // Set up the JavaScript message event listener to pass us some values from
   // the MessageEvent and make sure they match our expectations.
 
   WaitForMessages();
   ASSERT_TRUE(ClearListeners());
   // Have the listener pass back the class name of message_event and make sure
   // it's "MessageEvent".
   ASSERT_TRUE(AddEchoingListener("message_event.constructor.name"));
   message_data_.clear();
@@ skipping 137 matching lines @@
     ASSERT_TRUE(received_value <= kThreadsToRun);
     ++received_counts[received_value];
   }
   ASSERT_EQ(received_counts, expected_counts);
 
   message_data_.clear();
   ASSERT_TRUE(ClearListeners());
 
   PASS();
 }