Move PPB_VarArray and PPB_VarDictionary out of dev.

Move PPB_VarArray and PPB_VarDictionary out of dev.

BUG=236958

[raymes, 2013-06-09 06:23:32]

Noel, thanks for the comments!
PTAL at ppb_var_array.idl and ppb_var_dictionary.idl. I pasted your comments
into the code review.

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_array.idl
File ppapi/api/ppb_var_array.idl (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_array.id...
ppapi/api/ppb_var_array.idl:37: PP_Var Get([in] PP_Var array, [in] uint32_t
index);
noelallen@: Doc is a bit unclear...  How is the ref count different than any
other pp_var operation?  If the item in at the index is a string, dictionary, or
array, the PP_Var that gets contructed will do the normal ref count thing.  Or
is the entire array getting ref counted?  If so, how would we de-ref it?


I tried to make the doc a bit clearer regarding this. Let me know if you want me
to change it further.

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_array.id...
ppapi/api/ppb_var_array.idl:76: PP_Bool SetLength([in] PP_Var array, [in]
uint32_t length);
noelallen@: Doc consistency...  If all the other function mention ref counts,
this should mention de-reffing freed items when the length is reduced.

Done.

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_dictiona...
File ppapi/api/ppb_var_dictionary.idl (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_dictiona...
ppapi/api/ppb_var_dictionary.idl:18: * done by value instead of by reference.
noelallen: Not convinced this has to be a string.  For example it would be nice
if we could create a dictionary from an array, then add string keys later.
Doesn't have to be in rev 1.0 however.
In any case, I expect that developers will want PP_Var(s) that are ints, to
correctly index.   Otherwise, should we take a const char * the way that array
takes a uint32_t instead of a var?
NOTE: If the Pepper developer can't control how items are passed via JavaScript,
this could be problematic.  For example:

I believe this is consistent with JavaScript objects where property names can
only be strings. I believe the reason to have a PP_Var as the key type is
because GetKeys() returns a list of PP_Var. So this makes the [in] type for the
key, the same as the [out] type which just gives consistency to the interface.
yzshen@ might be able to comment more.

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_dictiona...
ppapi/api/ppb_var_dictionary.idl:42: PP_Var Get([in] PP_Var dict, [in] PP_Var
key);
Should this be:
  PP_Error Get([in] PP_Var dict, [in] PP_Var key, [out] PP_Var value)?
So that we can return PP_NO_KEY, PP_INVALID_KEY?
Otherwise we can not differentiate between a value that is currently set at
Undefined without also calling HasKey.
Is calling Set with a PP_Var undefined legal?   Is it possible to get an
undefined via a post message?
Maybe HasKey could return PP_OK, PP_NO_KEY, PP_INVALID_KEY...  instead?

I think returning undefined is reasonable because it matches the behavior of a
JavaScript object when the key doesn't exist. It is possible to receive an
undefined via PostMessage but undefined just signifies that no value has been
defined.

[darin (slow to review), 2013-06-10 16:00:26]

LGTM

https://codereview.chromium.org/16136009/diff/2001/ppapi/c/ppb_var_array.h
File ppapi/c/ppb_var_array.h (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/c/ppb_var_array.h#ne...
ppapi/c/ppb_var_array.h:80: * elements that no longer fit are removed and the
references to them will be
nit: ", and"

[dmichael (off chromium), 2013-06-11 17:19:47]

lgtm

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_dictiona...
File ppapi/api/ppb_var_dictionary.idl (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/api/ppb_var_dictiona...
ppapi/api/ppb_var_dictionary.idl:18: * done by value instead of by reference.
On 2013/06/09 06:23:32, raymes wrote:
> noelallen: Not convinced this has to be a string.  For example it would be
nice
> if we could create a dictionary from an array, then add string keys later.
I had a similar discussion with Yuzhu a while back. It turns out:
"An object property name can be any valid JavaScript string, or anything that
can be converted to a string, including the empty string."
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Working_with_Ob...
Also:
"Array elements are just object properties"
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...
var a = ["Hello"];
console.log(a["0"]);
> Hello
v8 certainly optimizes under the hood, but array elements really aren't
different from other properties in behavior (which surprised me, too).

> Doesn't have to be in rev 1.0 however.
Yeah, we talked about making it so that you can create a VarArray and also use
the VarDictionary interface on it. But the backend implementation would be a
little tricky (e.g., if you use VarDictionary to set a property with "0"
identifier, we have to check the array part to make sure it doesn't already have
that key. And vice versa when you add to the Array part. I don't think it would
give acceptable performance characteristics to just use a hash map to represent
Array.

So we might do something like that later, but yeah, not 1.0.

> In any case, I expect that developers will want PP_Var(s) that are ints, to
> correctly index.   Otherwise, should we take a const char * the way that
array
> takes a uint32_t instead of a var?
Using PP_Var allows us to take advantage of all the existing UTF8 checks we have
in place.

> NOTE: If the Pepper developer can't control how items are passed via
JavaScript,
> this could be problematic.  For example:
> 
> I believe this is consistent with JavaScript objects where property names can
> only be strings. I believe the reason to have a PP_Var as the key type is
> because GetKeys() returns a list of PP_Var. So this makes the [in] type for
the
> key, the same as the [out] type which just gives consistency to the interface.
> yzshen@ might be able to comment more.

https://codereview.chromium.org/16136009/diff/2001/ppapi/cpp/var_array.cc
File ppapi/cpp/var_array.cc (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/cpp/var_array.cc#new...
ppapi/cpp/var_array.cc:4: 
Kind of weird git diff didn't detect this as the same. Might be nice for
posterity's sake to upload with:
 git cl upload --similarity=20
(or something) so this might get recognized as A+ instead of A

[raymes, 2013-06-16 23:35:36]

Sorry just getting back to this after interrups last week. Thanks for the
comments!

Noel: please let me know if you have any other comments about the interface
files.

https://codereview.chromium.org/16136009/diff/2001/ppapi/c/ppb_var_array.h
File ppapi/c/ppb_var_array.h (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/c/ppb_var_array.h#ne...
ppapi/c/ppb_var_array.h:80: * elements that no longer fit are removed and the
references to them will be
On 2013/06/10 16:00:26, darin wrote:
> nit: ", and"

Done.

https://codereview.chromium.org/16136009/diff/2001/ppapi/cpp/var_array.cc
File ppapi/cpp/var_array.cc (right):

https://codereview.chromium.org/16136009/diff/2001/ppapi/cpp/var_array.cc#new...
ppapi/cpp/var_array.cc:4: 
On 2013/06/11 17:19:47, dmichael wrote:
> Kind of weird git diff didn't detect this as the same. Might be nice for
> posterity's sake to upload with:
>  git cl upload --similarity=20
> (or something) so this might get recognized as A+ instead of A

Done.

[noelallen1, 2013-06-17 17:43:42]

The IDL LGTM.  For the C++ interfaces however, shouldn't we use "bool" instead
of the "PP_Bool"?  Isn't that really for C?

https://codereview.chromium.org/16136009/diff/11001/ppapi/cpp/var_dictionary.cc
File ppapi/cpp/var_dictionary.cc (right):

https://codereview.chromium.org/16136009/diff/11001/ppapi/cpp/var_dictionary....
ppapi/cpp/var_dictionary.cc:78: PP_Bool VarDictionary::Set(const Var& key, const
Var& value) {
bool?

https://codereview.chromium.org/16136009/diff/11001/ppapi/cpp/var_dictionary....
ppapi/cpp/var_dictionary.cc:91: PP_Bool VarDictionary::HasKey(const Var& key)
const {
bool?

https://codereview.chromium.org/16136009/diff/11001/ppapi/cpp/var_dictionary.h
File ppapi/cpp/var_dictionary.h (right):

https://codereview.chromium.org/16136009/diff/11001/ppapi/cpp/var_dictionary....
ppapi/cpp/var_dictionary.h:82: PP_Bool HasKey(const Var& key) const;
Should this be "bool" in C++?  Isn't PP_Bool only for C?

[raymes, 2013-06-18 16:44:59]

Thanks Noel. As we discussed with Yuzhu, I think you're right and it's
consistent to use bool instead of PP_Bool. I've updated this in the following CL
which I will actually commit: https://codereview.chromium.org/17005006/

Closing this CL now.