readableStreamDefaultControllerError should return early if stream is not readable
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2018 05:35:25 +0000 (05:35 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2018 05:35:25 +0000 (05:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185602

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/streams/readable-streams/bad-underlying-sources-expected.txt:
* web-platform-tests/streams/readable-streams/bad-underlying-sources.dedicatedworker-expected.txt:
* web-platform-tests/streams/readable-streams/bad-underlying-sources.serviceworker.https-expected.txt:
* web-platform-tests/streams/readable-streams/garbage-collection-expected.txt:
* web-platform-tests/streams/readable-streams/garbage-collection.dedicatedworker-expected.txt:
* web-platform-tests/streams/readable-streams/garbage-collection.serviceworker.https-expected.txt:
* web-platform-tests/streams/readable-streams/tee-expected.txt:

Source/WebCore:

Return early if stream is not readable in @readableStreamDefaultControllerError.
Update call sites to no longer check for ReadableStream state.
Covered by unflaked and rebased tests.

* Modules/streams/ReadableStreamDefaultController.js:
(error):
* Modules/streams/ReadableStreamInternals.js:
(readableStreamDefaultControllerError):
(readableStreamDefaultControllerCallPullIfNeeded):

LayoutTests:

* TestExpectations:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying-sources-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying-sources.dedicatedworker-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying-sources.serviceworker.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/garbage-collection-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/garbage-collection.dedicatedworker-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/garbage-collection.serviceworker.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/tee-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/streams/ReadableStreamDefaultController.js
Source/WebCore/Modules/streams/ReadableStreamInternals.js

index 9771127..adc2df5 100644 (file)
@@ -1,5 +1,14 @@
 2018-05-14  Youenn Fablet  <youenn@apple.com>
 
+        readableStreamDefaultControllerError should return early if stream is not readable
+        https://bugs.webkit.org/show_bug.cgi?id=185602
+
+        Reviewed by Chris Dumez.
+
+        * TestExpectations:
+
+2018-05-14  Youenn Fablet  <youenn@apple.com>
+
         imported/w3c/web-platform-tests/fetch/api/response/response-stream-disturbed-6.html is flaky
         https://bugs.webkit.org/show_bug.cgi?id=185549
 
index e5bb594..01f075c 100644 (file)
@@ -615,10 +615,8 @@ http/tests/loading/text-content-type-with-binary-extension.html [ Pass Failure ]
 webkit.org/b/171094 streams/brand-checks.html [ Pass Failure ]
 webkit.org/b/171094 streams/reference-implementation/abstract-ops.html [ Pass Failure ]
 
-# tee.html is flaky with release and crashes with debug builds
-webkit.org/b/171094 imported/w3c/web-platform-tests/streams/readable-streams/tee.html [ Pass Failure Crash ]
-[ Debug ] imported/w3c/web-platform-tests/streams/readable-streams/tee.serviceworker.https.html [ Failure ]
-[ Debug ] imported/w3c/web-platform-tests/streams/readable-streams/tee.dedicatedworker.html [ Crash ]
+# tee.html is flaky with unhandled promise rejections
+imported/w3c/web-platform-tests/streams/readable-streams/tee.html [ DumpJSConsoleLogInStdErr ]
 
 # WPT tests that fail after doing full test repository reimport and need further investigation
 imported/w3c/web-platform-tests/dom/nodes/Document-createElement-namespace.html [ Pass Failure ]
index ef33796..b73e446 100644 (file)
@@ -1,5 +1,20 @@
 2018-05-14  Youenn Fablet  <youenn@apple.com>
 
+        readableStreamDefaultControllerError should return early if stream is not readable
+        https://bugs.webkit.org/show_bug.cgi?id=185602
+
+        Reviewed by Chris Dumez.
+
+        * web-platform-tests/streams/readable-streams/bad-underlying-sources-expected.txt:
+        * web-platform-tests/streams/readable-streams/bad-underlying-sources.dedicatedworker-expected.txt:
+        * web-platform-tests/streams/readable-streams/bad-underlying-sources.serviceworker.https-expected.txt:
+        * web-platform-tests/streams/readable-streams/garbage-collection-expected.txt:
+        * web-platform-tests/streams/readable-streams/garbage-collection.dedicatedworker-expected.txt:
+        * web-platform-tests/streams/readable-streams/garbage-collection.serviceworker.https-expected.txt:
+        * web-platform-tests/streams/readable-streams/tee-expected.txt:
+
+2018-05-14  Youenn Fablet  <youenn@apple.com>
+
         imported/w3c/web-platform-tests/fetch/api/response/response-stream-disturbed-6.html is flaky
         https://bugs.webkit.org/show_bug.cgi?id=185549
 
index dc2e183..51e6be5 100644 (file)
@@ -24,8 +24,8 @@ PASS Underlying source: calling close twice on a non-empty stream should throw t
 PASS Underlying source: calling close on an empty canceled stream should throw 
 PASS Underlying source: calling close on a non-empty canceled stream should throw 
 PASS Underlying source: calling close after error should throw 
-FAIL Underlying source: calling error twice should not throw ReadableStream is not readable
-FAIL Underlying source: calling error after close should not throw ReadableStream is not readable
+PASS Underlying source: calling error twice should not throw 
+PASS Underlying source: calling error after close should not throw 
 PASS Underlying source: calling error and returning a rejected promise from start should cause the stream to error with the first error 
 PASS Underlying source: calling error and returning a rejected promise from pull should cause the stream to error with the first error 
 PASS read should not error if it dequeues and pull() throws 
index dc2e183..51e6be5 100644 (file)
@@ -24,8 +24,8 @@ PASS Underlying source: calling close twice on a non-empty stream should throw t
 PASS Underlying source: calling close on an empty canceled stream should throw 
 PASS Underlying source: calling close on a non-empty canceled stream should throw 
 PASS Underlying source: calling close after error should throw 
-FAIL Underlying source: calling error twice should not throw ReadableStream is not readable
-FAIL Underlying source: calling error after close should not throw ReadableStream is not readable
+PASS Underlying source: calling error twice should not throw 
+PASS Underlying source: calling error after close should not throw 
 PASS Underlying source: calling error and returning a rejected promise from start should cause the stream to error with the first error 
 PASS Underlying source: calling error and returning a rejected promise from pull should cause the stream to error with the first error 
 PASS read should not error if it dequeues and pull() throws 
index b3dae7d..dff3da3 100644 (file)
@@ -25,8 +25,8 @@ PASS Underlying source: calling close twice on a non-empty stream should throw t
 PASS Underlying source: calling close on an empty canceled stream should throw 
 PASS Underlying source: calling close on a non-empty canceled stream should throw 
 PASS Underlying source: calling close after error should throw 
-FAIL Underlying source: calling error twice should not throw ReadableStream is not readable
-FAIL Underlying source: calling error after close should not throw ReadableStream is not readable
+PASS Underlying source: calling error twice should not throw 
+PASS Underlying source: calling error after close should not throw 
 PASS Underlying source: calling error and returning a rejected promise from start should cause the stream to error with the first error 
 PASS Underlying source: calling error and returning a rejected promise from pull should cause the stream to error with the first error 
 PASS read should not error if it dequeues and pull() throws 
index c72ce18..010f50a 100644 (file)
@@ -1,5 +1,5 @@
 
-FAIL ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream promise_test: Unhandled rejection with value: object "TypeError: ReadableStream is not readable"
+PASS ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream 
 PASS ReadableStream closed promise should fulfill even if the stream and reader JS references are lost 
 PASS ReadableStream closed promise should reject even if stream and reader JS references are lost 
 PASS Garbage-collecting a ReadableStreamDefaultReader should not unlock its stream 
index c72ce18..010f50a 100644 (file)
@@ -1,5 +1,5 @@
 
-FAIL ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream promise_test: Unhandled rejection with value: object "TypeError: ReadableStream is not readable"
+PASS ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream 
 PASS ReadableStream closed promise should fulfill even if the stream and reader JS references are lost 
 PASS ReadableStream closed promise should reject even if stream and reader JS references are lost 
 PASS Garbage-collecting a ReadableStreamDefaultReader should not unlock its stream 
index c171d12..effb9b5 100644 (file)
@@ -1,6 +1,6 @@
 
 PASS Service worker test setup 
-FAIL ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream promise_test: Unhandled rejection with value: object "TypeError: ReadableStream is not readable"
+PASS ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream 
 PASS ReadableStream closed promise should fulfill even if the stream and reader JS references are lost 
 PASS ReadableStream closed promise should reject even if stream and reader JS references are lost 
 PASS Garbage-collecting a ReadableStreamDefaultReader should not unlock its stream 
index 6de2142..e03a5c2 100644 (file)
@@ -1,7 +1,3 @@
-CONSOLE MESSAGE: Unhandled Promise Rejection: [object Object]
-CONSOLE MESSAGE: Unhandled Promise Rejection: [object Object]
-CONSOLE MESSAGE: Unhandled Promise Rejection: [object Object]
-CONSOLE MESSAGE: Unhandled Promise Rejection: [object Object]
 
 PASS ReadableStream teeing: rs.tee() returns an array of two ReadableStreams 
 PASS ReadableStream teeing: should be able to read one branch to the end without affecting the other 
@@ -11,6 +7,8 @@ PASS ReadableStream teeing: canceling branch1 should not impact branch2
 PASS ReadableStream teeing: canceling branch2 should not impact branch2 
 PASS ReadableStream teeing: canceling both branches should aggregate the cancel reasons into an array 
 PASS ReadableStream teeing: failing to cancel the original stream should cause cancel() to reject on branches 
+PASS ReadableStream teeing: erroring a teed stream should properly handle canceled branches 
 PASS ReadableStream teeing: closing the original should immediately close the branches 
 PASS ReadableStream teeing: erroring the original should immediately error the branches 
+PASS ReadableStreamTee should not use a modified ReadableStream constructor from the global object 
 
index af95b29..0ce1f28 100644 (file)
@@ -1,3 +1,20 @@
+2018-05-14  Youenn Fablet  <youenn@apple.com>
+
+        readableStreamDefaultControllerError should return early if stream is not readable
+        https://bugs.webkit.org/show_bug.cgi?id=185602
+
+        Reviewed by Chris Dumez.
+
+        Return early if stream is not readable in @readableStreamDefaultControllerError.
+        Update call sites to no longer check for ReadableStream state.
+        Covered by unflaked and rebased tests.
+
+        * Modules/streams/ReadableStreamDefaultController.js:
+        (error):
+        * Modules/streams/ReadableStreamInternals.js:
+        (readableStreamDefaultControllerError):
+        (readableStreamDefaultControllerCallPullIfNeeded):
+
 2018-05-14  Zalan Bujtas  <zalan@apple.com>
 
         [LFC] Implement width computation for non-replaced block level inflow elements.
index cf3deff..a373fad 100644 (file)
@@ -45,9 +45,6 @@ function error(error)
     if (!@isReadableStreamDefaultController(this))
         throw @makeThisTypeError("ReadableStreamDefaultController", "error");
 
-    if (@getByIdDirectPrivate(@getByIdDirectPrivate(this, "controlledReadableStream"), "state") !== @streamReadable)
-        @throwTypeError("ReadableStream is not readable");
-
     @readableStreamDefaultControllerError(this, error);
 }
 
index 3429a61..95aa75b 100644 (file)
@@ -85,8 +85,7 @@ function privateInitializeReadableStreamDefaultController(stream, underlyingSour
         @assert(!@getByIdDirectPrivate(controller, "pullAgain"));
         @readableStreamDefaultControllerCallPullIfNeeded(controller);
     }, (error) => {
-        if (@getByIdDirectPrivate(stream, "state") === @streamReadable)
-            @readableStreamDefaultControllerError(controller, error);
+        @readableStreamDefaultControllerError(controller, error);
     });
 
     @putByIdDirectPrivate(this, "cancel", @readableStreamDefaultControllerCancel);
