LGTM with a few nits.

https://codereview.chromium.org/15855012/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/15855012/diff/1/include/v8.h#newcode2397
include/v8.h:2397: virtual void* Allocate(size_t length) = 0;
I think it might make sense to include the current Isolate in the
arguments of Allocate/Free, even if we don't need it right now. In other
places we have always regretted our decision to leave it out so far, and
we have it at the call sites, anyway. Just to be future-proof...

https://codereview.chromium.org/15855012/diff/1/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/15855012/diff/1/src/d8.cc#newcode1589
src/d8.cc:1589: static ShellArrayBufferAllocator array_buffer_allocator;
Why do we need "static"?

https://codereview.chromium.org/15855012/diff/1/src/v8.h
File src/v8.h (right):

https://codereview.chromium.org/15855012/diff/1/src/v8.h#newcode129
src/v8.h:129: CHECK(array_buffer_allocator_ == NULL);
CHECK_EQ(NULL, array_buffer_allocator_);

https://codereview.chromium.org/15855012/diff/1/test/cctest/cctest.cc
File test/cctest/cctest.cc (right):

https://codereview.chromium.org/15855012/diff/1/test/cctest/cctest.cc#newcode113
test/cctest/cctest.cc:113: static CcTestArrayBufferAllocator
array_buffer_allocator;
Why do we need "static"?

https://codereview.chromium.org/15855012/

--
--
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