Draft of a PPAPI interface for ArrayBuffer and typed arrays.

Draft of a PPAPI interface for ArrayBuffer.

See the TypedArray spec for reference:
http://www.khronos.org/registry/typedarray/specs/latest/

Things in the spec that I'm omiting:
- slice (Having a view of the ArrayBuffer that has a different offset/length)

TODO in future CLs:
-Implementation for in-process/trusted + tests (almost ready)
-NaCl proxy
-OOP proxy
Later still:
- Support for ArrayBufferView and TypedArray based on that.

BUG=103435
TEST=N/A

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113355

[darin (slow to review), 2011-11-08 22:16:18]

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/dev/ppb_arraybuffe...
File ppapi/api/dev/ppb_arraybuffer.idl (right):

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/dev/ppb_arraybuffe...
ppapi/api/dev/ppb_arraybuffer.idl:12: };
i guess this file should have the _dev suffix too?

had you considered just using PPB_Buffer_Dev instead?  it seems very similar.

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/pp_var.idl
File ppapi/api/pp_var.idl (right):

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/pp_var.idl#newcode72
ppapi/api/pp_var.idl:72: PP_VARTYPE_ARRAY_BUFFER
I'm very curious about the idea of making this be PP_VARTYPE_BUFFER (meaning
PPB_Buffer), and then just commenting that a PPB_Buffer can be converted to a
ArrayBuffer in JS.

[dmichael (off chromium), 2011-11-08 22:32:20]

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/pp_var.idl
File ppapi/api/pp_var.idl (right):

http://codereview.chromium.org/8502030/diff/2001/ppapi/api/pp_var.idl#newcode72
ppapi/api/pp_var.idl:72: PP_VARTYPE_ARRAY_BUFFER
PPB_Buffer is a PP_Resource, whereas I really want this to be a PP_Var. It's a
shame to have the duplication; maybe video could eventually move to using
ArrayBuffer?

[Takashi Toyoshima, 2011-12-02 07:34:54]

Hi,

I try this change, but all ui tests related to ArrayBuffer failed.
I guess Patch Set 5 miss following patch.

diff --git a/ppapi/ppapi_sources.gypi b/ppapi/ppapi_sources.gypi
index f715443..79cf018 100644
--- a/ppapi/ppapi_sources.gypi
+++ b/ppapi/ppapi_sources.gypi
@@ -200,6 +200,8 @@
       'cpp/dev/text_input_dev.h',
       'cpp/dev/url_util_dev.cc',
       'cpp/dev/url_util_dev.h',
+      'cpp/dev/var_array_buffer_dev.cc',
+      'cpp/dev/var_array_buffer_dev.h',
       'cpp/dev/video_capture_client_dev.cc',
       'cpp/dev/video_capture_client_dev.h',
       'cpp/dev/video_capture_dev.cc',
@@ -376,4 +378,4 @@
       },
     }],
   ],
}

[Takashi Toyoshima, 2011-12-02 13:31:14]

I successfully implement WebSocket binding using this change as follows.
I'll request review after this change is landed.
http://codereview.chromium.org/8774042/

[dmichael (off chromium), 2011-12-02 18:53:52]

I decided to take the implementation stuff back out of this CL and handle that
in a separate CL. I think this is ready for review, now that I'm confident I
have an API that does the job and is implementable.

http://codereview.chromium.org/8502030/diff/14002/ppapi/api/dev/ppb_var_array...
File ppapi/api/dev/ppb_var_array_buffer_dev.idl (right):

http://codereview.chromium.org/8502030/diff/14002/ppapi/api/dev/ppb_var_array...
ppapi/api/dev/ppb_var_array_buffer_dev.idl:32: PP_Var Create([in] PP_Instance
instance,
Note we want to use PP_Instance (as opposed to PP_Module, which string vars use)
because:
1) These are never shared between plugins anyway, since they will always be
copied or transferred between plugins.
2) We can force-free the arrays associated with an instance when the instance
goes away. 'Object' vars and resources are handled this way.