@@ -101,7 +100,8 @@ function readableStreamDefaultControllerError(controller, error)
     "use strict";
 
     const stream = @getByIdDirectPrivate(controller, "controlledReadableStream");
-    @assert(@getByIdDirectPrivate(stream, "state") === @streamReadable);
+    if (@getByIdDirectPrivate(stream, "state") !== @streamReadable)
+        return;
     @putByIdDirectPrivate(controller, "queue", @newQueue());
     @readableStreamError(stream, error);
 }
@@ -341,8 +341,7 @@ function readableStreamDefaultControllerCallPullIfNeeded(controller)
             @readableStreamDefaultControllerCallPullIfNeeded(controller);
         }
     }, function(error) {
-        if (@getByIdDirectPrivate(stream, "state") === @streamReadable)
-            @readableStreamDefaultControllerError(controller, error);
+        @readableStreamDefaultControllerError(controller, error);
     });
 }
 
@@ -477,8 +476,7 @@ function readableStreamDefaultControllerEnqueue(controller, chunk)
         @enqueueValueWithSize(@getByIdDirectPrivate(controller, "queue"), chunk, chunkSize);
     }
     catch(error) {
-        if (@getByIdDirectPrivate(stream, "state") === @streamReadable)
-            @readableStreamDefaultControllerError(controller, error);
+        @readableStreamDefaultControllerError(controller, error);
         throw error;
     }
     @readableStreamDefaultControllerCallPullIfNeeded(controller);