Bug 151497 – [Streams API] pull function of tee should call readFromReadableStreamReader directly

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

pull function of tee should call readFromReadableStreamReader directly.
Currently it calls ReadableStream.read() which is suboptimal.

[youenn fablet]

Created attachment 265960 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 265960 [details]
Patch

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

> Source/WebCore/Modules/streams/ReadableStreamInternals.js:-119
> -        @Promise.prototype.@then.@call(reader.read(), function(result) {

I was kind of puzzled when I read this the first time so I guess it makes quite a lot of sense to change it.

[youenn fablet]

Comment on attachment 265960 [details]
Patch

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

>> Source/WebCore/Modules/streams/ReadableStreamInternals.js:-119
>> -        @Promise.prototype.@then.@call(reader.read(), function(result) {
> 
> I was kind of puzzled when I read this the first time so I guess it makes quite a lot of sense to change it.

In that specific case, it is safe to write (and much easier to read): @readFromReadableStreamReader(reader).@then(function ...);
It would be good to write it that way for most (if not all) parts of streams API code to keep consistency and improve readability.

That may mean some refactoring in invokeOrNoop and the likes to ensure @then is available.
All these changes might best be adressed as a follow-up patch.

[youenn fablet]

Created attachment 266348 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 266348 [details]
Patch for landing

Rejecting attachment 266348 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 266348, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
with fuzz 1.
patching file LayoutTests/streams/streams-promises-expected.txt
Hunk #1 FAILED at 8.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/streams/streams-promises-expected.txt.rej
patching file LayoutTests/streams/streams-promises.html
Hunk #1 FAILED at 125.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/streams/streams-promises.html.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/500863

[youenn fablet]

(In reply to comment #4)
> Created attachment 266348 [details]
> Patch for landing

Thanks for the r+
Along the lines of other related patches, I added a test to ensure we no longer use ReadableStreamReader.read().
It uses promise_test as a way to execute sequentially an otherwise async_test.

[WebKit Commit Bot]

Comment on attachment 266348 [details]
Patch for landing

Rejecting attachment 266348 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 266348, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/500949

[youenn fablet]

Created attachment 266350 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 266350 [details]
Patch for landing

Clearing flags on attachment: 266350

Committed r192879: <http://trac.webkit.org/changeset/192879>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.