Add internal fields to JSArrayBufferViews (JSTypedArray and JSDataView)

Add internal fields to JSArrayBufferViews (JSTypedArray and JSDataView)

In Blink, JSTypedArray and JSDataView objects act as "wrappers" for C++
objects. Wrapping protocol in Blink requires all wrapper JavaScript objects
to have a certain amount of internal fields that Blink uses for
book-keeping (essentially a pointer to C++ object and some type
information). This change adds those internal fields to JSTypedArray and
JSDataView, in a similiar way to how it is done for JSArrayBuffer.

R=titzer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15511

[Dmitry Lomov (no reviews), 2013-07-04 17:01:20]

titzer@: please review
svenpanne@: FYI (comments welcome as usual)

[Dmitry Lomov (no reviews), 2013-07-04 17:37:27]

Added zeroing out internal fields and tests that verify that.

[titzer, 2013-07-05 08:09:57]

https://codereview.chromium.org/18695004/diff/3001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/18695004/diff/3001/include/v8.h#newcode2492
include/v8.h:2492: #ifndef V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT
I'm not sure why you conditionally define a macro here. Is the macro potentially
defined somewhere else (maybe with a -D for testing?) If so, it would be nice to
have a sentence explaining why.

https://codereview.chromium.org/18695004/diff/3001/include/v8.h#newcode2525
include/v8.h:2525: V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT;
BTW, where are these internal fields defined?

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:2557: static void CheckInternalFields(v8::Handle<T>
value) {
rename to CheckInternalFieldsAreZero?

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:2574: CheckInternalFields<v8::ArrayBuffer>(ab);
Is C++ not able to infer the type argument <v8::ArrayBuffer> here?

[Dmitry Lomov (no reviews), 2013-07-05 09:56:02]

Thanks for the review! Comments addressed. PTAL

https://codereview.chromium.org/18695004/diff/3001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/18695004/diff/3001/include/v8.h#newcode2492
include/v8.h:2492: #ifndef V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT
On 2013/07/05 08:09:57, titzer wrote:
> I'm not sure why you conditionally define a macro here. Is the macro
potentially
> defined somewhere else (maybe with a -D for testing?) If so, it would be nice
to
> have a sentence explaining why.
The number of these external fields can vary from embedder to embedder. I chose
2 as a convenient default for Blink.
Added comment to that effect.

https://codereview.chromium.org/18695004/diff/3001/include/v8.h#newcode2525
include/v8.h:2525: V8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT;
On 2013/07/05 08:09:57, titzer wrote:
> BTW, where are these internal fields defined?

I am not sure what the question is (the rest of the patch defines the fields,
adding them to JSTypedArray and JSArrayBufferView). These fields are accessed by
the embedder, we do not know what they are, we just provide space.

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:2557: static void CheckInternalFields(v8::Handle<T>
value) {
On 2013/07/05 08:09:57, titzer wrote:
> rename to CheckInternalFieldsAreZero?

Done.

https://codereview.chromium.org/18695004/diff/3001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:2574: CheckInternalFields<v8::ArrayBuffer>(ab);
On 2013/07/05 08:09:57, titzer wrote:
> Is C++ not able to infer the type argument <v8::ArrayBuffer> here?

Done.

[titzer, 2013-07-05 10:02:34]

lgtm

[Dmitry Lomov (no reviews), 2013-07-05 10:12:44]

Committed patchset #3 manually as r15511 (presubmit successful).