[wasm] Support wasm module structured cloning.

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ScriptValueSerializer.h"
 
 #include "bindings/core/v8/Transferables.h"
 #include "bindings/core/v8/V8ArrayBuffer.h"
 #include "bindings/core/v8/V8ArrayBufferView.h"
 #include "bindings/core/v8/V8Blob.h"
@@ skipping 850 matching lines @@
         return writeAndGreyArrayBuffer(object, next);
     }
     if (object->IsSharedArrayBuffer()) {
         uint32_t index;
         if (!m_transferredArrayBuffers.tryGet(object, &index)) {
             return handleError(Status::DataCloneError, "A SharedArrayBuffer could not be cloned.", next);
         }
         return writeTransferredSharedArrayBuffer(object, index, next);
     }
 
+    if (object->IsWebAssemblyCompiledModule()) {
+        return writeWasmCompiledModule(object, next);
+    }

[jsbell, 2016/08/18 16:48:02]

Nit: no {} necessary for single line ifs

[Mircea Trofin, 2016/08/18 17:34:08]

Acknowledged.

[Mircea Trofin, 2016/08/18 23:02:59]

Done.

     // Transferable only objects
     if (V8MessagePort::hasInstance(object, isolate())) {
         uint32_t index;
         if (!m_transferredMessagePorts.tryGet(object, &index)) {
             return handleError(Status::DataCloneError, "A MessagePort could not be cloned.", next);
         }
         m_writer.writeTransferredMessagePort(index);
         return nullptr;
     }
     if (V8OffscreenCanvas::hasInstance(object, isolate())) {
@@ skipping 287 matching lines @@
     //
     // We do need to grey the underlying buffer before we grey its view, however;
     // ArrayBuffers may be shared, so they need to be given reference IDs, and an
     // ArrayBufferView cannot be constructed without a corresponding ArrayBuffer
     // (or without an additional tag that would allow us to do two-stage construction
     // like we do for Objects and Arrays).
     greyObject(object);
     return nullptr;
 }
 
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeWasmCompiledModule(v8::Local<v8::Object> object, StateBase* next)
+{
+    v8::Local<v8::WasmCompiledModule> wasmModule = object.As<v8::WasmCompiledModule>();
+    v8::WasmCompiledModule::SerializedModule data = wasmModule->Serialize();

[jsbell, 2016/08/18 16:48:02]

Is this a copy, or does SerializedModule point into the existing bytes? If it's
a copy, can we know the size before we call Serialize(), and change Serialize()
to take a writer?

(that could be follow-up work to avoid the copy if necessary and possible)

[Mircea Trofin, 2016/08/18 17:34:08]

SerializedModule points to existing bytes, at the moment.

I am overall concerned with the need to copy bytes. I would prefer what you
suggest - taking some form of writer, and, in addition, avoiding the need to
allocate contiguous memory for the data. Wasm compiled modules may be assumed to
be in the order of tens of megs, at least, so I'm concerned both with copying
and with the memory pressure the contiguous memory requirement creates. On that,
is segmented writing something currently supported?

I was planning to look into those details as part of next steps. My first goal
was to ensure the end to end story holds up. Any pushback to staging it this
way?

[jsbell, 2016/08/18 18:13:14]

Staging is great! (I'm a fan of leaving TODOs in the code in that case)

[Mircea Trofin, 2016/08/18 23:02:59]

Done.

+    m_writer.append(WasmModuleTag);
+    m_writer.doWriteUint32(static_cast<uint32_t>(data.second));
+    m_writer.append(data.first.get(), static_cast<int>(data.second));
+    return nullptr;
+}
+
 ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer(v8::Local<v8::Object> object, StateBase* next)
 {
     DOMArrayBuffer* arrayBuffer = V8ArrayBuffer::toImpl(object);
     if (!arrayBuffer)
         return nullptr;
     if (arrayBuffer->isNeutered())
         return handleError(Status::DataCloneError, "An ArrayBuffer is neutered and could not be cloned.", next);
 
     uint32_t index;
     if (m_transferredArrayBuffers.tryGet(object, &index)) {
@@ skipping 280 matching lines @@
         break;
     }
     case ArrayBufferViewTag: {
         if (!m_version)
             return false;
         if (!readArrayBufferView(value, deserializer))
             return false;
         deserializer.pushObjectReference(*value);
         break;
     }
+    case WasmModuleTag: {
+        if (!readWasmCompiledModule(value))
+            return false;
+        deserializer.pushObjectReference(*value);
+        break;
+    }
     case ArrayBufferTag: {
         if (!m_version)
             return false;
         if (!readArrayBuffer(value))
             return false;
         deserializer.pushObjectReference(*value);
         break;
     }
     case GenerateFreshObjectTag: {
         if (!m_version)
@@ skipping 332 matching lines @@
     uint32_t byteLength;
     if (!doReadUint32(&byteLength))
         return nullptr;
     if (m_position + byteLength > m_length)
         return nullptr;
     const void* bufferStart = m_buffer + m_position;
     m_position += byteLength;
     return DOMArrayBuffer::create(bufferStart, byteLength);
 }
 
+bool SerializedScriptValueReader::readWasmCompiledModule(v8::Local<v8::Value>* value)
+{
+    uint32_t size = 0;
+    if (!doReadUint32(&size))
+        return false;
+    if (m_position + size > m_length)
+        return false;
+    const uint8_t* buf = m_buffer + m_position;
+    v8::WasmCompiledModule::SerializedModule data = {
+        std::unique_ptr<const uint8_t[]>(buf),

[jsbell, 2016/08/18 16:48:02]

Uh... it seems like the SerializedModule constructor should not take a
unique_ptr if this is safe. If it's not claiming ownership of the data (i.e.
doing a std::move on the ptr) why is it taking a unique_ptr in the first place?

[Mircea Trofin, 2016/08/18 17:34:08]

SerializedModule is just a pair used by both serializer and deserializer. The
idea was to enforce the contract that 
1) the deserializer doesn't own the data, and shouldn't delete it. Passing const
uint8_t* would't be enough, one can still delete that. A const std::unique_ptr&,
however, makes that trickier.
2) the serializer relinquishes the control of the data to the caller (hence the
unique_ptr)

But maybe that's too overboard. We could just pass to the deserializer the const
uint8_t* and the length and document the ownership aspect. The serializer and
deserializer wouldn't have symmetric APIs, but that's not a strong argument,
imho, especially when I doubt any symmetry would be left once we have a
non-copying, segmented design.

...and maybe there's another option?

[jsbell, 2016/08/18 18:13:14]

I'd start with this, pending any better ideas.

[Mircea Trofin, 2016/08/18 23:02:59]

Added a TODO, and will follow with the v8 CL + chrome CL subsequently

+        static_cast<size_t>(size)
+    };
+    v8::MaybeLocal<v8::WasmCompiledModule> retval =
+        v8::WasmCompiledModule::Deserialize(isolate(), data);

