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#newcode2557
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#newcode2574
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.

https://codereview.chromium.org/18695004/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to