[Fetch API] Fetch response stream should enqueue Uint8Array

https://bugs.webkit.org/show_bug.cgi?id=160083

Patch by Youenn Fablet <youenn@apple.com> on 2016-07-23
Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

* web-platform-tests/fetch/api/resources/utils.js:

Source/WebCore:

Covered by updated tests.

Before enqueuing, ReadableStreamController::enqueue will convert ArrayBuffer as Uint8Array.
It also returns a boolean whether the operation is successful or not.

If returned value is false, calling code will stop loading or if everything is loaded it will refrain from closing the stream.
The enqueuing should be succesful except in OutOfMemory cases. This case is not yet handled in test cases.

Updated the code to remove templated enqueuing as Fetch has no use of it.

* Modules/fetch/FetchBody.cpp:
(WebCore::FetchBody::consumeAsStream): Do not close the stream if enqueuing failed.
* Modules/fetch/FetchBodyOwner.cpp:
(WebCore::FetchBodyOwner::blobChunk): Stop blob loading if enqueuing failed.
* Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::BodyLoader::didReceiveData): Stop resource loading if enqueuing failed.
(WebCore::FetchResponse::consumeBodyAsStream): Ditto.
* Modules/fetch/FetchResponseSource.h:
* bindings/js/ReadableStreamController.h:
(WebCore::ReadableStreamController::enqueue):
(WebCore::ReadableStreamController::enqueue<RefPtr<JSC::ArrayBuffer>>): Deleted.

Canonical link: https://commits.webkit.org/178268@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203637 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog

@@ -1,3 +1,12 @@
+2016-07-23  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch API] Fetch response stream should enqueue Uint8Array
+        https://bugs.webkit.org/show_bug.cgi?id=160083
+
+        Reviewed by Sam Weinig.
+
+        * web-platform-tests/fetch/api/resources/utils.js:
+
 2016-07-22  Chris Dumez  <cdumez@apple.com>
 
         Parameter to HTMLCollection.item() / namedItem() should be mandatory

LayoutTests/imported/w3c/web-platform-tests/fetch/api/resources/utils.js

