Support ImageData cloning in the V8-based structured clone path.

third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp

Index: third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp b/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
index 2e03c090e611b9e57232bb40b00babf95f65f639..1674215d708636a7810e638e281690610b097b9d 100644
--- a/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
@@ -5,7 +5,9 @@
 #include "bindings/core/v8/serialization/V8ScriptValueSerializer.h"
 
 #include "bindings/core/v8/ToV8.h"
+#include "bindings/core/v8/V8ImageData.h"
 #include "core/dom/DOMArrayBufferBase.h"
+#include "core/html/ImageData.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "wtf/AutoReset.h"
 
@@ -83,6 +85,22 @@ void V8ScriptValueSerializer::transfer(Transferables* transferables, ExceptionSt
     }
 }
 
+bool V8ScriptValueSerializer::writeDOMObject(ScriptWrappable* wrappable, ExceptionState& exceptionState)
+{
+    const WrapperTypeInfo* wrapperTypeInfo = wrappable->wrapperTypeInfo();
+    if (wrapperTypeInfo == &V8ImageData::wrapperTypeInfo) {

[esprehn, 2016/09/16 18:41:27]

fwiw I wonder if we should use a HashMap of <WrapperTypeInfo*, WTF::Function>
instead and modules should just register() all the types with the map instead of
this virtual override dance. It would mean that modules no longer needs to
subclass this thing, and individual modules can just register their serializers
and the function would take a serializer instance.

[jbroman, 2016/09/16 19:40:02]

I suggested doing that to ScriptValueSerializer a while back, and got push back.
And to be honest, I now do think it's simpler this way, especially because you'd
need a similar map (from SerializationTag) on the other side, so you'd have each
one implementing two WTF::Functions. The natural response to that is to make it
a class that gets registered with a WrapperTypeInfo* and a SerializationTag.
It's not even a perfectly clean abstraction, because some, like Blob and
MessagePort, are special snowflakes that need to do particular things
(WebBlobInfoArray needs to be exported so that the storage layer can manage the
blob/indexeddb connection; message port entanglement is currently done
specially).

+        ImageData* imageData = wrappable->toImpl<ImageData>();
+        DOMUint8ClampedArray* pixels = imageData->data();
+        writeTag(ImageDataTag);
+        writeUint32(imageData->width());
+        writeUint32(imageData->height());
+        writeUint32(pixels->length());
+        writeRawBytes(pixels->data(), pixels->length());
+        return true;
+    }
+    return false;
+}
+
 void V8ScriptValueSerializer::ThrowDataCloneError(v8::Local<v8::String> v8Message)
 {
     DCHECK(m_exceptionState);
@@ -93,4 +111,27 @@ void V8ScriptValueSerializer::ThrowDataCloneError(v8::Local<v8::String> v8Messag
     V8ThrowException::throwException(m_scriptState->isolate(), exception);
 }
 
+v8::Maybe<bool> V8ScriptValueSerializer::WriteHostObject(v8::Isolate* isolate, v8::Local<v8::Object> object)
+{
+    DCHECK(m_exceptionState);
+    DCHECK_EQ(isolate, m_scriptState->isolate());
+    ExceptionState exceptionState(
+        isolate, m_exceptionState->context(),
+        m_exceptionState->interfaceName(), m_exceptionState->propertyName());
+
+    if (!V8DOMWrapper::isWrapper(isolate, object)) {
+        exceptionState.throwDOMException(DataCloneError, "An object could not be cloned.");
+        return v8::Nothing<bool>();
+    }
+    ScriptWrappable* wrappable = toScriptWrappable(object);
+    bool wroteDOMObject = writeDOMObject(wrappable, exceptionState);
+    if (wroteDOMObject) {
+        DCHECK(!exceptionState.hadException());
+        return v8::Just(true);
+    }
+    if (!exceptionState.hadException())
+        exceptionState.throwDOMException(DataCloneError, "An object could not be cloned.");

[haraken, 2016/09/16 14:17:53]

If you want to pass in exceptionState to writeDOMObject(), it would be better to
set DataCloneError in writeDOMObject().

[jbroman, 2016/09/16 14:34:33]

I did it this way with the goal of making it simpler when there's an override of
writeDOMObject. I expect this to look roughly like:

bool V8ScriptValueSerializerForModules::writeDOMObject(ScriptWrappable*
wrappable, ExceptionState& exceptionState)
{
    if (V8ScriptValueSerializer::writeDOMObject(wrappable, exceptionState))
        return true;
    if (exceptionState.hadException())
        return false;
    // check for modules types
    return false;
}

If V8ScriptValueSerializer::writeDOMObject throws an exception for objects which
aren't valid _core_ types, then V8ScriptValueSerializerForModules won't have an
opportunity to return a valid object (unless it clears the exception, but
throwing and then clearing an exception seems weird).

There needs to be a way for V8ScriptValueSerializer to indicate to the derived
class that it was not able to serialize the object -- but not because it saw any
particular error, just because it didn't recognize the kind of object.

+    return v8::Nothing<bool>();
+}
+
 } // namespace blink