http://codereview.chromium.org/8502030/diff/14002/ppapi/api/dev/ppb_var_array...
ppapi/api/dev/ppb_var_array_buffer_dev.idl:47: mem_t Map([in] PP_Var array);
Left out UnMap, because it seems potentially dangerous to provide the ability to
UnMap the buffer but also cache a pointer to the buffer, which would get
invalidated if the buffer is unmapped (see the C++ class).

http://codereview.chromium.org/8502030/diff/14002/ppapi/cpp/dev/var_array_buf...
File ppapi/cpp/dev/var_array_buffer_dev.h (right):

http://codereview.chromium.org/8502030/diff/14002/ppapi/cpp/dev/var_array_buf...
ppapi/cpp/dev/var_array_buffer_dev.h:29: explicit VarArrayBuffer_Dev(const Var&
var);
This is basically down-casting the 'var handle'. There are other ways to do
this, but this way seemed fairly simple to implement and use.

[dmichael (off chromium), 2011-12-02 23:46:46]

Talked to brettw offline; the instance parameter won't be necessary. It could be
used protect us against memory leaks for in-process plugins, but doesn't help
for out-of-process plugins or NaCl apps. So it's not worth the trouble.

(I'll follow up with a CL to remove the use of Module from String vars and the
internal implementation. It's no longer necessary for the same reasons)

This patch also adds the new PP_VARTYPE_ARRAY_BUFFER_VAR enum value, and touches
some files that use this to add do-nothing cases.

PTAL.

[brettw, 2011-12-03 00:00:49]

http://codereview.chromium.org/8502030/diff/20001/ppapi/api/pp_var.idl
File ppapi/api/pp_var.idl (right):

http://codereview.chromium.org/8502030/diff/20001/ppapi/api/pp_var.idl#newcode73
ppapi/api/pp_var.idl:73: PP_VARTYPE_ARRAY_BUFFER = 9
I don't understand how this is different than ARRAY. Array isn't implemented and
I was intending it to be what you seem to be implementing here.

[dmichael (off chromium), 2011-12-03 00:19:36]

I was planning to have Array map to a conventional js array which can
contain objects and which can be sparse. ArrayBuffer is limited to
contiguous binary data.
On Dec 2, 2011 5:00 PM, <brettw@chromium.org> wrote:

>
>
http://codereview.chromium.**org/8502030/diff/20001/ppapi/**api/pp_var.idl<ht...
> File ppapi/api/pp_var.idl (right):
>
> http://codereview.chromium.**org/8502030/diff/20001/ppapi/**
>
api/pp_var.idl#newcode73<http://codereview.chromium.org/8502030/diff/20001/ppapi/api/pp_var.idl#newcode73>
> ppapi/api/pp_var.idl:73: PP_VARTYPE_ARRAY_BUFFER = 9
> I don't understand how this is different than ARRAY. Array isn't
> implemented and I was intending it to be what you seem to be
> implementing here.
>
>
http://codereview.chromium.**org/8502030/<http://codereview.chromium.org/8502...
>

[dmichael (off chromium), 2011-12-03 01:25:22]

Expanding a bit, I expected normal PPB_VarArray to have some kind of
get/set that takes an index and PP_Bar so you can contain arbitrary stuff.
We can optimize dense arrays pretty easily while still allowing arbitrary
integer indices. I think apps will want to be able to use conventional
arrays, but ArrayBuffer fills a different niche for transmitting raw
binary. It's also what's used in the websockets spec (along with Blob).

I'm also expecting we'll add interfaces for other special types that nacl
apps would find useful, like blob, arraybufferview, etc.
On Dec 2, 2011 5:19 PM, "David Michael" <dmichael@chromium.org> wrote:

> I was planning to have Array map to a conventional js array which can
> contain objects and which can be sparse. ArrayBuffer is limited to
> contiguous binary data.
> On Dec 2, 2011 5:00 PM, <brettw@chromium.org> wrote:
>
>>
>> http://codereview.chromium.**org/8502030/diff/20001/ppapi/**
>>
api/pp_var.idl<http://codereview.chromium.org/8502030/diff/20001/ppapi/api/pp_var.idl>
>> File ppapi/api/pp_var.idl (right):
>>
>> http://codereview.chromium.**org/8502030/diff/20001/ppapi/**
>>
api/pp_var.idl#newcode73<http://codereview.chromium.org/8502030/diff/20001/ppapi/api/pp_var.idl#newcode73>
>> ppapi/api/pp_var.idl:73: PP_VARTYPE_ARRAY_BUFFER = 9
>> I don't understand how this is different than ARRAY. Array isn't
>> implemented and I was intending it to be what you seem to be
>> implementing here.
>>
>>
http://codereview.chromium.**org/8502030/<http://codereview.chromium.org/8502...
>>
>

[brettw, 2011-12-03 01:42:49]

On Fri, Dec 2, 2011 at 4:19 PM, David Michael <dmichael@chromium.org> wrote:
> I was planning to have Array map to a conventional js array which can
> contain objects and which can be sparse. ArrayBuffer is limited to
> contiguous binary data.

Do we have that concept in V8? I thought arrays were just dictionaries
like all objects. I think V8 may optimize some of these cases, but I
don't think there is a JS concept of an array (I'm not an expert). I
don't want to make a confusing API that depends on optimizations in
the VM.

