Change ArrayBuffer API and implementation to use embedder-provided allocator.

Change ArrayBuffer API and implementation to use embedder-provided allocator.

R=svenpanne@chromium.org

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

[Dmitry Lomov (no reviews), 2013-06-10 13:30:25]

Hi Sven,
please take a look.
Any suggestions on how to make this better definitely welcome!

[Sven Panne, 2013-06-11 09:54:37]

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

[Dmitry Lomov (no reviews), 2013-06-11 10:40:09]

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;
ArrayBuffer allocator very specifically does not depend on current isolate. One
of our primary use cases is WebKit where ArrayBuffers backing store can move
between isolates, so if it is allocated in the context of one isolate, it might
be freed in the context of another. 
On 2013/06/11 09:54:37, Sven Panne wrote:
> 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;
On 2013/06/11 09:54:37, Sven Panne wrote:
> Why do we need "static"?

Done.

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);
On 2013/06/11 09:54:37, Sven Panne wrote:
> CHECK_EQ(NULL, array_buffer_allocator_);

Done.

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;
On 2013/06/11 09:54:37, Sven Panne wrote:
> Why do we need "static"?

Done.

[Dmitry Lomov (no reviews), 2013-06-11 10:41:23]

Committed patchset #2 manually as r15056 (presubmit successful).