Open Bug 1502355 Opened 2 years ago Updated 5 days ago

Implement ReadableStream.prototype.pipeTo and pipeThrough

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

REOPENED
mozilla74
Tracking Status
firefox73 --- wontfix
firefox74 --- fixed

People

(Reporter: jorendorff, Assigned: Waldo)

References

(Depends on 2 open bugs, Blocks 1 open bug, )

Details

(Keywords: leave-open)

Attachments

(39 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.
Blocks: streams-meta
Priority: -- → P3

Added dependency on Writable Streams. We are hearing from web platforms that piping is a critical missing feature of Readable Streams right now. It should be implemented once Writable Sreams are available.

Depends on: WritableStream
Assignee: nobody → jwalden
Status: NEW → ASSIGNED

The one patch added implements basically the surface of piping, but not any of the internals -- if you invoke them, an error is thrown. A lot more patching is going to have to happen to actually make piping work.

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/d3ab478c8e73
Implement the user-visible portion of RS.prototype.pipeTo, behind a RealmOption, with the underlying piping algorithm *only* just stubbed out.  r=arai
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---

Is this something expected to be actually completed and functional for 73 or is it going to move off to later? Docs planning for 73 going on. Thanks!

There is zero chance of this being finished by Monday for the next merge date. I'm currently bogged down in a separate bug and largely haven't started any meaningful work on this yet.

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/d1408b4cf53a
Implement a very barebones start of a |PipeToState| class to hold the state of an ongoing |pipeTo| operation.  r=arai
Target Milestone: mozilla73 → mozilla74

Arno Renevier has posted elaborations on the hyper-partial stabbing I have done so far, FWIW. Still looking into what of that I can integrate into my own work here.

https://phabricator.services.mozilla.com/D59258

Since this is just early implementation details, and nowhere near a shipping feature, I'll remove dev-doc-needed from here. We'll track it again in future when it is a bit nearer to completion.

Keywords: dev-doc-needed
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/8a104e44cbe6
Make |ReadableStreamPipeTo| return |PromiseObject*|.  r=arai
https://hg.mozilla.org/integration/autoland/rev/7b7fb2498eb6
Fill in more of |ReadableStreamPipeTo|, as regards consing up a |PipeToState| closure to store state variables.  r=arai
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/99f6870e3611
Make the |ForAuthorCodeBool| argument to |CreateReadableStreamDefaultReader| not defaulted, for explicitness at calling sites.  r=arai
https://hg.mozilla.org/integration/autoland/rev/ac82299cbd75
Support wrapping a |JS::MutableHandle<Maybe<JS::Value>>|.  r=arai
https://hg.mozilla.org/integration/autoland/rev/9fd9dc3b2edb
Add skeletal code checking for source/dest closure at start of a pipe-to operation.  r=arai
https://hg.mozilla.org/integration/autoland/rev/fed0a5f809d2
Add reactions handling source/dest becoming closed or errored.  r=arai
https://hg.mozilla.org/integration/autoland/rev/ae4985a75c89
Begin defining the shutdown and shutdown-with-action algorithms, starting with the "only perform shutdown once" step.  r=arai
https://hg.mozilla.org/integration/autoland/rev/93d8076776df
Implement writable-and-not-closing checks in shutdown steps.  r=arai

FYI, the type-narrowing patches just posted are mostly intended to permit making the types of values in some of the piping algorithms 1) clearer that an actual promise is returned, and 2) clearer that a promise from the current realm/compartment is being returned. (I have the start of a first patch that actually puts this to some use, but it's just slightly too sketchy to be reviewable yet.)

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/6590b48d7450
Don't use |PromiseObject::unforgeableResolve| where the passed-in value is known to not be a promise, and add/use |PromiseResolvedWithUndefined| for cases where the value is |undefined|.  r=arai
https://hg.mozilla.org/integration/autoland/rev/b993ebbff7e4
Rename |CanPipeStreams| to |SourceOrDestErroredOrClosed| for more clarity.  r=arai
https://hg.mozilla.org/integration/autoland/rev/03bf4a6f38a7
Make |ReadableStreamDefaultControllerPullSteps| return a narrower |PromiseObject*| type.  r=arai
https://hg.mozilla.org/integration/autoland/rev/10f67be0415c
Make |PromiseRejectedWithPendingError| return a narrower |PromiseObject*| type.  r=arai
https://hg.mozilla.org/integration/autoland/rev/14f74f970269
Make |ReadableByteStreamControllerPullSteps| return a narrower |PromiseObject*| type.  r=arai
https://hg.mozilla.org/integration/autoland/rev/fcf4151b9e07
Make |ReadableStreamControllerPullSteps| return a narrower |PromiseObject*| type.  r=arai
https://hg.mozilla.org/integration/autoland/rev/d1287e914d42
Make |ReadableStreamDefaultReaderRead| return a narrower |PromiseObject*| type.  r=arai
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/2084b0479a61
Make |WritableStreamDefaultWriterWrite| return a narrower |PromiseObject*| type for more clarity.  r=arai
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/a5997699b481
Read from the source stream once permitted to do so.  r=arai
https://hg.mozilla.org/integration/autoland/rev/b71b149b451c
React to a stream reader read result.  r=arai
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/42fb5205d051
Allow handler functions to store an extra *value*, not merely an extra T* that's an object.  r=arai
https://hg.mozilla.org/integration/autoland/rev/146e9fa5e758
Implement |WritableStreamDefaultWriterCloseWithErrorPropagation|.  r=arai
https://hg.mozilla.org/integration/autoland/rev/d4b8ef062b42
React to a read promise that rejects.  r=arai
https://hg.mozilla.org/integration/autoland/rev/a7d2cf07580f
Store the value optionally passed to finalizing as an extra value in a handler function.  r=arai
https://hg.mozilla.org/integration/autoland/rev/c2066ce91465
Save to the side the action specified when we're supposed to "shutdown with an action".  r=arai
https://hg.mozilla.org/integration/autoland/rev/cb9ba1647a9a
Wait for completed reads to finish writing before finalizing, during pipe-to shutdown with an action.  r=arai
https://hg.mozilla.org/integration/autoland/rev/22e38ee59de3
Implement some of the process of performing an action and then finalizing, in the shutdown with an action process.  r=arai
https://hg.mozilla.org/integration/autoland/rev/148d65d325da
Fill in most of the body of the "finalize" operation in the piping algorithm.  r=arai
Attachment #9170545 - Attachment description: Bug 1502355 - Declare |JSClass| in consistent fashion everywhere. r=jonco! → Bug 1502355 - Declare |JSClass| in consistent fashion everywhere. r=arai!
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/207f8942ffd2
Declare |JSClass| in consistent fashion everywhere.  r=arai
https://hg.mozilla.org/integration/autoland/rev/eeff6597f16f
Remove various unused detritus from jsfriendapi.h.  r=arai
https://hg.mozilla.org/integration/autoland/rev/deb00e023f3b
Add support for unwrapping a value/object, known at one time to have a given class, as an object of that class.  r=arai
https://hg.mozilla.org/integration/autoland/rev/254c794085e1
Correctly detect AbortSignal instances using a |const JSClass*| supplied by the embedding.  r=arai
https://hg.mozilla.org/integration/autoland/rev/77027a06c438
Recognize already-aborted signals passed to |ReadableStreamPipeTo| and don't pipe in this case.  r=arai
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/78982a089258
Fix Rust build to not depend on previously-bootlegged symbols.
Depends on: 1660555

(In reply to Mike Conca [:mconca] from comment #1)

Added dependency on Writable Streams. We are hearing from web platforms that piping is a critical missing feature of Readable Streams right now. It should be implemented once Writable Sreams are available.

I agree on this idea, though both of them should be implemented in parallel if possible.

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/195dfba58e22
Allocate |PipeToState| as tenured, for perhaps marginally greater efficiency.  r=arai
You need to log in before you can comment on or make changes to this bug.