Summary: | [Readable Streams API] Implement ReadableByteStreamController cancel internal method | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Romain Bellessort <romain.wkt> | ||||||||||
Component: | WebCore Misc. | Assignee: | Romain Bellessort <romain.wkt> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | benjamin, calvaris, commit-queue, youennf | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Romain Bellessort
2016-12-13 08:36:26 PST
Created attachment 297015 [details]
Patch
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? Created attachment 297086 [details]
Patch
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. Created attachment 297191 [details]
Patch
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. Created attachment 297195 [details]
Patch
(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. Comment on attachment 297195 [details] Patch Clearing flags on attachment: 297195 Committed r209915: <http://trac.webkit.org/changeset/209915> All reviewed patches have been landed. Closing bug. |