[Readable Streams API] Implement ReadableStreamBYOBRequest respond() (readable stream...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2017 18:07:49 +0000 (18:07 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2017 18:07:49 +0000 (18:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169759

Patch by Romain Bellessort <romain.bellessort@crf.canon.fr> on 2017-03-22
Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Updated test expectations.

* web-platform-tests/streams/readable-byte-streams/general-expected.txt:
* web-platform-tests/streams/readable-byte-streams/general.dedicatedworker-expected.txt:

Source/WebCore:

Implemented readable state part of respond() function. Parts of code cannot
currently be reached as they require ReadableStreamBYOBReader to be implemented.

Added new tests and updated expectations.

* Modules/streams/ReadableByteStreamInternals.js:
(readableByteStreamControllerEnqueue): Name shortened.
(readableByteStreamControllerRespondInternal): Updated with readable state case.
(cloneArrayBuffer): Added.
(readableByteStreamControllerRespondInReadableState): Added.
(readableByteStreamControllerRespondInClosedState): Updated.
(readableByteStreamControllerProcessPullDescriptors): Added.
(readableByteStreamControllerFillDescriptorFromQueue): Added.
(readableByteStreamControllerShiftPendingDescriptor): Name shortened.
(readableByteStreamControllerCommitDescriptor): Name shortened.
(readableByteStreamControllerConvertDescriptor): Name shortened.

LayoutTests:

Added new tests to check code that can currently be reached.

* streams/readable-stream-byob-request-expected.txt: Updated.
* streams/readable-stream-byob-request.js: Updated.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214265 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/streams/readable-byte-streams/general-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-byte-streams/general.dedicatedworker-expected.txt
LayoutTests/streams/readable-stream-byob-request-expected.txt
LayoutTests/streams/readable-stream-byob-request.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableByteStreamInternals.js

index bd9684c..2e74791 100644 (file)
@@ -1,3 +1,15 @@
+2017-03-22  Romain Bellessort  <romain.bellessort@crf.canon.fr>
+
+        [Readable Streams API] Implement ReadableStreamBYOBRequest respond() (readable stream state)
+        https://bugs.webkit.org/show_bug.cgi?id=169759
+
+        Reviewed by Youenn Fablet.
+
+        Added new tests to check code that can currently be reached.
+
+        * streams/readable-stream-byob-request-expected.txt: Updated.
+        * streams/readable-stream-byob-request.js: Updated.
+
 2017-03-22  Youenn Fablet  <youenn@apple.com>
 
         Move LayoutTests/webrtc/rtcconfiguration-icecandidatepoolsize.html to web-platform-tests
index ecce2da..01ab012 100644 (file)
@@ -1,3 +1,15 @@
+2017-03-22  Romain Bellessort  <romain.bellessort@crf.canon.fr>
+
+        [Readable Streams API] Implement ReadableStreamBYOBRequest respond() (readable stream state)
+        https://bugs.webkit.org/show_bug.cgi?id=169759
+
+        Reviewed by Youenn Fablet.
+
+        Updated test expectations.
+
+        * web-platform-tests/streams/readable-byte-streams/general-expected.txt:
+        * web-platform-tests/streams/readable-byte-streams/general.dedicatedworker-expected.txt:
+
 2017-03-22  Youenn Fablet  <youenn@apple.com>
 
         Move LayoutTests/webrtc/rtcconfiguration-icecandidatepoolsize.html to web-platform-tests
index c7adc33..bc0d80e 100644 (file)
@@ -15,8 +15,8 @@ FAIL ReadableStream with byte source: Test that erroring a stream does not relea
 PASS ReadableStream with byte source: releaseLock() on ReadableStreamReader with pending read() must throw 
 PASS ReadableStream with byte source: Automatic pull() after start() 
 PASS ReadableStream with byte source: Automatic pull() after start() and read() 
-FAIL ReadableStream with byte source: autoAllocateChunkSize assert_equals: pull() must have been invoked once expected 1 but got 0
-FAIL ReadableStream with byte source: Mix of auto allocate and BYOB promise_test: Unhandled rejection with value: object "TypeError: Readable state is not yet supported"
+PASS ReadableStream with byte source: autoAllocateChunkSize 
+FAIL ReadableStream with byte source: Mix of auto allocate and BYOB promise_test: Unhandled rejection with value: object "TypeError: ReadableStreamBYOBReader is not implemented"
 PASS ReadableStream with byte source: Automatic pull() after start() and read(view) 
 PASS ReadableStream with byte source: enqueue(), getReader(), then read() 
 PASS ReadableStream with byte source: Push source that doesn't understand pull signal 
index c7adc33..bc0d80e 100644 (file)
@@ -15,8 +15,8 @@ FAIL ReadableStream with byte source: Test that erroring a stream does not relea
 PASS ReadableStream with byte source: releaseLock() on ReadableStreamReader with pending read() must throw 
 PASS ReadableStream with byte source: Automatic pull() after start() 
 PASS ReadableStream with byte source: Automatic pull() after start() and read() 
-FAIL ReadableStream with byte source: autoAllocateChunkSize assert_equals: pull() must have been invoked once expected 1 but got 0
-FAIL ReadableStream with byte source: Mix of auto allocate and BYOB promise_test: Unhandled rejection with value: object "TypeError: Readable state is not yet supported"
+PASS ReadableStream with byte source: autoAllocateChunkSize 
+FAIL ReadableStream with byte source: Mix of auto allocate and BYOB promise_test: Unhandled rejection with value: object "TypeError: ReadableStreamBYOBReader is not implemented"
 PASS ReadableStream with byte source: Automatic pull() after start() and read(view) 
 PASS ReadableStream with byte source: enqueue(), getReader(), then read() 
 PASS ReadableStream with byte source: Push source that doesn't understand pull signal 
index d5dbbc3..1a6cf1c 100644 (file)
@@ -8,6 +8,8 @@ PASS Calling respond() with a bytesWritten value which is not a number should th
 PASS Calling respond() with a positive infinity bytesWritten value should throw a RangeError 
 PASS Calling respond() with a bytesWritten value different from 0 when stream is closed should throw a TypeError 
 PASS Calling respond() with a bytesWritten value of 0 when stream is closed should succeed 
+PASS Calling respond() with a bytesWritten value greater than autoAllocateChunkSize should fail 
+PASS Calling respond() with a bytesWritten value lower than autoAllocateChunkSize should succeed 
 PASS ReadableStreamBYOBRequest instances should have the correct list of properties 
 PASS By default, byobRequest should be undefined 
 PASS byobRequest.view length should be equal to autoAllocateChunkSize 
@@ -17,4 +19,6 @@ PASS Calling respond() with a bytesWritten value which is not a number should th
 PASS Calling respond() with a positive infinity bytesWritten value should throw a RangeError 
 PASS Calling respond() with a bytesWritten value different from 0 when stream is closed should throw a TypeError 
 PASS Calling respond() with a bytesWritten value of 0 when stream is closed should succeed 
+PASS Calling respond() with a bytesWritten value greater than autoAllocateChunkSize should fail 
+PASS Calling respond() with a bytesWritten value lower than autoAllocateChunkSize should succeed 
 
index e77dadd..b4ad4c4 100644 (file)
@@ -198,4 +198,47 @@ test(function() {
 
 }, "Calling respond() with a bytesWritten value of 0 when stream is closed should succeed");
 
+test(function() {
+
+    let controller;
+    const rs = new ReadableStream({
+        autoAllocateChunkSize: 16,
+        start: function(c) {
+            controller = c;
+        },
+        type: "bytes"
+    });
+
+    rs.getReader().read();
+    const byobReq = controller.byobRequest;
+    assert_throws(new RangeError("bytesWritten value is too great"),
+        function() { byobReq.respond(17); });
+
+}, "Calling respond() with a bytesWritten value greater than autoAllocateChunkSize should fail");
+
+promise_test(function() {
+
+    const rs = new ReadableStream({
+        autoAllocateChunkSize: 16,
+        pull: function(controller) {
+            const br = controller.byobRequest;
+            br.view[0] = 1;
+            br.view[1] = 2;
+            br.respond(2);
+        },
+        type: "bytes"
+    });
+
+    return rs.getReader().read().then(result => {
+        assert_equals(result.value.byteLength, 2);
+        assert_equals(result.value.byteOffset, 0);
+        assert_equals(result.value.buffer.byteLength, 16);
+        assert_equals(result.value[0], 1);
+        assert_equals(result.value[1], 2);
+    });
+}, "Calling respond() with a bytesWritten value lower than autoAllocateChunkSize should succeed");
+
+// FIXME: when ReadableStreamBYOBReader is implemented, add tests with elementSize different from 1
+// so that more code can be covered.
+
 done();
index fa5808e..1cf348f 100644 (file)
@@ -1,3 +1,27 @@
+2017-03-22  Romain Bellessort  <romain.bellessort@crf.canon.fr>
+
+        [Readable Streams API] Implement ReadableStreamBYOBRequest respond() (readable stream state)
+        https://bugs.webkit.org/show_bug.cgi?id=169759
+
+        Reviewed by Youenn Fablet.
+
+        Implemented readable state part of respond() function. Parts of code cannot
+        currently be reached as they require ReadableStreamBYOBReader to be implemented.
+
+        Added new tests and updated expectations.
+
+        * Modules/streams/ReadableByteStreamInternals.js:
+        (readableByteStreamControllerEnqueue): Name shortened.
+        (readableByteStreamControllerRespondInternal): Updated with readable state case.
+        (cloneArrayBuffer): Added.
+        (readableByteStreamControllerRespondInReadableState): Added.
+        (readableByteStreamControllerRespondInClosedState): Updated.
+        (readableByteStreamControllerProcessPullDescriptors): Added.
+        (readableByteStreamControllerFillDescriptorFromQueue): Added.
+        (readableByteStreamControllerShiftPendingDescriptor): Name shortened.
+        (readableByteStreamControllerCommitDescriptor): Name shortened.
+        (readableByteStreamControllerConvertDescriptor): Name shortened.
+
 2017-03-22  Youenn Fablet  <youenn@apple.com>
 
         RTCPeerConnection is crashing if no backend is created
index 44367c1..3a755d8 100644 (file)
@@ -314,7 +314,7 @@ function readableByteStreamControllerEnqueue(controller, chunk)
 
     if (@readableStreamHasDefaultReader(stream)) {
         if (!stream.@reader.@readRequests.length)
-            @readableByteStreamControllerEnqueueChunkToQueue(controller, transferredBuffer, byteOffset, byteLength);
+            @readableByteStreamControllerEnqueueChunk(controller, transferredBuffer, byteOffset, byteLength);
         else {
             @assert(!controller.@queue.length);
             let transferredView = new @Uint8Array(transferredBuffer, byteOffset, byteLength);
@@ -331,10 +331,11 @@ function readableByteStreamControllerEnqueue(controller, chunk)
     }
 
     @assert(!@isReadableStreamLocked(stream));
-    @readableByteStreamControllerEnqueueChunkToQueue(controller, transferredBuffer, byteOffset, byteLength);
+    @readableByteStreamControllerEnqueueChunk(controller, transferredBuffer, byteOffset, byteLength);
 }
 
-function readableByteStreamControllerEnqueueChunkToQueue(controller, buffer, byteOffset, byteLength)
+// Spec name: readableByteStreamControllerEnqueueChunkToQueue.
+function readableByteStreamControllerEnqueueChunk(controller, buffer, byteOffset, byteLength)
 {
     "use strict";
 
@@ -372,12 +373,51 @@ function readableByteStreamControllerRespondInternal(controller, bytesWritten)
             @throwTypeError("bytesWritten is different from 0 even though stream is closed");
         @readableByteStreamControllerRespondInClosedState(controller, firstDescriptor);
     } else {
-        // FIXME: Also implement case of readable state (distinct patch to avoid adding too many different cases
-        // in a single patch).
-        @throwTypeError("Readable state is not yet supported");
+        @assert(stream.@state === @streamReadable);
+        @readableByteStreamControllerRespondInReadableState(controller, bytesWritten, firstDescriptor);
     }
 }
 
+function cloneArrayBuffer(srcBuffer, srcByteOffset, srcLength)
+{
+    "use strict";
+
+    // FIXME: Below implementation returns the appropriate data but does not perform
+    // exactly what is described by ECMAScript CloneArrayBuffer operation. This should
+    // be fixed in a follow up patch implementing cloneArrayBuffer in JSC (similarly to
+    // structuredCloneArrayBuffer implementation).
+    return srcBuffer.slice(srcByteOffset, srcByteOffset + srcLength);
+}
+
+function readableByteStreamControllerRespondInReadableState(controller, bytesWritten, pullIntoDescriptor)
+{
+    "use strict";
+
+    if (pullIntoDescriptor.bytesFilled + bytesWritten > pullIntoDescriptor.byteLength)
+        @throwRangeError("bytesWritten value is too great");
+
+    @assert(controller.@pendingPullIntos.length === 0 || controller.@pendingPullIntos[0] === pullIntoDescriptor);
+    @readableByteStreamControllerInvalidateBYOBRequest(controller);
+    pullIntoDescriptor.bytesFilled += bytesWritten;
+
+    if (pullIntoDescriptor.bytesFilled < pullIntoDescriptor.elementSize)
+        return;
+
+    @readableByteStreamControllerShiftPendingDescriptor(controller);
+    const remainderSize = pullIntoDescriptor.bytesFilled % pullIntoDescriptor.elementSize;
+
+    if (remainderSize > 0) {
+        const end = pullIntoDescriptor.byteOffset + pullIntoDescriptor.bytesFilled;
+        const remainder = @cloneArrayBuffer(pullIntoDescriptor.buffer, end - remainderSize, remainderSize);
+        @readableByteStreamControllerEnqueueChunk(controller, remainder, 0, remainder.byteLength);
+    }
+
+    pullIntoDescriptor.buffer = @transferBufferToCurrentRealm(pullIntoDescriptor.buffer);
+    pullIntoDescriptor.bytesFilled -= remainderSize;
+    @readableByteStreamControllerCommitDescriptor(controller.@controlledReadableStream, pullIntoDescriptor);
+    @readableByteStreamControllerProcessPullDescriptors(controller);
+}
+
 function readableByteStreamControllerRespondInClosedState(controller, firstDescriptor)
 {
     "use strict";
@@ -393,12 +433,87 @@ function readableByteStreamControllerRespondInClosedState(controller, firstDescr
         return;
 
     while (controller.@reader.@readIntoRequests.length > 0) {
-        let pullIntoDescriptor = @readableByteStreamControllerShiftPendingPullInto(controller);
-        @readableByteStreamControllerCommitPullIntoDescriptor(controller.@controlledReadableStream, pullIntoDescriptor);
+        let pullIntoDescriptor = @readableByteStreamControllerShiftPendingDescriptor(controller);
+        @readableByteStreamControllerCommitDescriptor(controller.@controlledReadableStream, pullIntoDescriptor);
+    }
+}
+
+// Spec name: readableByteStreamControllerProcessPullIntoDescriptorsUsingQueue (shortened for readability).
+function readableByteStreamControllerProcessPullDescriptors(controller)
+{
+    "use strict";
+
+    @assert(!controller.@closeRequested);
+    while (controller.@pendingPullIntos.length > 0) {
+        if (controller.@totalQueuedBytes === 0)
+            return;
+        let pullIntoDescriptor = controller.@pendingPullIntos[0];
+        if (@readableByteStreamControllerFillDescriptorFromQueue(controller, pullIntoDescriptor)) {
+            @readableByteStreamControllerShiftPendingDescriptor(controller);
+            @readableByteStreamControllerCommitDescriptor(controller.@controlledReadableStream, pullIntoDescriptor);
+        }
+    }
+}
+
+// Spec name: readableByteStreamControllerFillPullIntoDescriptorFromQueue (shortened for readability).
+function readableByteStreamControllerFillDescriptorFromQueue(controller, pullIntoDescriptor)
+{
+    "use strict";
+
+    const currentAlignedBytes = pullIntoDescriptor.bytesFilled - (pullIntoDescriptor.bytesFilled % pullIntoDescriptor.elementSize);
+    const maxBytesToCopy = controller.@totalQueuedBytes < pullIntoDescriptor.byteLength - pullIntoDescriptor.bytesFilled ?
+                controller.@totalQueuedBytes : pullIntoDescriptor.byteLength - pullIntoDescriptor.bytesFilled;
+    const maxBytesFilled = pullIntoDescriptor.bytesFilled + maxBytesToCopy;
+    const maxAlignedBytes = maxBytesFilled - (maxBytesFilled % pullIntoDescriptor.elementSize);
+    let totalBytesToCopyRemaining = maxBytesToCopy;
+    let ready = false;
+
+    if (maxAlignedBytes > currentAlignedBytes) {
+        totalBytesToCopyRemaining = maxAlignedBytes - pullIntoDescriptor.bytesFilled;
+        ready = true;
     }
+
+    while (totalBytesToCopyRemaining > 0) {
+        let headOfQueue = controller.@queue[0];
+        const bytesToCopy = totalBytesToCopyRemaining < headOfQueue.byteLength ? totalBytesToCopyRemaining : headOfQueue.byteLength;
+        // Copy appropriate part of pullIntoDescriptor.buffer to headOfQueue.buffer.
+        // Remark: this implementation is not completely aligned on the definition of CopyDataBlockBytes
+        // operation of ECMAScript (the case of Shared Data Block is not considered here, but it doesn't seem to be an issue).
+        let fromIndex = pullIntoDescriptor.byteOffset + pullIntoDescriptor.bytesFilled;
+        let count = bytesToCopy;
+        let toIndex = headOfQueue.byteOffset;
+        while (count > 0) {
+            headOfQueue.buffer[toIndex] = pullIntoDescriptor.buffer[fromIndex];
+            toIndex++;
+            fromIndex++;
+            count--;
+        }
+
+        if (headOfQueue.byteLength === bytesToCopy)
+            controller.@queue.@shift();
+        else {
+            headOfQueue.byteOffset += bytesToCopy;
+            headOfQueue.byteLength -= bytesToCopy;
+        }
+
+        controller.@totalQueuedBytes -= bytesToCopy;
+        @assert(controller.@pendingPullIntos.length === 0 || controller.@pendingPullIntos[0] === pullIntoDescriptor);
+        @readableByteStreamControllerInvalidateBYOBRequest(controller);
+        pullIntoDescriptor.bytesFilled += bytesToCopy;
+        totalBytesToCopyRemaining -= bytesToCopy;
+    }
+
+    if (!ready) {
+        @assert(controller.@totalQueuedBytes === 0);
+        @assert(pullIntoDescriptor.bytesFilled > 0);
+        @assert(pullIntoDescriptor.bytesFilled < pullIntoDescriptor.elementSize);
+    }
+
+    return ready;
 }
 
-function readableByteStreamControllerShiftPendingPullInto(controller)
+// Spec name: readableByteStreamControllerShiftPendingPullInto (renamed for consistency).
+function readableByteStreamControllerShiftPendingDescriptor(controller)
 {
     "use strict";
 
@@ -418,7 +533,8 @@ function readableByteStreamControllerInvalidateBYOBRequest(controller)
     controller.@byobRequest = @undefined;
 }
 
-function readableByteStreamControllerCommitPullIntoDescriptor(stream, pullIntoDescriptor)
+// Spec name: readableByteStreamControllerCommitPullIntoDescriptor (shortened for readability).
+function readableByteStreamControllerCommitDescriptor(stream, pullIntoDescriptor)
 {
     "use strict";
 
@@ -428,7 +544,7 @@ function readableByteStreamControllerCommitPullIntoDescriptor(stream, pullIntoDe
         @assert(!pullIntoDescriptor.bytesFilled);
         done = true;
     }
-    let filledView = @readableByteStreamControllerConvertPullIntoDescriptor(pullIntoDescriptor);
+    let filledView = @readableByteStreamControllerConvertDescriptor(pullIntoDescriptor);
     if (pullIntoDescriptor.readerType === "default")
         @readableStreamFulfillReadRequest(stream, filledView, done);
     else {
@@ -437,11 +553,12 @@ function readableByteStreamControllerCommitPullIntoDescriptor(stream, pullIntoDe
     }
 }
 
-function readableByteStreamControllerConvertPullIntoDescriptor(pullIntoDescriptor)
+// Spec name: readableByteStreamControllerConvertPullIntoDescriptor (shortened for readability).
+function readableByteStreamControllerConvertDescriptor(pullIntoDescriptor)
 {
     "use strict";
 
-    @assert(pullIntoDescriptor.bytesFilled <= pullIntoDescriptor.bytesLength);
+    @assert(pullIntoDescriptor.bytesFilled <= pullIntoDescriptor.byteLength);
     @assert(pullIntoDescriptor.bytesFilled % pullIntoDescriptor.elementSize === 0);
 
     return new pullIntoDescriptor.ctor(pullIntoDescriptor.buffer, pullIntoDescriptor.byteOffset, pullIntoDescriptor.bytesFilled / pullIntoDescriptor.elementSize);