Mostly nits.
https://codereview.chromium.org/13958007/diff/3001/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/13958007/diff/3001/include/v8.h#newcode1971
include/v8.h:1971:
Style: add empty line
https://codereview.chromium.org/13958007/diff/3001/include/v8.h#newcode1973
include/v8.h:1973: * An instance of the built-in ArrayBuffer
construnctor (ES6 draft 15.13.5).
Add a comment that this is still "experimental".
https://codereview.chromium.org/13958007/diff/3001/include/v8.h#newcode1994
include/v8.h:1994: * Create a new ArrayBuffer over existing memory
block.
Nit: "over an existing..."
https://codereview.chromium.org/13958007/diff/3001/include/v8.h#newcode5138
include/v8.h:5138:
STyle: add empty line
https://codereview.chromium.org/13958007/diff/3001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/13958007/diff/3001/src/api.cc#newcode5810
src/api.cc:5810: return i::Smi::cast(byte_length)->value();
The case distinction shouldn't be needed, it's done by Object::Number.
https://codereview.chromium.org/13958007/diff/3001/src/heap.h
File src/heap.h (right):
https://codereview.chromium.org/13958007/diff/3001/src/heap.h#newcode696
src/heap.h:696:
Style: remove extra line
https://codereview.chromium.org/13958007/diff/3001/src/heap.h#newcode697
src/heap.h:697: // Allocates the JS ArrayBuffer object.
Nit: "Allocate a JS ArrayBuffer..."
https://codereview.chromium.org/13958007/diff/3001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/13958007/diff/3001/src/runtime.cc#newcode674
src/runtime.cc:674: isolate->heap()->NumberFromDouble(
Use the handlified abstraction isolate->factory()->NewNumber (or perhaps
NewNumberFromUint).
https://codereview.chromium.org/13958007/diff/3001/src/runtime.cc#newcode684
src/runtime.cc:684: bool Runtime::AllocateAndSetupArrayBuffer(
Can we rename this to something like SetUpArrayBufferAllocatingData, to
avoid confusion?
https://codereview.chromium.org/13958007/diff/3001/src/runtime.cc#newcode692
src/runtime.cc:692: if (data == NULL) {
Nit: I'd put the return on the same line and remove the adjacent empty
lines.
https://codereview.chromium.org/13958007/diff/3001/src/runtime.cc#newcode701
src/runtime.cc:701: if (!SetupArrayBuffer(isolate, array_buffer, data,
allocated_length))
Nit: spurious extra space
https://codereview.chromium.org/13958007/diff/3001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):
https://codereview.chromium.org/13958007/diff/3001/test/cctest/test-api.cc#newcode2253
test/cctest/test-api.cc:2253:
Style: add extra line
https://codereview.chromium.org/13958007/diff/3001/test/cctest/test-api.cc#newcode2265
test/cctest/test-api.cc:2265: uint8_t* data =
static_cast<uint8_t*>(ab->Data());
Maybe add an assertion that Data is non-null.
https://codereview.chromium.org/13958007/diff/3001/test/cctest/test-api.cc#newcode2277
test/cctest/test-api.cc:2277: CHECK_EQ(0xAA, data[1]);
Also test the inverse, i.e., writing to data, reading from JS (same
below).
https://codereview.chromium.org/13958007/
--
--
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.
