Add PPB_VarDictionary_Dev support - part 1.

ppapi/shared_impl/var_value_conversions.cc

Index: ppapi/shared_impl/var_value_conversions.cc
diff --git a/ppapi/shared_impl/var_value_conversions.cc b/ppapi/shared_impl/var_value_conversions.cc
new file mode 100644
index 0000000000000000000000000000000000000000..605a984b238d5661ebddb0407f99bcefaf610533
--- /dev/null
+++ b/ppapi/shared_impl/var_value_conversions.cc
@@ -0,0 +1,349 @@
+// 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 <limits>
+#include <set>
+#include <stack>
+
+#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_stdint.h"
+#include "ppapi/shared_impl/dictionary_var.h"
+#include "ppapi/shared_impl/ppapi_globals.h"
+#include "ppapi/shared_impl/scoped_pp_var.h"
+#include "ppapi/shared_impl/var.h"
+#include "ppapi/shared_impl/var_tracker.h"
+
+namespace ppapi {
+
+namespace {
+
+// In CreateValueFromVar(), a stack is used to keep track of conversion progress
+// of array and dictionary vars. VarNodeBase is the base class of stack
+// elements.
+class VarNodeBase : public base::RefCounted<VarNodeBase> {
+ public:
+  virtual bool IsArrayVar() const { return false; }
+  virtual bool IsDictionaryVar() const { return false; }
+
+ protected:
+  friend class base::RefCounted<VarNodeBase>;
+
+  VarNodeBase() {}
+  virtual ~VarNodeBase() {}
+};
+
+class DictionaryVarNode : public VarNodeBase {
+ public:
+  DictionaryVarNode(int64_t in_var_id, const DictionaryVar* dict_var)
+      : var_id(in_var_id),
+        next(dict_var->key_value_map().begin()),
+        end(dict_var->key_value_map().end()),
+        dict_value(new base::DictionaryValue()) {
+  }
+
+  virtual bool IsDictionaryVar() const OVERRIDE {
+    return true;
+  }
+
+  int64_t var_id;
+  DictionaryVar::KeyValueMap::const_iterator next;
+  DictionaryVar::KeyValueMap::const_iterator end;
+  std::string current_key;
+  scoped_ptr<base::DictionaryValue> dict_value;
+
+ private:
+  virtual ~DictionaryVarNode() {}
+};
+
+// In CreateVarFromValue(), a stack is used to keep track of conversion progress
+// of list and dictionary values. ValueNodeBase is the base class of stack
+// elements.
+class ValueNodeBase : public base::RefCounted<ValueNodeBase> {
+ public:
+  virtual bool IsListValue() const { return false; }
+  virtual bool IsDictionaryValue() const { return false; }
+
+ protected:
+  friend class base::RefCounted<ValueNodeBase>;
+
+  ValueNodeBase() {}
+  virtual ~ValueNodeBase() {}
+};
+
+class DictionaryValueNode : public ValueNodeBase {
+ public:
+  explicit DictionaryValueNode(const base::DictionaryValue& dict_value)
+      : iter(dict_value),
+        dict_var(new DictionaryVar()) {
+  }
+
+  virtual bool IsDictionaryValue() const OVERRIDE {
+    return true;
+  }
+
+  base::DictionaryValue::Iterator iter;
+  std::string current_key;
+  scoped_refptr<DictionaryVar> dict_var;
+
+ private:
+  virtual ~DictionaryValueNode() {}
+};
+
+// Helper function for CreateValueFromVar(). If |var| is neither array nor
+// dictionary, the conversion result is stored in |value|; otherwise,
+// |parent_ids| and |state| are updated.
+//
+// Returns false on failure.
+bool CreateValueFromVarHelper(const PP_Var& var,
+                              scoped_ptr<base::Value>* value,
+                              std::set<int64_t>* parent_ids,

[dmichael (off chromium), 2013/03/15 17:35:48]

As I mentioned on IM, I think we probably should deal with more than just
cycles, since Value doesn't support a node having >1 parent.

E.g., stuff like:
  A
 / \
B-->C

  A
 / \
B   C
|   ^
v   |
D----

(I.e., C is a child of both A and D).

Put another way, I think we only want to support tree structures, not DAGs.

This might be a different story for Messaging when we convert these things to
WebKit/v8 types. But for Value it kind of looks like we should detect these
cases and spit out an error.

[yzshen1, 2013/03/15 22:56:49]

Done. (Discussed offline.)
On 2013/03/15 17:35:48, dmichael wrote:

+                              std::stack<scoped_refptr<VarNodeBase> >* state) {
+  switch (var.type) {
+    case PP_VARTYPE_UNDEFINED:
+    case PP_VARTYPE_NULL: {
+      value->reset(base::Value::CreateNullValue());
+      return true;
+    }
+    case PP_VARTYPE_BOOL: {
+      value->reset(new base::FundamentalValue(PP_ToBool(var.value.as_bool)));
+      return true;
+    }
+    case PP_VARTYPE_INT32: {
+      value->reset(new base::FundamentalValue(var.value.as_int));
+      return true;
+    }
+    case PP_VARTYPE_DOUBLE: {
+      value->reset(new base::FundamentalValue(var.value.as_double));
+      return true;
+    }
+    case PP_VARTYPE_STRING: {
+      StringVar* string_var = StringVar::FromPPVar(var);
+      if (!string_var)
+        return false;
+
+      value->reset(new base::StringValue(string_var->value()));
+      return true;
+    }
+    case PP_VARTYPE_OBJECT: {
+      return false;
+    }
+    case PP_VARTYPE_ARRAY: {
+      // TODO(yzshen): Implement it once array var is supported.
+      return false;
+    }
+    case PP_VARTYPE_DICTIONARY: {
+      if (parent_ids->find(var.value.as_id) != parent_ids->end()) {

[dmichael (off chromium), 2013/03/15 17:35:48]

optional suggestion: you can also ContainsKey from stl_util.h for this

[yzshen1, 2013/03/15 22:56:49]

Done.

+        // A circular reference is found.
+        return false;
+      }
+
+      DictionaryVar* dict_var = DictionaryVar::FromPPVar(var);
+      if (!dict_var)
+        return false;
+
+      parent_ids->insert(var.value.as_id);
+      state->push(new DictionaryVarNode(var.value.as_id, dict_var));
+      return true;
+    }
+    case PP_VARTYPE_ARRAY_BUFFER: {
+      ArrayBufferVar* array_buffer = ArrayBufferVar::FromPPVar(var);
+      if (!array_buffer)
+        return false;
+
+      base::BinaryValue* binary_value =
+          base::BinaryValue::CreateWithCopiedBuffer(
+              static_cast<const char*>(array_buffer->Map()),
+              array_buffer->ByteLength());
+      array_buffer->Unmap();
+      value->reset(binary_value);
+      return true;
+    }
+  }
+  NOTREACHED();
+  return false;
+}
+
+// Helper function for CreateVarFromValue(). If |value| is neither list nor
+// dictionary, the conversion result is stored in |var|; otherwise, |state| is
+// updated.
+//
+// Returns false on failure.
+bool CreateVarFromValueHelper(
+    const base::Value& value,
+    ScopedPPVar* var,
+    std::stack<scoped_refptr<ValueNodeBase> >* state) {
+  switch (value.GetType()) {
+    case base::Value::TYPE_NULL: {
+      *var = PP_MakeNull();
+      return true;
+    }
+    case base::Value::TYPE_BOOLEAN: {
+      bool result = false;
+      if (value.GetAsBoolean(&result)) {
+        *var = PP_MakeBool(PP_FromBool(result));
+        return true;
+      }
+      return false;
+    }
+    case base::Value::TYPE_INTEGER: {
+      int result = 0;
+      if (value.GetAsInteger(&result)) {
+        *var = PP_MakeInt32(result);
+        return true;
+      }
+      return false;
+    }
+    case base::Value::TYPE_DOUBLE: {
+      double result = 0;
+      if (value.GetAsDouble(&result)) {
+        *var = PP_MakeDouble(result);
+        return true;
+      }
+      return false;
+    }
+    case base::Value::TYPE_STRING: {
+      std::string result;
+      if (value.GetAsString(&result)) {
+        *var = ScopedPPVar(ScopedPPVar::PassRef(),
+                           StringVar::StringToPPVar(result));
+        return true;
+      }
+      return false;
+    }
+    case base::Value::TYPE_BINARY: {
+      const base::BinaryValue& binary_value =
+          static_cast<const base::BinaryValue&>(value);
+
+      size_t size = binary_value.GetSize();
+      if (size > std::numeric_limits<uint32>::max())
+        return false;
+
+      ScopedPPVar temp(
+          ScopedPPVar::PassRef(),
+          PpapiGlobals::Get()->GetVarTracker()->MakeArrayBufferPPVar(
+              static_cast<uint32>(size), binary_value.GetBuffer()));
+      if (temp.get().type == PP_VARTYPE_ARRAY_BUFFER) {
+        *var = temp;
+        return true;
+      }
+      return false;
+    }
+    case base::Value::TYPE_DICTIONARY: {
+      state->push(new DictionaryValueNode(
+          static_cast<const base::DictionaryValue&>(value)));
+      return true;
+    }
+    case base::Value::TYPE_LIST: {
+      // TODO(yzshen): Add support once array var is supported.
+      return false;
+    }
+  }
+  NOTREACHED();
+  return false;
+}
+
+}  // namespace
+
+base::Value* CreateValueFromVar(const PP_Var& var) {
+  // Used to detect circular references.
+  std::set<int64_t> parent_ids;
+  std::stack<scoped_refptr<VarNodeBase> > state;
+  scoped_ptr<base::Value> current_value;
+
+  if (!CreateValueFromVarHelper(var, &current_value, &parent_ids, &state))
+    return NULL;
+
+  while (!state.empty()) {
+    PP_Var current_var = PP_MakeUndefined();
+    if (state.top()->IsDictionaryVar()) {
+      scoped_refptr<DictionaryVarNode> top(
+          static_cast<DictionaryVarNode*>(state.top().get()));
+
+      DCHECK(current_value.get() || top->current_key.empty());
+      if (current_value.get()) {
+        top->dict_value->SetWithoutPathExpansion(top->current_key,
+                                                 current_value.release());

[dmichael (off chromium), 2013/03/15 17:35:48]

As we discussed offline, I had a little bit of trouble following the algorithm
initially. There are a lot of moving pieces, and they aren't really explained. I
think the part that was most confusing to me was the state that's held in the
VarNode things. There's sort of an inner loop here, where state.top() will
remain unchanged for a while as you iterate through its children via the if/else
below. It's an unusual construct. I'd prefer if it was avoided somehow and you
had a clear nested loop. Failing that, some explanatory comments about it would
be nice.

Also as mentioned in IM...  I think it might be cleaner as a BFS, work-queue
approach. I'm not tied to either way, but I would prefer if this logic was made
a little easier to follow.

[yzshen1, 2013/03/15 22:56:49]

Done. Thanks!

On 2013/03/15 17:35:48, dmichael wrote:

+      }
+      if (top->next == top->end) {
+        current_value.reset(top->dict_value.release());
+        parent_ids.erase(top->var_id);
+        state.pop();
+        continue;
+      } else {
+        top->current_key = top->next->first;
+        current_var = top->next->second.get();
+        ++top->next;
+
+        // Ignore the key-value pair if the value is undefined.
+        if (current_var.type == PP_VARTYPE_UNDEFINED) {
+          top->current_key.clear();
+          continue;
+        }
+      }
+    } else {
+      NOTREACHED();
+      return NULL;
+    }
+
+    DCHECK(!current_value.get());
+    if (!CreateValueFromVarHelper(current_var, &current_value, &parent_ids,
+                                  &state)) {
+      return NULL;
+    }
+  }
+  DCHECK(parent_ids.empty());
+  return current_value.release();
+}
+
+PP_Var CreateVarFromValue(const base::Value& value) {
+  std::stack<scoped_refptr<ValueNodeBase> > state;
+  ScopedPPVar current_var;
+
+  if (!CreateVarFromValueHelper(value, &current_var, &state))
+    return PP_MakeUndefined();
+
+  while (!state.empty()) {
+    const base::Value* current_value = NULL;
+    if (state.top()->IsDictionaryValue()) {
+      scoped_refptr<DictionaryValueNode> top(
+          static_cast<DictionaryValueNode*>(state.top().get()));
+
+      DCHECK(current_var.get().type != PP_VARTYPE_UNDEFINED ||
+             top->current_key.empty());
+      if (current_var.get().type != PP_VARTYPE_UNDEFINED &&
+          !top->dict_var->SetWithStringKey(
+              top->current_key, current_var.get())) {
+        return PP_MakeUndefined();
+      }
+      current_var = PP_MakeUndefined();
+      if (top->iter.IsAtEnd()) {
+        current_var = ScopedPPVar(ScopedPPVar::PassRef(),
+                                  top->dict_var->GetPPVar());
+        state.pop();
+        continue;
+      } else {
+        top->current_key = top->iter.key();
+        current_value = &top->iter.value();
+        top->iter.Advance();
+      }
+    } else {
+      NOTREACHED();
+      return PP_MakeUndefined();
+    }
+
+    DCHECK(current_var.get().type == PP_VARTYPE_UNDEFINED);
+    if (!CreateVarFromValueHelper(*current_value, &current_var, &state))
+      return PP_MakeUndefined();
+  };
+
+  return current_var.Release();
+}
+}  // namespace ppapi
+