[jbroman, 2016/08/18 16:59:09]

If I read this correctly, every time the V8 version changes every compiled
module stored in IndexedDB will become invalid, and any attempt to read a record
containing a module will fail (IIRC this appears as the object being
deserialized as null).

https://github.com/WebAssembly/design/blob/master/JS.md#structured-clone-of-a...
doesn't mention this behaviour.

[Mircea Trofin, 2016/08/18 17:34:08]

That's correct, the intent is to not support deserialization across v8 versions.
Returning undefined is one option, throwing is another one (there may be more).
The spec leaves this detail unspecified (I'll follow up on that), and we still
need to figure out, for IndexedDB, what would happen with the stored bits in
this case. So there are a few loose ends still left to be tied, but the goal of
this CL is to be a first step and make sure the basic end to end plumbing works
as expected.

Does this staging approach for enabling the feature make sense?

[jbroman, 2016/08/18 17:55:50]

Seems fine, as long as this is addressed before shipping.

[jsbell, 2016/08/18 18:13:14]

I think what we settled on (in email conversations) is that we'd yield a Module
that was "neutered" in the same way as a closed Blob - an object makes it out of
the database but produces errors if you try and use it. But that may not have
made it into the spec.
+1 to staging. (and sprinkles of TODOs that link to tracking crbugs would be
awesome!)

[Mircea Trofin, 2016/08/18 23:02:59]

Done.

+    data.first.release();
+    m_position += size;
+    return retval.ToLocal(value);
+}
+
 bool SerializedScriptValueReader::readArrayBuffer(v8::Local<v8::Value>* value)
 {
     DOMArrayBuffer* arrayBuffer = doReadArrayBuffer();
     if (!arrayBuffer)
         return false;
     *value = toV8(arrayBuffer, m_scriptState->context()->Global(), isolate());
     return !value->IsEmpty();
 }
 
 bool SerializedScriptValueReader::readArrayBufferView(v8::Local<v8::Value>* value, ScriptValueDeserializer& deserializer)
@@ skipping 607 matching lines @@
         return false;
     uint32_t objectReference = m_openCompositeReferenceStack[m_openCompositeReferenceStack.size() - 1];
     m_openCompositeReferenceStack.shrink(m_openCompositeReferenceStack.size() - 1);
     if (objectReference >= m_objectPool.size())
         return false;
     *object = m_objectPool[objectReference];
     return true;
 }
 
 } // namespace blink