WebMessaging: Send transferable ArrayBuffers by copy-and-neuter semantics

third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp

Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
index 55c8776877b810a98581050cde04171b953437c2..947ba493b73fa2c7a5c1862588318793398314fd 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
@@ -149,7 +149,7 @@ void SerializedScriptValue::toWireBytes(Vector<char>& result) const {
   }
 }
 
-static void acculumateArrayBuffersForAllWorlds(
+static void accumulateArrayBuffersForAllWorlds(
     v8::Isolate* isolate,
     DOMArrayBuffer* object,
     Vector<v8::Local<v8::ArrayBuffer>, 4>& buffers) {
@@ -230,64 +230,8 @@ void SerializedScriptValue::transferArrayBuffers(
     v8::Isolate* isolate,
     const ArrayBufferArray& arrayBuffers,
     ExceptionState& exceptionState) {
-  if (!arrayBuffers.size())
-    return;
-
-  for (size_t i = 0; i < arrayBuffers.size(); ++i) {
-    if (arrayBuffers[i]->isNeutered()) {
-      exceptionState.throwDOMException(
-          DataCloneError, "ArrayBuffer at index " + String::number(i) +
-                              " is already neutered.");
-      return;
-    }
-  }
-
-  std::unique_ptr<ArrayBufferContentsArray> contents =
-      wrapUnique(new ArrayBufferContentsArray(arrayBuffers.size()));
-
-  HeapHashSet<Member<DOMArrayBufferBase>> visited;
-  for (size_t i = 0; i < arrayBuffers.size(); ++i) {
-    if (visited.contains(arrayBuffers[i]))
-      continue;
-    visited.add(arrayBuffers[i]);
-
-    if (arrayBuffers[i]->isShared()) {
-      bool result = arrayBuffers[i]->shareContentsWith(contents->at(i));
-      if (!result) {
-        exceptionState.throwDOMException(
-            DataCloneError, "SharedArrayBuffer at index " + String::number(i) +
-                                " could not be transferred.");
-        return;
-      }
-    } else {
-      Vector<v8::Local<v8::ArrayBuffer>, 4> bufferHandles;
-      v8::HandleScope handleScope(isolate);
-      acculumateArrayBuffersForAllWorlds(
-          isolate, static_cast<DOMArrayBuffer*>(arrayBuffers[i].get()),
-          bufferHandles);
-      bool isNeuterable = true;
-      for (size_t j = 0; j < bufferHandles.size(); ++j)
-        isNeuterable &= bufferHandles[j]->IsNeuterable();
-
-      DOMArrayBufferBase* toTransfer = arrayBuffers[i];
-      if (!isNeuterable)
-        toTransfer =
-            DOMArrayBuffer::create(arrayBuffers[i]->buffer()->data(),
-                                   arrayBuffers[i]->buffer()->byteLength());
-      bool result = toTransfer->transfer(contents->at(i));
-      if (!result) {
-        exceptionState.throwDOMException(
-            DataCloneError, "ArrayBuffer at index " + String::number(i) +
-                                " could not be transferred.");
-        return;
-      }
-
-      if (isNeuterable)
-        for (size_t j = 0; j < bufferHandles.size(); ++j)
-          bufferHandles[j]->Neuter();
-    }
-  }
-  m_arrayBufferContentsArray = std::move(contents);
+  m_arrayBufferContentsArray =
+      transferArrayBufferContents(isolate, arrayBuffers, exceptionState);
 }
 
 v8::Local<v8::Value> SerializedScriptValue::deserialize(
@@ -402,6 +346,72 @@ bool SerializedScriptValue::extractTransferables(
   return true;
 }
 
+std::unique_ptr<ArrayBufferContentsArray>
+SerializedScriptValue::transferArrayBufferContents(
+    v8::Isolate* isolate,
+    const ArrayBufferArray& arrayBuffers,
+    ExceptionState& exceptionState) {
+  if (!arrayBuffers.size())
+    return nullptr;
+
+  for (size_t i = 0; i < arrayBuffers.size(); ++i) {

[haraken, 2016/10/19 09:49:28]

Can we use an iterator? An index-based access is slow because we have an
out-of-range check.

[nhiroki, 2016/10/20 07:44:02]

Done.

(Just to confirm: The out-of-range check indicates RELEASE_ASSERT in
WTF::Vector::at(), right?)

+    if (arrayBuffers[i]->isNeutered()) {
+      exceptionState.throwDOMException(
+          DataCloneError, "ArrayBuffer at index " + String::number(i) +
+                              " is already neutered.");
+      return nullptr;
+    }
+  }
+
+  std::unique_ptr<ArrayBufferContentsArray> contents =
+      wrapUnique(new ArrayBufferContentsArray(arrayBuffers.size()));
+
+  HeapHashSet<Member<DOMArrayBufferBase>> visited;
+  for (size_t i = 0; i < arrayBuffers.size(); ++i) {

[haraken, 2016/10/19 09:49:28]

Ditto.

[nhiroki, 2016/10/20 07:44:02]

Done.

+    if (visited.contains(arrayBuffers[i]))
+      continue;
+    visited.add(arrayBuffers[i]);
+
+    if (arrayBuffers[i]->isShared()) {
+      bool result = arrayBuffers[i]->shareContentsWith(contents->at(i));
+      if (!result) {
+        exceptionState.throwDOMException(
+            DataCloneError, "SharedArrayBuffer at index " + String::number(i) +
+                                " could not be transferred.");
+        return nullptr;
+      }
+    } else {
+      Vector<v8::Local<v8::ArrayBuffer>, 4> bufferHandles;
+      v8::HandleScope handleScope(isolate);
+      accumulateArrayBuffersForAllWorlds(
+          isolate, static_cast<DOMArrayBuffer*>(arrayBuffers[i].get()),
+          bufferHandles);
+      bool isNeuterable = true;
+      for (size_t j = 0; j < bufferHandles.size(); ++j)

[haraken, 2016/10/19 09:49:28]

Ditto.

[nhiroki, 2016/10/20 07:44:02]

Done.

+        isNeuterable &= bufferHandles[j]->IsNeuterable();
+
+      DOMArrayBufferBase* toTransfer = arrayBuffers[i];
+      if (!isNeuterable) {
+        toTransfer =
+            DOMArrayBuffer::create(arrayBuffers[i]->buffer()->data(),
+                                   arrayBuffers[i]->buffer()->byteLength());
+      }
+      bool result = toTransfer->transfer(contents->at(i));
+      if (!result) {
+        exceptionState.throwDOMException(
+            DataCloneError, "ArrayBuffer at index " + String::number(i) +
+                                " could not be transferred.");
+        return nullptr;
+      }
+
+      if (isNeuterable)
+        for (size_t j = 0; j < bufferHandles.size(); ++j)

[haraken, 2016/10/19 09:49:28]

Ditto.

[nhiroki, 2016/10/20 07:44:02]

Done.

+          bufferHandles[j]->Neuter();
+    }
+  }
+  return contents;
+}
+
 void SerializedScriptValue::registerMemoryAllocatedWithCurrentScriptContext() {
   if (m_externallyAllocatedMemory)
     return;
@@ -410,8 +420,4 @@ void SerializedScriptValue::registerMemoryAllocatedWithCurrentScriptContext() {
       m_externallyAllocatedMemory);
 }
 
-bool SerializedScriptValue::containsTransferableArrayBuffer() const {
-  return m_arrayBufferContentsArray && !m_arrayBufferContentsArray->isEmpty();
-}
-
 }  // namespace blink