Closed Bug 1730589 Opened 4 months ago Closed 4 months ago

Initial DOM Streams prototype

Categories

(Core :: DOM: Streams, task)

task

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: mgaudet, Assigned: mgaudet)

References

(Blocks 5 open bugs)

Details

Attachments

(1 file)

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

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.

Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Blocks: 1731070
Blocks: 1734174
Blocks: 1734239
Blocks: 1734240
Blocks: 1734241
Blocks: 1734243
Blocks: 1734244
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b25a4258575d
Initial Implementation of ReadableStreams using WebIDL and DOM technologies r=smaug
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
Regressions: 1734523

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.

Flags: needinfo?(mgaudet)

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

Flags: needinfo?(mgaudet)
You need to log in before you can comment on or make changes to this bug.