RESOLVED FIXED165798
[Readable Streams API] Implement ReadableByteStreamController cancel internal method
https://bugs.webkit.org/show_bug.cgi?id=165798
Summary [Readable Streams API] Implement ReadableByteStreamController cancel internal...
Romain Bellessort
Reported 2016-12-13 08:36:26 PST
Implement ReadableByteStreamController cancel internal method
Attachments
Patch (7.48 KB, patch)
2016-12-13 08:57 PST, Romain Bellessort
no flags
Patch (10.15 KB, patch)
2016-12-14 06:55 PST, Romain Bellessort
no flags
Patch (9.83 KB, patch)
2016-12-15 05:53 PST, Romain Bellessort
no flags
Patch (9.88 KB, patch)
2016-12-15 09:10 PST, Romain Bellessort
no flags
Romain Bellessort
Comment 1 2016-12-13 08:57:44 PST
youenn fablet
Comment 2 2016-12-14 03:44:06 PST
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
Comment 3 2016-12-14 06:55:57 PST
youenn fablet
Comment 4 2016-12-14 12:27:28 PST
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
Comment 5 2016-12-15 05:53:06 PST
youenn fablet
Comment 6 2016-12-15 07:16:00 PST
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
Comment 7 2016-12-15 09:10:02 PST
Romain Bellessort
Comment 8 2016-12-15 10:05:40 PST
(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 9 2016-12-16 02:02:26 PST
Comment on attachment 297195 [details] Patch Clearing flags on attachment: 297195 Committed r209915: <http://trac.webkit.org/changeset/209915>
WebKit Commit Bot
Comment 10 2016-12-16 02:02:31 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.