Summary: | [Readable Streams API] Implement ReadableStreamBYOBReader releaseLock() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Romain Bellessort <romain.wkt> | ||||||
Component: | WebCore Misc. | Assignee: | Romain Bellessort <romain.wkt> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, buildbot, calvaris, cdumez, commit-queue, nael.ouedp, youennf | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Romain Bellessort
2017-05-15 03:11:00 PDT
Created attachment 310127 [details]
Patch
Comment on attachment 310127 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310127&action=review r=me with a couple of minor comments. > Source/WebCore/ChangeLog:8 > + Added tests to check releaseLock behaviour. I believe we use US english, so "behavior". > Source/WebCore/ChangeLog:9 > + Please always provide a link to the part of the spec you are implementing in your patch to facilitate review. Here, I would expect it is: https://streams.spec.whatwg.org/#byob-reader-release-lock and https://streams.spec.whatwg.org/#readable-stream-reader-generic-release > Source/WebCore/Modules/streams/ReadableStreamInternals.js:508 > reader.@ownerReadableStream = null; Spec says to set this to undefined, no? Created attachment 310244 [details]
Patch
(In reply to Chris Dumez from comment #2) > Comment on attachment 310127 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310127&action=review > > r=me with a couple of minor comments. > > > Source/WebCore/ChangeLog:8 > > + Added tests to check releaseLock behaviour. > > I believe we use US english, so "behavior". > > > Source/WebCore/ChangeLog:9 > > + > > Please always provide a link to the part of the spec you are implementing in > your patch to facilitate review. Here, I would expect it is: > https://streams.spec.whatwg.org/#byob-reader-release-lock > and > https://streams.spec.whatwg.org/#readable-stream-reader-generic-release > Thanks, I'll take care to do so (and I've updated the ChangeLog in the new version of the patch). > > Source/WebCore/Modules/streams/ReadableStreamInternals.js:508 > > reader.@ownerReadableStream = null; > > Spec says to set this to undefined, no? Right, I've fixed that in the new version of the patch (I hadn't fixed it previously as I thought this property used "null" to enable a specific behavior, but in fact I was confusing with another property). Comment on attachment 310244 [details] Patch Clearing flags on attachment: 310244 Committed r216926: <http://trac.webkit.org/changeset/216926> All reviewed patches have been landed. Closing bug. |