Add PPB_VarDictionary_Dev support - part 1.

ppapi/shared_impl/var_value_conversions_unittest.cc

Index: ppapi/shared_impl/var_value_conversions_unittest.cc
diff --git a/ppapi/shared_impl/var_value_conversions_unittest.cc b/ppapi/shared_impl/var_value_conversions_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ad04d5691ef2218e85489189b8b24b6f6560a3a2
--- /dev/null
+++ b/ppapi/shared_impl/var_value_conversions_unittest.cc
@@ -0,0 +1,198 @@
+// Copyright (c) 2013 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/shared_impl/var_value_conversions.h"
+
+#include <cmath>
+#include <cstring>
+
+#include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "ppapi/c/pp_bool.h"
+#include "ppapi/c/pp_var.h"
+#include "ppapi/shared_impl/dictionary_var.h"
+#include "ppapi/shared_impl/proxy_lock.h"
+#include "ppapi/shared_impl/scoped_pp_var.h"
+#include "ppapi/shared_impl/test_globals.h"
+#include "ppapi/shared_impl/var.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ppapi {
+namespace {
+
+bool Equals(const base::Value& value, const PP_Var& var) {
+  switch (value.GetType()) {
+    case base::Value::TYPE_NULL: {
+      return var.type == PP_VARTYPE_NULL;
+    }
+    case base::Value::TYPE_BOOLEAN: {
+      bool result = false;
+      return var.type == PP_VARTYPE_BOOL &&
+             value.GetAsBoolean(&result) &&
+             result == PP_ToBool(var.value.as_bool);
+    }
+    case base::Value::TYPE_INTEGER: {
+      int result = 0;
+      return var.type == PP_VARTYPE_INT32 &&
+             value.GetAsInteger(&result) &&
+             result == var.value.as_int;
+    }
+    case base::Value::TYPE_DOUBLE: {
+      double result = 0;
+      return var.type == PP_VARTYPE_DOUBLE &&
+             value.GetAsDouble(&result) &&
+             fabs(result - var.value.as_double) < 1.0e-4;
+    }
+    case base::Value::TYPE_STRING: {
+      std::string result;
+      StringVar* string_var = StringVar::FromPPVar(var);
+      return string_var &&
+             value.GetAsString(&result) &&
+             result == string_var->value();
+    }
+    case base::Value::TYPE_BINARY: {
+      const base::BinaryValue& binary_value =
+          static_cast<const base::BinaryValue&>(value);
+      ArrayBufferVar* array_buffer_var = ArrayBufferVar::FromPPVar(var);
+      if (!array_buffer_var ||
+          binary_value.GetSize() != array_buffer_var->ByteLength()) {
+        return false;
+      }
+
+      bool result = !memcmp(binary_value.GetBuffer(), array_buffer_var->Map(),
+                            binary_value.GetSize());
+      array_buffer_var->Unmap();
+      return result;
+    }
+    case base::Value::TYPE_DICTIONARY: {
+      const base::DictionaryValue& dict_value =
+          static_cast<const base::DictionaryValue&>(value);
+      DictionaryVar* dict_var = DictionaryVar::FromPPVar(var);
+      if (!dict_var || dict_value.size() != dict_var->key_value_map().size())
+        return false;
+
+      for (base::DictionaryValue::Iterator value_iter(dict_value);
+           !value_iter.IsAtEnd(); value_iter.Advance()) {

[dmichael (off chromium), 2013/03/05 22:01:03]

nit: I prefer 1 statement per line if they don't fit

[yzshen1, 2013/03/14 05:38:21]

Done.

+        DictionaryVar::KeyValueMap::const_iterator var_iter =
+            dict_var->key_value_map().find(value_iter.key());
+        if (var_iter == dict_var->key_value_map().end() ||
+            !Equals(value_iter.value(), var_iter->second.get()))
+          return false;
+      }
+      return true;
+    }
+    case base::Value::TYPE_LIST: {
+      // TODO(yzshen): add support once array var is supported.
+      return false;
+    }
+  }
+  NOTREACHED();
+  return false;
+}
+
+class VarValueConversionsTest : public testing::Test {
+ public:
+  VarValueConversionsTest() {
+  }
+  virtual ~VarValueConversionsTest() {
+  }
+
+  // testing::Test implementation.
+  virtual void SetUp() {
+    ProxyLock::Acquire();
+  }
+  virtual void TearDown() {
+    ProxyLock::Release();
+  }
+
+ private:
+  TestGlobals globals_;
+};
+
+}  // namespace
+
+TEST_F(VarValueConversionsTest, CreateValueFromVar) {
+  // Undefined var is not a valid input.
+  ASSERT_EQ(NULL, CreateValueFromVar(PP_MakeUndefined()));
+
+  {
+    // Undefined var is not allowed even if it is stored in a dictionary var.

[dmichael (off chromium), 2013/03/05 22:01:03]

FWIW, I would consider this overly restrictive for Messaging. I would want
messaging to happily provide JS with an Object with an undefined field (and vice
versa). But I guess they don't need to be consistent; you're expecting the dev
to provide you with an object of a certain form.

How do you deal with optional fields? Are you expecting that they have to be
absent from the Dictionary?

[yzshen1, 2013/03/14 05:38:21]

Thanks! I looked at the relevant code more closely and changed  the behavior to:
- PP_VARTYPE_UNDEFINED -> base::Value::TYPE_NULL (This is mostly for array
conversion.)
- If a dictionary var has a field whose value is PP_VARTYPE_UNDEFINED, the field
is skipped when converted to base::DictionaryValue.

+    scoped_refptr<DictionaryVar> dict_var(new DictionaryVar());
+    ASSERT_TRUE(dict_var->SetWithStringKey("key_1", PP_MakeUndefined()));
+    ScopedPPVar var(ScopedPPVar::PassRef(), dict_var->GetPPVar());
+    ASSERT_EQ(NULL, CreateValueFromVar(var.get()));
+  }
+
+  {
+    // Var holding a ref to itself is not a valid input.
+    scoped_refptr<DictionaryVar> dict_var_1(new DictionaryVar());
+    ScopedPPVar var_1(ScopedPPVar::PassRef(), dict_var_1->GetPPVar());
+    scoped_refptr<DictionaryVar> dict_var_2(new DictionaryVar());
+    ScopedPPVar var_2(ScopedPPVar::PassRef(), dict_var_2->GetPPVar());
+
+    ASSERT_TRUE(dict_var_1->SetWithStringKey("key_1", var_2.get()));
+    scoped_ptr<base::Value> value(CreateValueFromVar(var_1.get()));
+    ASSERT_TRUE(value.get() != NULL);
+
+    ASSERT_TRUE(dict_var_2->SetWithStringKey("key_2", var_1.get()));
+    value.reset(CreateValueFromVar(var_1.get()));
+    ASSERT_EQ(NULL, value.get());
+
+    // Make sure |var_1| doesn't indirectly hold a ref to itself, otherwise it
+    // is leaked.
+    dict_var_1->DeleteWithStringKey("key_1");

[dmichael (off chromium), 2013/03/05 22:01:03]

Maybe assert at the end of all of these that the var tracker has no live vars?

[yzshen1, 2013/03/14 05:38:21]

Done. I added a check in TearDown().
On 2013/03/05 22:01:03, dmichael wrote:

+  }
+
+  {
+    // Test valid inputs.
+    scoped_refptr<DictionaryVar> dict_var_1(new DictionaryVar());
+    ScopedPPVar dict_pp_var_1(ScopedPPVar::PassRef(), dict_var_1->GetPPVar());
+    scoped_refptr<DictionaryVar> dict_var_2(new DictionaryVar());
+    ScopedPPVar dict_pp_var_2(ScopedPPVar::PassRef(), dict_var_2->GetPPVar());
+    scoped_refptr<StringVar> string_var(new StringVar("string_value"));
+    ScopedPPVar string_pp_var(ScopedPPVar::PassRef(), string_var->GetPPVar());
+
+    ASSERT_TRUE(dict_var_1->SetWithStringKey("null_key", PP_MakeNull()));
+    ASSERT_TRUE(dict_var_1->SetWithStringKey("string_key",
+                                             string_pp_var.get()));
+    ASSERT_TRUE(dict_var_1->SetWithStringKey("dict_key", dict_pp_var_2.get()));
+
+    ASSERT_TRUE(dict_var_2->SetWithStringKey("double_key", PP_MakeDouble(1)));
+    ASSERT_TRUE(dict_var_2->SetWithStringKey("int_key", PP_MakeInt32(2)));
+    ASSERT_TRUE(dict_var_2->SetWithStringKey("bool_key", PP_MakeBool(PP_TRUE)));
+
+    scoped_ptr<base::Value> value(CreateValueFromVar(dict_pp_var_1.get()));
+    ASSERT_TRUE(value.get() != NULL);
+    ASSERT_TRUE(Equals(*value, dict_pp_var_1.get()));
+  }
+
+  {
+    // Test that dictionary keys containing '.' are handled correctly.
+    scoped_refptr<DictionaryVar> dict_var(new DictionaryVar());
+    ScopedPPVar dict_pp_var(ScopedPPVar::PassRef(), dict_var->GetPPVar());
+
+    ASSERT_TRUE(dict_var->SetWithStringKey("double.key", PP_MakeDouble(1)));
+    ASSERT_TRUE(dict_var->SetWithStringKey("int.key..name", PP_MakeInt32(2)));
+
+    scoped_ptr<base::Value> value(CreateValueFromVar(dict_pp_var.get()));
+    ASSERT_TRUE(value.get() != NULL);
+    ASSERT_TRUE(Equals(*value, dict_pp_var.get()));
+  }
+}
+
+TEST_F(VarValueConversionsTest, CreateVarFromValue) {
+  base::DictionaryValue dict_value;
+  dict_value.Set("null_key", base::Value::CreateNullValue());
+  dict_value.SetString("string_key", "string_value");
+  dict_value.SetDouble("dict_key.double_key", 1);
+  dict_value.SetInteger("dict_key.int_key", 2);
+  dict_value.SetBoolean("dict_key.bool_key", true);
+
+  ScopedPPVar var(ScopedPPVar::PassRef(), CreateVarFromValue(dict_value));
+  ASSERT_TRUE(Equals(dict_value, var.get()));
+}
+
+}  // namespace ppapi