[webkit-reviews] review granted: [Bug 171665] [Readable Streams API] Enable creation of ReadableStreamBYOBReader : [Attachment 309155] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Romain Bellessort
<romain.wkt at gmail.com>'s request for review:
Bug 171665: [Readable Streams API] Enable creation of ReadableStreamBYOBReader
https://bugs.webkit.org/show_bug.cgi?id=171665

Attachment 309155: Patch

https://bugs.webkit.org/attachment.cgi?id=309155&action=review




--- Comment #12 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 309155
  --> https://bugs.webkit.org/attachment.cgi?id=309155
Patch

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

> Source/WebCore/Modules/streams/ReadableStream.js:100
> +    if (mode === 'byob')

Looking at the spec, this looks fine.
Is there a difference between these two lines and "if (mode == 'byob')".

> Source/WebCore/Modules/streams/ReadableStreamBYOBReader.idl:16
> + *	  from this software without specific prior written permission.

This is the old license template. The current one is a two steps list license.
Might want to use https://trac.webkit.org/browser/trunk/Source/WebKit/LICENSE
instead.

> Source/WebCore/bindings/js/JSReadableStreamPrivateConstructors.cpp:73
> +EncodedJSValue JSC_HOST_CALL constructJSReadableStreamBYOBReader(ExecState&
exec)

This function is very similar to constructJSReadableStreamDefaultReader.
Can we share code?

> Source/WebCore/bindings/js/JSReadableStreamPrivateConstructors.cpp:77
> +    JSDOMGlobalObject& globalObject =
*static_cast<JSDOMGlobalObject*>(exec.lexicalGlobalObject());

Might be good to modernize a bit constructJSReadableStreamDefaultReader and
constructJSReadableStreamBYOBReader.
Can you use auto for these lines?
Also, JSC::jsCast is used in the current code base for global object casting.