@@ -59,6 +59,7 @@ function validateBufferFromString(buffer, expectedValue, message)
 function validateStreamFromString(reader, expectedValue, retrievedArrayBuffer) {
   return reader.read().then(function(data) {
     if (!data.done) {
+      assert_true(data.value instanceof Uint8Array, "Fetch ReadableStream chunks should be Uint8Array");
       var newBuffer;
       if (retrievedArrayBuffer) {
         newBuffer =  new ArrayBuffer(data.value.length + retrievedArrayBuffer.length);

Source/WebCore/ChangeLog

@@ -1,3 +1,32 @@
+2016-07-23  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch API] Fetch response stream should enqueue Uint8Array
+        https://bugs.webkit.org/show_bug.cgi?id=160083
+
+        Reviewed by Sam Weinig.
+
+        Covered by updated tests.
+
+        Before enqueuing, ReadableStreamController::enqueue will convert ArrayBuffer as Uint8Array.
+        It also returns a boolean whether the operation is successful or not.
+
+        If returned value is false, calling code will stop loading or if everything is loaded it will refrain from closing the stream.
+        The enqueuing should be succesful except in OutOfMemory cases. This case is not yet handled in test cases.
+
+        Updated the code to remove templated enqueuing as Fetch has no use of it.
+
+        * Modules/fetch/FetchBody.cpp:
+        (WebCore::FetchBody::consumeAsStream): Do not close the stream if enqueuing failed.
+        * Modules/fetch/FetchBodyOwner.cpp:
+        (WebCore::FetchBodyOwner::blobChunk): Stop blob loading if enqueuing failed.
+        * Modules/fetch/FetchResponse.cpp:
+        (WebCore::FetchResponse::BodyLoader::didReceiveData): Stop resource loading if enqueuing failed.
+        (WebCore::FetchResponse::consumeBodyAsStream): Ditto.
+        * Modules/fetch/FetchResponseSource.h:
+        * bindings/js/ReadableStreamController.h:
+        (WebCore::ReadableStreamController::enqueue):
+        (WebCore::ReadableStreamController::enqueue<RefPtr<JSC::ArrayBuffer>>): Deleted.
+
 2016-07-22  Youenn Fablet  <youenn@apple.com>
 
         Use a private property to implement FetchResponse.body getter

Source/WebCore/Modules/fetch/FetchBody.cpp

@@ -151,14 +151,14 @@ void FetchBody::consumeAsStream(FetchBodyOwner& owner, FetchResponseSource& sour
 
     switch (m_type) {
     case Type::ArrayBuffer:
-        source.enqueue(m_data);
-        source.close();
+        ASSERT(m_data);
+        if (source.enqueue(RefPtr<JSC::ArrayBuffer>(m_data)))
+            source.close();
         return;
     case Type::Text: {
         Vector<uint8_t> data = extractFromText();
-        // FIXME: We should not close the source if ArrayBuffer;;tryCreate returns null.
-        source.enqueue(ArrayBuffer::tryCreate(data.data(), data.size()));
-        source.close();
+        if (source.enqueue(ArrayBuffer::tryCreate(data.data(), data.size())))
+            source.close();
         return;
     }
     case Type::Blob:

Source/WebCore/Modules/fetch/FetchBodyOwner.cpp

@@ -204,10 +204,11 @@ void FetchBodyOwner::blobLoadingFailed()
 
 void FetchBodyOwner::blobChunk(const char* data, size_t size)
 {
+    ASSERT(data);
 #if ENABLE(STREAMS_API)
     ASSERT(m_readableStreamSource);
-    // FIXME: If ArrayBuffer::tryCreate returns null, we should probably cancel the load.
-    m_readableStreamSource->enqueue(ArrayBuffer::tryCreate(data, size));
+    if (!m_readableStreamSource->enqueue(ArrayBuffer::tryCreate(data, size)))
+        stop();
 #else
     UNUSED_PARAM(data);
     UNUSED_PARAM(size);

Source/WebCore/Modules/fetch/FetchResponse.cpp

@@ -203,8 +203,8 @@ void FetchResponse::BodyLoader::didReceiveData(const char* data, size_t size)
 #if ENABLE(STREAMS_API)
     ASSERT(m_response.m_readableStreamSource);
 
-    // FIXME: If ArrayBuffer::tryCreate returns null, we should probably cancel the load.
-    m_response.m_readableStreamSource->enqueue(ArrayBuffer::tryCreate(data, size));
+    if (!m_response.m_readableStreamSource->enqueue(ArrayBuffer::tryCreate(data, size)))
+        stop();
 #else
     UNUSED_PARAM(data);
     UNUSED_PARAM(size);
@@ -237,7 +237,7 @@ void FetchResponse::consumeBodyAsStream()
     if (body().type() != FetchBody::Type::Loading) {
         body().consumeAsStream(*this, *m_readableStreamSource);
         if (!m_readableStreamSource->isStarting())
-            m_readableStreamSource = nullptr;        
+            m_readableStreamSource = nullptr;
         return;
     }
 
@@ -246,9 +246,8 @@ void FetchResponse::consumeBodyAsStream()
     RefPtr<SharedBuffer> data = m_bodyLoader->startStreaming();
     if (data) {
         // FIXME: We might want to enqueue each internal SharedBuffer chunk as an individual ArrayBuffer.
-        // Also, createArrayBuffer might return nullptr which will lead to erroring the stream.
-        // We might want to cancel the load and rename createArrayBuffer to tryCreateArrayBuffer.
-        m_readableStreamSource->enqueue(data->createArrayBuffer());
+        if (!m_readableStreamSource->enqueue(data->createArrayBuffer()))
+            stop();
     }
 }
 

Source/WebCore/Modules/fetch/FetchResponseSource.h

@@ -32,6 +32,10 @@
 
 #include "ReadableStreamSource.h"
 
+namespace JSC {
+class ArrayBuffer;
+};
+
 namespace WebCore {
 
 class FetchResponse;
@@ -40,7 +44,7 @@ class FetchResponseSource final : public ReadableStreamSource {
 public:
     FetchResponseSource(FetchResponse&);
 
-    template<typename T> void enqueue(const T& t) { controller().enqueue(t); }
+    bool enqueue(RefPtr<JSC::ArrayBuffer>&& chunk) { return controller().enqueue(WTFMove(chunk)); }
     void close();
     void error(const String&);
 

Source/WebCore/bindings/js/ReadableStreamController.h

@@ -35,6 +35,7 @@
 #include "JSReadableStreamController.h"
 #include <runtime/JSCJSValue.h>
 #include <runtime/JSCJSValueInlines.h>
+#include <runtime/TypedArrays.h>
 
 namespace WebCore {
 
@@ -46,8 +47,7 @@ class ReadableStreamController {
 
     static JSC::JSValue invoke(JSC::ExecState&, JSC::JSObject&, const char*, JSC::JSValue);
 
-    template<class ResolveResultType>
-    void enqueue(const ResolveResultType&);
+    bool enqueue(RefPtr<JSC::ArrayBuffer>&&);
 
     template<class ResolveResultType>
     void error(const ResolveResultType&);
@@ -72,16 +72,21 @@ inline JSDOMGlobalObject* ReadableStreamController::globalObject() const
     return static_cast<JSDOMGlobalObject*>(m_jsController->globalObject());
 }
 
-template<>
-inline void ReadableStreamController::enqueue<RefPtr<JSC::ArrayBuffer>>(const RefPtr<JSC::ArrayBuffer>& result)
+inline bool ReadableStreamController::enqueue(RefPtr<JSC::ArrayBuffer>&& buffer)
 {
     JSC::ExecState& state = *globalObject()->globalExec();
     JSC::JSLockHolder locker(&state);
 
-    if (result)
-        enqueue(state, toJS(&state, globalObject(), result.get()));
-    else
+    if (!buffer) {
         error(state, createOutOfMemoryError(&state));
+        return false;
+    }
+    auto length = buffer->byteLength();
+    auto chunk = JSC::Uint8Array::create(WTFMove(buffer), 0, length);
+    ASSERT(chunk);
+    enqueue(state, toJS(&state, globalObject(), chunk.get()));
+    ASSERT(!state.hadException());
+    return true;
 }
 
 template<>