Title: [223279] trunk
Revision
223279
Author
romain.belless...@crf.canon.fr
Date
2017-10-13 02:22:48 -0700 (Fri, 13 Oct 2017)

Log Message

[Readable Streams API] Align queue with spec for ReadableStreamDefaultController
https://bugs.webkit.org/show_bug.cgi?id=178082

Reviewed by Xabier Rodriguez-Calvar.

LayoutTests/imported/w3c:

Updated test expectations for tests that were previously failing but now pass.

* web-platform-tests/streams/readable-streams/floating-point-total-queue-size-expected.txt:
* web-platform-tests/streams/readable-streams/floating-point-total-queue-size.dedicatedworker-expected.txt:

Source/WebCore:

Implemented new queue behavior for dequeueValue (used by ReadableStreamDefaultController),
which fixes rounding errors (as described in https://github.com/whatwg/streams/pull/661).
Also aligned ReadableByteStreamController queue so that both queues are implemented in
the same way.

No new tests (covered by existing tests, especially WPT tests that now pass).

* Modules/streams/ReadableByteStreamInternals.js:
(privateInitializeReadableByteStreamController): Aligned queue with RSDC.
(readableByteStreamControllerCancel): Aligned queue with RSDC.
(readableByteStreamControllerError): Aligned queue with RSDC.
(readableByteStreamControllerClose): Aligned queue with RSDC.
(readableByteStreamControllerHandleQueueDrain): Aligned queue with RSDC.
(readableByteStreamControllerPull): Aligned queue with RSDC.
(readableByteStreamControllerEnqueue): Aligned queue with RSDC.
(readableByteStreamControllerEnqueueChunk): Aligned queue with RSDC.
(readableByteStreamControllerProcessPullDescriptors): Aligned queue with RSDC.
(readableByteStreamControllerFillDescriptorFromQueue): Aligned queue with RSDC.
(readableByteStreamControllerPullInto): Aligned queue with RSDC.
* Modules/streams/StreamInternals.js:
(dequeueValue): Updated to match spec.
* bindings/js/WebCoreBuiltinNames.h: Removed now useless "totalQueuedBytes".

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (223278 => 223279)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2017-10-13 09:22:48 UTC (rev 223279)
@@ -1,3 +1,15 @@
+2017-10-13  Romain Bellessort  <romain.belless...@crf.canon.fr>
+
+        [Readable Streams API] Align queue with spec for ReadableStreamDefaultController
+        https://bugs.webkit.org/show_bug.cgi?id=178082
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Updated test expectations for tests that were previously failing but now pass.
+
+        * web-platform-tests/streams/readable-streams/floating-point-total-queue-size-expected.txt:
+        * web-platform-tests/streams/readable-streams/floating-point-total-queue-size.dedicatedworker-expected.txt:
+
 2017-10-12  Chris Dumez  <cdu...@apple.com>
 
         import-w3c-tests modifies test sources and sometimes causes them to fail

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size-expected.txt (223278 => 223279)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size-expected.txt	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size-expected.txt	2017-10-13 09:22:48 UTC (rev 223279)
@@ -1,6 +1,6 @@
 
-FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1
-FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16
+PASS Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) 
+PASS Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) 
 PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) 
 PASS Floating point arithmetic must manifest near 0 (total ends up zero) 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size.dedicatedworker-expected.txt (223278 => 223279)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size.dedicatedworker-expected.txt	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/floating-point-total-queue-size.dedicatedworker-expected.txt	2017-10-13 09:22:48 UTC (rev 223279)
@@ -1,6 +1,6 @@
 
-FAIL Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1
-FAIL Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) assert_equals: [[queueTotalSize]] must clamp to 0 if it becomes negative expected 0 but got 1.1102230246251565e-16
+PASS Floating point arithmetic must manifest near NUMBER.MAX_SAFE_INTEGER (total ends up positive) 
+PASS Floating point arithmetic must manifest near 0 (total ends up positive, but clamped) 
 PASS Floating point arithmetic must manifest near 0 (total ends up positive, and not clamped) 
 PASS Floating point arithmetic must manifest near 0 (total ends up zero) 
 

Modified: trunk/Source/WebCore/ChangeLog (223278 => 223279)


