Change code path for structured cloning ImageBitmap

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

Index: third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
index f143d25720079fb6e9fbe46b92f409591d3cfd6e..35d1d56345a9f5959b3032c980ea24f40e44d59f 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
@@ -312,10 +312,11 @@ void SerializedScriptValueWriter::writeImageData(uint32_t width, uint32_t height
     doWriteImageData(width, height, pixelData, pixelDataLength);
 }
 
-void SerializedScriptValueWriter::writeImageBitmap(uint32_t width, uint32_t height, uint32_t isOriginClean, const uint8_t* pixelData, uint32_t pixelDataLength)
+void SerializedScriptValueWriter::writeImageBitmap(uint32_t width, uint32_t height, uint32_t isOriginClean, uint32_t isPremultiplied, const uint8_t* pixelData, uint32_t pixelDataLength)
 {
     append(ImageBitmapTag);
     append(isOriginClean);
+    append(isPremultiplied);
     doWriteImageData(width, height, pixelData, pixelDataLength);
 }
 
@@ -1145,8 +1146,8 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyImageBitmap
         m_writer.writeTransferredImageBitmap(index);
     } else {
         greyObject(object);
-        std::unique_ptr<uint8_t[]> pixelData = imageBitmap->copyBitmapData(PremultiplyAlpha);
-        m_writer.writeImageBitmap(imageBitmap->width(), imageBitmap->height(), static_cast<uint32_t>(imageBitmap->originClean()), pixelData.get(), imageBitmap->width() * imageBitmap->height() * 4);
+        std::unique_ptr<uint8_t[]> pixelData = imageBitmap->copyBitmapData(imageBitmap->isPremultiplied() ? PremultiplyAlpha : DontPremultiplyAlpha, BGRAColorType);
+        m_writer.writeImageBitmap(imageBitmap->width(), imageBitmap->height(), static_cast<uint32_t>(imageBitmap->originClean()), static_cast<uint32_t>(imageBitmap->isPremultiplied()), pixelData.get(), imageBitmap->width() * imageBitmap->height() * 4);
     }
     return nullptr;
 }
@@ -1748,32 +1749,33 @@ bool SerializedScriptValueReader::readNumberObject(v8::Local<v8::Value>* value)
     return true;
 }
 
-// Helper function shared by readImageData and readImageBitmap
-ImageData* SerializedScriptValueReader::doReadImageData()
+// Helper function used by readImageData and readImageBitmap.
+bool SerializedScriptValueReader::doReadImageDataProperties(uint32_t* width, uint32_t* height, uint32_t* pixelDataLength)
+{
+    if (!doReadUint32(width))
+        return false;
+    if (!doReadUint32(height))
+        return false;
+    if (!doReadUint32(pixelDataLength))
+        return false;
+    if (m_position + *pixelDataLength > m_length || m_length - m_position < *pixelDataLength)

[jbroman, 2016/06/29 15:10:33]

nit: I don't think you need to do the full check twice here, just account for
overflow.

The latter, "m_length - m_position < *pixelDataLength" is sufficient, as long as
you can rely on m_position not to be greater than m_length (which would
underflow). I think doReadUint32 promises that, so just that condition should
work. (My original suggestion also verified it, but that's not strictly
necessary).

As long as that underflow doesn't happen, this is fine and we don't need to do
the check with addition as well.

[xidachen, 2016/06/29 15:51:14]

Done.

+        return false;
+    return true;
+}
+
+bool SerializedScriptValueReader::readImageData(v8::Local<v8::Value>* value)
 {
     uint32_t width;
     uint32_t height;
     uint32_t pixelDataLength;
-    if (!doReadUint32(&width))
-        return nullptr;
-    if (!doReadUint32(&height))
-        return nullptr;
-    if (!doReadUint32(&pixelDataLength))
-        return nullptr;
-    if (m_position + pixelDataLength > m_length)
-        return nullptr;
+    if (!doReadImageDataProperties(&width, &height, &pixelDataLength))
+        return false;
     ImageData* imageData = ImageData::create(IntSize(width, height));
     DOMUint8ClampedArray* pixelArray = imageData->data();
     ASSERT(pixelArray);
     ASSERT(pixelArray->length() >= pixelDataLength);
     memcpy(pixelArray->data(), m_buffer + m_position, pixelDataLength);
     m_position += pixelDataLength;
-    return imageData;
-}
-
-bool SerializedScriptValueReader::readImageData(v8::Local<v8::Value>* value)
-{
-    ImageData* imageData = doReadImageData();
     if (!imageData)
         return false;
     *value = toV8(imageData, m_scriptState->context()->Global(), isolate());
@@ -1785,12 +1787,18 @@ bool SerializedScriptValueReader::readImageBitmap(v8::Local<v8::Value>* value)
     uint32_t isOriginClean;
     if (!doReadUint32(&isOriginClean))
         return false;
-    ImageData* imageData = doReadImageData();
-    if (!imageData)
+    uint32_t isPremultiplied;
+    if (!doReadUint32(&isPremultiplied))
+        return false;
+    uint32_t width;
+    uint32_t height;
+    uint32_t pixelDataLength;
+    if (!doReadImageDataProperties(&width, &height, &pixelDataLength))
         return false;
-    ImageBitmapOptions options;
-    options.setPremultiplyAlpha("none");
-    ImageBitmap* imageBitmap = ImageBitmap::create(imageData, IntRect(0, 0, imageData->width(), imageData->height()), options, true, isOriginClean);
+    std::unique_ptr<uint8_t[]> pixelData = wrapArrayUnique(new uint8_t[pixelDataLength]);
+    memcpy(pixelData.get(), m_buffer + m_position, pixelDataLength);
+    m_position += pixelDataLength;
+    ImageBitmap* imageBitmap = ImageBitmap::create(std::move(pixelData), width, height, isPremultiplied, isOriginClean);
     if (!imageBitmap)
         return false;
     *value = toV8(imageBitmap, m_scriptState->context()->Global(), isolate());