Bug 172111 - [Readable Streams API] Implement ReadableStreamBYOBReader releaseLock()
Summary: [Readable Streams API] Implement ReadableStreamBYOBReader releaseLock()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Romain Bellessort
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-15 03:11 PDT by Romain Bellessort
Modified: 2017-05-16 05:47 PDT (History)
7 users (show)

See Also:


Attachments
Patch (13.36 KB, patch)
2017-05-15 03:33 PDT, Romain Bellessort
no flags Details | Formatted Diff | Diff
Patch (13.56 KB, patch)
2017-05-16 01:44 PDT, Romain Bellessort
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Romain Bellessort 2017-05-15 03:11:00 PDT
Implement the releaseLock() method of ReadableStreamBYOBReader
Comment 1 Romain Bellessort 2017-05-15 03:33:38 PDT
Created attachment 310127 [details]
Patch
Comment 2 Chris Dumez 2017-05-15 09:43:57 PDT
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?
Comment 3 Romain Bellessort 2017-05-16 01:44:44 PDT
Created attachment 310244 [details]
Patch
Comment 4 Romain Bellessort 2017-05-16 03:46:23 PDT
(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 5 WebKit Commit Bot 2017-05-16 05:47:25 PDT
Comment on attachment 310244 [details]
Patch

Clearing flags on attachment: 310244

Committed r216926: <http://trac.webkit.org/changeset/216926>
Comment 6 WebKit Commit Bot 2017-05-16 05:47:27 PDT
All reviewed patches have been landed.  Closing bug.