Externalization API for ArrayBuffer

src/api.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 6001 matching lines @@
   i::Handle<i::JSObject> paragon_handle(i::JSObject::cast(paragon));
   EXCEPTION_PREAMBLE(isolate);
   ENTER_V8(isolate);
   i::Handle<i::JSObject> result = i::Copy(paragon_handle);
   has_pending_exception = result.is_null();
   EXCEPTION_BAILOUT_CHECK(isolate, Local<Object>());
   return Utils::ToLocal(result);
 }
 
 
+bool v8::ArrayBuffer::IsExternal() const {
+  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();

[Sven Panne, 2013/05/23 07:57:01]

Just remove this line and the following IsDeadCheck. This idiom is fundamentally
broken, anyway, and we want to remove it completely in the future, so no need
for it in new code.

[Dmitry Lomov (no reviews), 2013/05/23 08:42:15]

Done.

+  if (IsDeadCheck(isolate, "v8::ArrayBuffer::IsExternal()")) return 0;
+  i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(this);

[Sven Panne, 2013/05/23 07:57:01]

Nit: No need to name this temporary.

[Dmitry Lomov (no reviews), 2013/05/23 08:42:15]

Done.

+  return obj->is_external();
+}
+
+v8::ArrayBufferContents::~ArrayBufferContents() {
+  if (data_ != NULL) {

[Sven Panne, 2013/05/23 07:57:01]

No need for this guard, free(NULL) is by definition a no-op.

[Dmitry Lomov (no reviews), 2013/05/23 08:42:15]

Done.

+    free(data_);
+    data_ = NULL;
+  }
+  byte_length_ = 0;
+}
+
+
+void v8::ArrayBuffer::Externalize(ArrayBufferContents* contents) {
+  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();

[Sven Panne, 2013/05/23 07:57:01]

Kill the IsDeadCheck here, too.

[Dmitry Lomov (no reviews), 2013/05/23 08:42:15]

Done.

+  if (IsDeadCheck(isolate, "v8::ArrayBuffer::Data()")) return;
+  i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(this);
+  ApiCheck(!obj->is_external(),
+            "v8::ArrayBuffer::Externalize",
+            "ArrayBuffer already externalized");
+  obj->set_is_external(true);
+  size_t byte_length = static_cast<size_t>(obj->byte_length()->Number());
+  contents->data_ = obj->backing_store();

[Sven Panne, 2013/05/23 07:57:01]

We silently overwrite any previous backing store in contents here. This part of
the API looks very fragile and will easily lead to programming errors. Can we do
this differently?

[Dmitry Lomov (no reviews), 2013/05/23 08:42:15]

As discussed offline, added an ApiCheck
On 2013/05/23 07:57:01, Sven Panne wrote:

+  contents->byte_length_ = byte_length;
+}
+
+
 size_t v8::ArrayBuffer::ByteLength() const {
   i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
   if (IsDeadCheck(isolate, "v8::ArrayBuffer::ByteLength()")) return 0;
   i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(this);
   return static_cast<size_t>(obj->byte_length()->Number());
 }
 
 
-void* v8::ArrayBuffer::Data() const {
-  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
-  if (IsDeadCheck(isolate, "v8::ArrayBuffer::Data()")) return 0;
-  i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(this);
-  return obj->backing_store();
-}
-
-
 Local<ArrayBuffer> v8::ArrayBuffer::New(size_t byte_length) {
   i::Isolate* isolate = i::Isolate::Current();
   EnsureInitializedForIsolate(isolate, "v8::ArrayBuffer::New(size_t)");
   LOG_API(isolate, "v8::ArrayBuffer::New(size_t)");
   ENTER_V8(isolate);
   i::Handle<i::JSArrayBuffer> obj =
       isolate->factory()->NewJSArrayBuffer();
   i::Runtime::SetupArrayBufferAllocatingData(isolate, obj, byte_length);
   return Utils::ToLocal(obj);
 }
 
 
 Local<ArrayBuffer> v8::ArrayBuffer::New(void* data, size_t byte_length) {
   i::Isolate* isolate = i::Isolate::Current();
   EnsureInitializedForIsolate(isolate, "v8::ArrayBuffer::New(void*, size_t)");
   LOG_API(isolate, "v8::ArrayBuffer::New(void*, size_t)");
   ENTER_V8(isolate);
   i::Handle<i::JSArrayBuffer> obj =
       isolate->factory()->NewJSArrayBuffer();
-  i::Runtime::SetupArrayBuffer(isolate, obj, data, byte_length);
+  i::Runtime::SetupArrayBuffer(isolate, obj, true, data, byte_length);
   return Utils::ToLocal(obj);
 }
 
 
 Local<ArrayBuffer> v8::TypedArray::Buffer() {
   i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
   if (IsDeadCheck(isolate, "v8::TypedArray::Buffer()"))
     return Local<ArrayBuffer>();
   i::Handle<i::JSTypedArray> obj = Utils::OpenHandle(this);
   ASSERT(obj->buffer()->IsJSArrayBuffer());
@@ skipping 1806 matching lines @@
 
   v->VisitPointers(blocks_.first(), first_block_limit_);
 
   for (int i = 1; i < blocks_.length(); i++) {
     v->VisitPointers(blocks_[i], &blocks_[i][kHandleBlockSize]);
   }
 }
 
 
 } }  // namespace v8::internal