--- trunk/Source/WebCore/ChangeLog	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/Source/WebCore/ChangeLog	2017-10-13 09:22:48 UTC (rev 223279)
@@ -1,3 +1,33 @@
+2017-10-13  Romain Bellessort  <romain.belless...@crf.canon.fr>
+
+        [Readable Streams API] Align queue with spec for ReadableStreamDefaultController
+        https://bugs.webkit.org/show_bug.cgi?id=178082
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Implemented new queue behavior for dequeueValue (used by ReadableStreamDefaultController),
+        which fixes rounding errors (as described in https://github.com/whatwg/streams/pull/661).
+        Also aligned ReadableByteStreamController queue so that both queues are implemented in
+        the same way.
+
+        No new tests (covered by existing tests, especially WPT tests that now pass).
+
+        * Modules/streams/ReadableByteStreamInternals.js:
+        (privateInitializeReadableByteStreamController): Aligned queue with RSDC.
+        (readableByteStreamControllerCancel): Aligned queue with RSDC.
+        (readableByteStreamControllerError): Aligned queue with RSDC.
+        (readableByteStreamControllerClose): Aligned queue with RSDC.
+        (readableByteStreamControllerHandleQueueDrain): Aligned queue with RSDC.
+        (readableByteStreamControllerPull): Aligned queue with RSDC.
+        (readableByteStreamControllerEnqueue): Aligned queue with RSDC.
+        (readableByteStreamControllerEnqueueChunk): Aligned queue with RSDC.
+        (readableByteStreamControllerProcessPullDescriptors): Aligned queue with RSDC.
+        (readableByteStreamControllerFillDescriptorFromQueue): Aligned queue with RSDC.
+        (readableByteStreamControllerPullInto): Aligned queue with RSDC.
+        * Modules/streams/StreamInternals.js:
+        (dequeueValue): Updated to match spec.
+        * bindings/js/WebCoreBuiltinNames.h: Removed now useless "totalQueuedBytes".
+
 2017-10-13  Wenson Hsieh  <wenson_hs...@apple.com>
 
         "text/html" data is not exposed when dragging and dropping across origins

Modified: trunk/Source/WebCore/Modules/streams/ReadableByteStreamInternals.js (223278 => 223279)


--- trunk/Source/WebCore/Modules/streams/ReadableByteStreamInternals.js	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/Source/WebCore/Modules/streams/ReadableByteStreamInternals.js	2017-10-13 09:22:48 UTC (rev 223279)
@@ -59,8 +59,7 @@
     this.@pullAgain = false;
     this.@pulling = false;
     @readableByteStreamControllerClearPendingPullIntos(this);
-    this.@queue = [];
-    this.@totalQueuedBytes = 0;
+    this.@queue = @newQueue();
     this.@started = false;
     this.@closeRequested = false;
 
@@ -137,8 +136,7 @@
 
     if (controller.@pendingPullIntos.length > 0)
         controller.@pendingPullIntos[0].bytesFilled = 0;
-    controller.@queue = [];
-    controller.@totalQueuedBytes = 0;
+    controller.@queue = @newQueue();
     return @promiseInvokeOrNoop(controller.@underlyingByteSource, "cancel", [reason]);
 }
 
@@ -148,7 +146,7 @@
 
     @assert(controller.@controlledReadableStream.@state === @streamReadable);
     @readableByteStreamControllerClearPendingPullIntos(controller);
-    controller.@queue = [];
+    controller.@queue = @newQueue();
     @readableStreamError(controller.@controlledReadableStream, e);
 }
 
@@ -159,7 +157,7 @@
     @assert(!controller.@closeRequested);
     @assert(controller.@controlledReadableStream.@state === @streamReadable);
 
-    if (controller.@totalQueuedBytes > 0) {
+    if (controll...@queue.size > 0) {
         controller.@closeRequested = true;
         return;
     }
@@ -194,7 +192,7 @@
    if (stream.@state === @streamClosed)
        return 0;
 
-   return controller.@strategyHWM - controller.@totalQueuedBytes;
+   return controller.@strategyHWM - controll...@queue.size;
 }
 
 function readableStreamHasBYOBReader(stream)
@@ -216,7 +214,7 @@
     "use strict";
 
     @assert(controller.@controlledReadableStream.@state === @streamReadable);
-    if (!controller.@totalQueuedBytes && controller.@closeRequested)
+    if (!controll...@queue.size && controller.@closeRequested)
         @readableStreamClose(controller.@controlledReadableStream);
     else
         @readableByteStreamControllerCallPullIfNeeded(controller);
@@ -229,10 +227,10 @@
     const stream = controller.@controlledReadableStream;
     @assert(@readableStreamHasDefaultReader(stream));
 
