Fix out-of-memory crashes related to ArrayBuffer allocation

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 d4906a8b6133873bc43d7d31a33c98a721c514bc..d6b5f739f05e0d3d32a8ccabfaef973f132a7574 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
@@ -252,7 +252,7 @@ void SerializedScriptValueWriter::writeArrayBufferView(const DOMArrayBufferView&
 {
     append(ArrayBufferViewTag);
 #if ENABLE(ASSERT)
-    ASSERT(static_cast<const uint8_t*>(arrayBufferView.bufferBase()->data()) + arrayBufferView.byteOffset() ==
+    ASSERT(static_cast<const uint8_t*>(arrayBufferView.bufferBaseOrNull()->data()) + arrayBufferView.byteOffset() ==
         static_cast<const uint8_t*>(arrayBufferView.baseAddress()));
 #endif
     DOMArrayBufferView::ViewType type = arrayBufferView.type();
@@ -1003,10 +1003,10 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer
     ASSERT(!object.IsEmpty());
     DOMArrayBufferView* arrayBufferView = V8ArrayBufferView::toImpl(object);
     if (!arrayBufferView)
-        return 0;
-    if (!arrayBufferView->bufferBase())
+        return nullptr;
+    if (!arrayBufferView->bufferBaseOrNull())
         return handleError(DataCloneError, "An ArrayBuffer could not be cloned.", next);
-    v8::Local<v8::Value> underlyingBuffer = toV8(arrayBufferView->bufferBase(), m_scriptState->context()->Global(), isolate());
+    v8::Local<v8::Value> underlyingBuffer = toV8(arrayBufferView->bufferBaseOrNull(), m_scriptState->context()->Global(), isolate());
     if (underlyingBuffer.IsEmpty())
         return handleError(DataCloneError, "An ArrayBuffer could not be cloned.", next);
     StateBase* stateOut = doSerializeArrayBuffer(underlyingBuffer, next);
@@ -1580,7 +1580,10 @@ bool SerializedScriptValueReader::readImageData(v8::Local<v8::Value>* value)
         return false;
     if (m_position + pixelDataLength > m_length)
         return false;
-    ImageData* imageData = ImageData::create(IntSize(width, height));
+    NonThrowableExceptionState exceptionState;

[haraken, 2015/10/29 16:24:34]

Why can't we use a normal ExecptionState?

[Justin Novosad, 2015/10/29 18:26:02]

Added an explanatory comment.

+    ImageData* imageData = ImageData::create(IntSize(width, height), exceptionState);
+    if (exceptionState.hadException())
+        return false;
     DOMUint8ClampedArray* pixelArray = imageData->data();
     ASSERT(pixelArray);
     ASSERT(pixelArray->length() >= pixelDataLength);
@@ -1604,7 +1607,7 @@ bool SerializedScriptValueReader::readCompositorProxy(v8::Local<v8::Value>* valu
     return !value->IsEmpty();
 }
 
-PassRefPtr<DOMArrayBuffer> SerializedScriptValueReader::doReadArrayBuffer()
+PassRefPtr<DOMArrayBuffer> SerializedScriptValueReader::doReadArrayBufferOrNull()
 {
     uint32_t byteLength;
     if (!doReadUint32(&byteLength))
@@ -1613,14 +1616,18 @@ PassRefPtr<DOMArrayBuffer> SerializedScriptValueReader::doReadArrayBuffer()
         return nullptr;
     const void* bufferStart = m_buffer + m_position;
     m_position += byteLength;
-    return DOMArrayBuffer::create(bufferStart, byteLength);
+    return DOMArrayBuffer::createOrNull(bufferStart, byteLength);
 }
 
 bool SerializedScriptValueReader::readArrayBuffer(v8::Local<v8::Value>* value)
 {
-    RefPtr<DOMArrayBuffer> arrayBuffer = doReadArrayBuffer();
-    if (!arrayBuffer)
-        return false;
+    RefPtr<DOMArrayBuffer> arrayBuffer = doReadArrayBufferOrNull();
+    // FIXME(crbug.com/536816): Instead of the following assert, we should consider doing:

[haraken, 2015/10/29 16:24:34]

FIXME => TODO(junov)

+    //   if (!arrayBuffer)
+    //       return false;
+    // To do that, we need to make sure that call sites would react correctly
+    // in this case, with the value not having been set.
+    RELEASE_ASSERT(arrayBuffer); // This is essentially an out of memory crash
     *value = toV8(arrayBuffer.release(), m_scriptState->context()->Global(), isolate());
     return !value->IsEmpty();
 }