Brett

[dmichael (off chromium), 2011-12-03 03:07:06]

On 2011/12/03 01:42:49, brettw wrote:
> On Fri, Dec 2, 2011 at 4:19 PM, David Michael <mailto:dmichael@chromium.org>
wrote:
> > I was planning to have Array map to a conventional js array which can
> > contain objects and which can be sparse. ArrayBuffer is limited to
> > contiguous binary data.
> 
> Do we have that concept in V8? I thought arrays were just dictionaries
> like all objects.
All JS objects are dictionaries, but some of them have special 'native' stuff
that's supported by the browser. In the case of Array, one key example is:
"""
15.4.5.2 length
The length property of this Array object is a data property whose value is
always numerically greater than 
the name of every deletable property whose name is an array index
"""
This property is also settable and affects the contents of the array. Just as
importantly as supporting Array's behavior, I want to be able to support
sending/receiving a real Array so that plugins can communicate to JS with high
fidelity. When you send an Array from PPAPI it arrives at JS as a real JS Array
object, and not just a dictionary (same the other direction). I think this will
lead to the least surprise for developers. E.g., we had a bug filed against the
implementation before sync scripting was taken out for not handling Array
properly:
http://code.google.com/p/nativeclient/issues/detail?id=1367

We can also make PPB_VarDictionary (when we write it) respond to other object
vars (array, arraybuffer, blob, etc) so that we can support having extra
properties on them.

> I think V8 may optimize some of these cases,
I know V8 can optimize for dense arrays, though I'm not sure of the details. I'm
imagining the OOP version of the array would have a vector plus a hash map. If
the elements have contiguous indices starting at 0, they'll all be in the
vector. Weird usages of arrays would fall back to the hash map for at least some
of the elements. Details to be worked out, but I think we can get something
pretty efficient for the common case without too much trouble.
> but I
> don't think there is a JS concept of an array (I'm not an expert). I
> don't want to make a confusing API that depends on optimizations in
> the VM.
> 
> Brett

[brettw, 2011-12-05 21:29:03]

lgtm

[commit-bot: I haz the power, 2011-12-06 21:22:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmichael@chromium.org/8502030/25001

[commit-bot: I haz the power, 2011-12-06 22:04:22]

Try job failure for 8502030-25001 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2011-12-07 04:32:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmichael@chromium.org/8502030/25001

[commit-bot: I haz the power, 2011-12-07 06:49:03]

Change committed as 113355