Disallow passing a SharedArrayBuffer in the transfer list.

src/value-serializer.cc

Index: src/value-serializer.cc
diff --git a/src/value-serializer.cc b/src/value-serializer.cc
index 81301fd55395b4e8d2f356ec9413da6b6425953b..2ac04fcb3b8c0e5ca775a9ff900a83d3e8512190 100644
--- a/src/value-serializer.cc
+++ b/src/value-serializer.cc
@@ -442,7 +442,7 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
     case JS_SET_TYPE:
       return WriteJSSet(Handle<JSSet>::cast(receiver));
     case JS_ARRAY_BUFFER_TYPE:
-      return WriteJSArrayBuffer(JSArrayBuffer::cast(*receiver));
+      return WriteJSArrayBuffer(Handle<JSArrayBuffer>::cast(receiver));
     case JS_TYPED_ARRAY_TYPE:
     case JS_DATA_VIEW_TYPE:
       return WriteJSArrayBufferView(JSArrayBufferView::cast(*receiver));
@@ -724,20 +724,37 @@ Maybe<bool> ValueSerializer::WriteJSSet(Handle<JSSet> set) {
   return Just(true);
 }
 
-Maybe<bool> ValueSerializer::WriteJSArrayBuffer(JSArrayBuffer* array_buffer) {
+Maybe<bool> ValueSerializer::WriteJSArrayBuffer(
+    Handle<JSArrayBuffer> array_buffer) {
   uint32_t* transfer_entry = array_buffer_transfer_map_.Find(array_buffer);
-  if (transfer_entry) {
-    WriteTag(array_buffer->is_shared()
-                 ? SerializationTag::kSharedArrayBufferTransfer
-                 : SerializationTag::kArrayBufferTransfer);
-    WriteVarint(*transfer_entry);
-    return Just(true);
-  }
-
   if (array_buffer->is_shared()) {
-    ThrowDataCloneError(
-        MessageTemplate::kDataCloneErrorSharedArrayBufferNotTransferred);
-    return Nothing<bool>();
+    if (!delegate_) {
+      ThrowDataCloneError(MessageTemplate::kDataCloneError, array_buffer);
+      return Nothing<bool>();
+    }
+
+    if (transfer_entry) {
+      // SharedArrayBuffer must not be in the transfer list.
+      ThrowDataCloneError(

[Jakob Kummerow, 2016/12/13 02:34:58]

Would it make more sense to enforce this with a
DCHECK(!array_buffer->is_shared()) in ValueSerializer::TransferArrayBuffer()
(and/or a DCHECK here)? Then you could move the
array_buffer_transfer_map_.Find() call into the branch below where
|transfer_entry| is actually required.

[jbroman, 2016/12/13 20:56:00]

+1

[binji, 2016/12/14 23:58:30]

Done.

+          MessageTemplate::kDataCloneErrorSharedArrayBufferTransferred);
+      return Nothing<bool>();
+    }
+
+    v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
+    Maybe<uint32_t> index = delegate_->TransferSharedArrayBuffer(
+        v8_isolate, Utils::ToLocal(array_buffer));
+    RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing<bool>());
+    DCHECK(!index.IsNothing());
+
+    WriteTag(SerializationTag::kSharedArrayBufferTransfer);

[jbroman, 2016/12/13 20:56:00]

nit: Should this be renamed to just kSharedArrayBuffer, since it's not a
transfer anymore?

[binji, 2016/12/14 23:58:30]

Done.

+    WriteVarint(index.FromJust());
+    return Just(true);
+  } else {

[jbroman, 2016/12/13 20:56:00]

nit: From my perspective, having to match this else with the corresponding if
makes this harder to read. Since the other branch always returns anyhow, could
this be formatted as:

if (array_buffer->is_shared()) {
  // shared array buffers
  return Just(true);
}

uint32_t* transfer_entry = array_buffer_transfer_map_.Find(array_buffer);
if (transfer_entry) {
  // array buffer transfer
  return Just(true);
}

// remaining code

[binji, 2016/12/14 23:58:30]

Done.

+    if (transfer_entry) {
+      WriteTag(SerializationTag::kArrayBufferTransfer);
+      WriteVarint(*transfer_entry);
+      return Just(true);
+    }
   }
   if (array_buffer->was_neutered()) {
     ThrowDataCloneError(MessageTemplate::kDataCloneErrorNeuteredArrayBuffer);
@@ -745,7 +762,7 @@ Maybe<bool> ValueSerializer::WriteJSArrayBuffer(JSArrayBuffer* array_buffer) {
   }
   double byte_length = array_buffer->byte_length()->Number();
   if (byte_length > std::numeric_limits<uint32_t>::max()) {
-    ThrowDataCloneError(MessageTemplate::kDataCloneError, handle(array_buffer));
+    ThrowDataCloneError(MessageTemplate::kDataCloneError, array_buffer);
     return Nothing<bool>();
   }
   WriteTag(SerializationTag::kArrayBuffer);