[Readable Streams API] Implement ReadableByteStreamController cancel internal method

[Romain Bellessort]

Implement ReadableByteStreamController cancel internal method

[Romain Bellessort]

Created attachment 297015 [details]
Patch

[youenn fablet]

Comment on attachment 297015 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297015&action=review

> Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:95
> +        firstDescriptor.bytesFilled = 0;

Could be done as one liner.

> LayoutTests/streams/readable-byte-stream-controller.js:233
> +const test5 = async_test("Calling cancel() on a readable ReadableStream that is not locked to a reader should return a promise whose fulfillment handler returns undefined");

Can you use promise_test instead of async_test?

[Romain Bellessort]

Created attachment 297086 [details]
Patch

[youenn fablet]

Comment on attachment 297086 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297086&action=review

> LayoutTests/streams/readable-byte-stream-controller.js:237
> +            assert_unreached("No error should be thrown when calling cancel() on a readable ReadableStream that is not locked to a reader");

You do not need that assert_unreached. If you return a rejected promise, the test will be marked as fail.

[Romain Bellessort]

Created attachment 297191 [details]
Patch

[youenn fablet]

Comment on attachment 297191 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297191&action=review

> LayoutTests/streams/readable-byte-stream-controller.js:164
> +        function(err) {

Sorry for not spotting this one early.
You might be able to use promise_rejects for such cases, something like:
    return promise_rejects(test, myError, rs.getReader().read());

You might want to fix it later on if you wish.

[Romain Bellessort]

Created attachment 297195 [details]
Patch

[Romain Bellessort]

(In reply to comment #6)
> Comment on attachment 297191 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=297191&action=review
> 
> > LayoutTests/streams/readable-byte-stream-controller.js:164
> > +        function(err) {
> 
> Sorry for not spotting this one early.
> You might be able to use promise_rejects for such cases, something like:
>     return promise_rejects(test, myError, rs.getReader().read());
> 
> You might want to fix it later on if you wish.

Thanks for the advice, the new patch uses promise_rejects.

[WebKit Commit Bot]

Comment on attachment 297195 [details]
Patch

Clearing flags on attachment: 297195

Committed r209915: <http://trac.webkit.org/changeset/209915>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.