1730589 - Initial DOM Streams prototype

Status: RESOLVED FIXED

(Core :: DOM: Streams, task)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

This bug covers landing the initial DOM streams prototype in a disabled state.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1730589 - Initial Implementation of ReadableStreams using WebIDL and DOM technologies r?smaug

This experimental patch re-implements ReadableStreams using WebIDL and DOM technology (vs the existing JS streams implementation). Some more background is here

=== A Brief Tour: ===

1. One task is untangling the special casing in the WebIDL generator for ReadableStreams, created by RedableStreams originally being SpiderMonkey interfaces. In order to support building with both DOM and JS streams, this has meant conditional-parsing inside the WebIDL parser.
2. Another is the import of the streams WebIDL. From this, I used mach webidl-example to generate boilerplate classes, which I then filled in.
3. I disabled the existing Gecko integration with ReadableStreams for the purposes of experimentation by commenting out the support in Fetch and Blob::Stream.
4. The bulk of the patch is line-by-line implementation of spec algorithms.

MG:XXX comments indicate places and things I'm unclear on. Some more listed below.

==== Stylistic and Programatic Issues I'm aware of and would love feedback on ====

1. Because the specification doesn't give a care about internal slot protection, all classes mostly have their members as public, and I directly reference them.
2. Parameter passing for RefPtrT>... I use RefPtr<T>& a lot, but I am not sure that's DOM style, where I think I should be passing T*.
3. ErrorResults tend to be called errorResult; I think the DOM style way to do this would be to call it rv
4. CountQueingStrategy has a Function size member; but I couldn't figure out if it was possible to natively implement a Function, and given that it just is 'a sequence of steps that returns 1.0', I just inlined the implementation; but there's some polymorphism in algorithms from ByteQueingStrategy; I think this is a problem for future patches.

=== Known Weakness, Gaps and Omissions: ===

1. I have done my best to implement cycle collection correctly, but I am definitely with a deficit of experience.

2. ByteStreams and ReadableStream.tee will come in future patches.

3. I intentionally crash in other parts of the DOM that require streams (Fetch, Response, Blob), until the integration work is done in future patches.

   My current plan for that integration doesn't involve re-creating the alternative 'external streams' API from SpiderMonkey's implementationn, but I have yet to do enough development to verify that will work.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b25a4258575d
Initial Implementation of ReadableStreams using WebIDL and DOM technologies r=smaug

Comment 3

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b25a4258575d

Comment 4

[Glenn Watson [:gw]]

I haven't verified that the regression in https://bugzilla.mozilla.org/show_bug.cgi?id=1734523 comes from this bug, but it seems likely since it just started occurring on m-c and this landed in that merge.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Definitely caused by this patch: I see work on that bug to resolve this. Apologies!