-    if (controller.@totalQueuedBytes > 0) {
+    if (controll...@queue.size > 0) {
         @assert(stream.@reader.@readRequests.length === 0);
-        const entry = controller.@queue.@shift();
-        controller.@totalQueuedBytes -= entry.byteLength;
+        const entry = controller.@queue.content.@shift();
+        controll...@queue.size -= entry.byteLength;
         @readableByteStreamControllerHandleQueueDrain(controller);
         let view;
         try {
@@ -341,7 +339,7 @@
         if (!stream.@reader.@readRequests.length)
             @readableByteStreamControllerEnqueueChunk(controller, transferredBuffer, byteOffset, byteLength);
         else {
-            @assert(!controller.@queue.length);
+            @assert(!controller.@queue.content.length);
             let transferredView = new @Uint8Array(transferredBuffer, byteOffset, byteLength);
             @readableStreamFulfillReadRequest(stream, transferredView, false);
         }
@@ -363,12 +361,12 @@
 {
     "use strict";
 
-    controller.@queue.@push({
+    controller.@queue.content.@push({
         buffer: buffer,
         byteOffset: byteOffset,
         byteLength: byteLength
     });
-    controller.@totalQueuedBytes += byteLength;
+    controll...@queue.size += byteLength;
 }
 
 function readableByteStreamControllerRespondWithNewView(controller, view)
@@ -471,7 +469,7 @@
 
     @assert(!controller.@closeRequested);
     while (controller.@pendingPullIntos.length > 0) {
-        if (controller.@totalQueuedBytes === 0)
+        if (controll...@queue.size === 0)
             return;
         let pullIntoDescriptor = controller.@pendingPullIntos[0];
         if (@readableByteStreamControllerFillDescriptorFromQueue(controller, pullIntoDescriptor)) {
@@ -487,8 +485,8 @@
     "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 maxBytesToCopy = controll...@queue.size < pullIntoDescriptor.byteLength - pullIntoDescriptor.bytesFilled ?
+                controll...@queue.size : pullIntoDescriptor.byteLength - pullIntoDescriptor.bytesFilled;
     const maxBytesFilled = pullIntoDescriptor.bytesFilled + maxBytesToCopy;
     const maxAlignedBytes = maxBytesFilled - (maxBytesFilled % pullIntoDescriptor.elementSize);
     let totalBytesToCopyRemaining = maxBytesToCopy;
@@ -500,7 +498,7 @@
     }
 
     while (totalBytesToCopyRemaining > 0) {
-        let headOfQueue = controller.@queue[0];
+        let headOfQueue = controller.@queue.content[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
@@ -516,13 +514,13 @@
         }
 
         if (headOfQueue.byteLength === bytesToCopy)
-            controller.@queue.@shift();
+            controller.@queue.content.@shift();
         else {
             headOfQueue.byteOffset += bytesToCopy;
             headOfQueue.byteLength -= bytesToCopy;
         }
 
-        controller.@totalQueuedBytes -= bytesToCopy;
+        controll...@queue.size -= bytesToCopy;
         @assert(controller.@pendingPullIntos.length === 0 || controller.@pendingPullIntos[0] === pullIntoDescriptor);
         @readableByteStreamControllerInvalidateBYOBRequest(controller);
         pullIntoDescriptor.bytesFilled += bytesToCopy;
@@ -530,7 +528,7 @@
     }
 
     if (!ready) {
-        @assert(controller.@totalQueuedBytes === 0);
+        @assert(controll...@queue.size === 0);
         @assert(pullIntoDescriptor.bytesFilled > 0);
         @assert(pullIntoDescriptor.bytesFilled < pullIntoDescriptor.elementSize);
     }
@@ -659,7 +657,7 @@
         return @Promise.@resolve({ value: emptyView, done: true });
     }
 
-    if (controller.@totalQueuedBytes > 0) {
+    if (controll...@queue.size > 0) {
         if (@readableByteStreamControllerFillDescriptorFromQueue(controller, pullIntoDescriptor)) {
             const filledView = @readableByteStreamControllerConvertDescriptor(pullIntoDescriptor);
             @readableByteStreamControllerHandleQueueDrain(controller);

Modified: trunk/Source/WebCore/Modules/streams/StreamInternals.js (223278 => 223279)


--- trunk/Source/WebCore/Modules/streams/StreamInternals.js	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/Source/WebCore/Modules/streams/StreamInternals.js	2017-10-13 09:22:48 UTC (rev 223279)
@@ -105,6 +105,9 @@
 
     const record = queue.content.@shift();
     queue.size -= record.size;
+    // As described by spec, below case may occur due to rounding errors.
+    if (queue.size < 0)
+        queue.size = 0;
     return record.value;
 }
 

Modified: trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h (223278 => 223279)


--- trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h	2017-10-13 07:14:29 UTC (rev 223278)
+++ trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h	2017-10-13 09:22:48 UTC (rev 223279)
@@ -266,7 +266,6 @@
     macro(structuredCloneArrayBuffer) \
     macro(structuredCloneArrayBufferView) \
     macro(top) \
-    macro(totalQueuedBytes) \
     macro(underlyingByteSource) \
     macro(underlyingSink) \
     macro(underlyingSource) \
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to