Closed Bug 1749845 Opened 2 years ago Closed 2 years ago

Consider letting DOM ReadableStream be enabled by a runtime flag

Categories

(Core :: DOM: Streams, task, P3)

Product:
Core
Component:
DOM: Streams
Type:
task

Tracking

()

Status:
RESOLVED DUPLICATE of bug 1734722

People

(Reporter: saschanaz, Unassigned)

References

(Blocks 1 open bug)

Details

Currently it's it's harder to make sure everything works as expected because it's behind a build flag and thus is not tested by CI. That makes flipping it be a dangerous and potentially destructive work, with no way for users to easily revert to the previous behavior.

I suggest letting it be enabled in runtime to make things both dev- and user-friendly, although we know there should probably be nontrivial work for the bindings part.

That said, testing will still be hard unless there is a way to configure relevant WPT tests twice with the flag on and off.

Looking at Codegen.py, I think we'll have to add a lot of conditions wherever isSpiderMonkeyInterface() check is used.. 🥲

Matthew, could you give some thoughts here? I think this largely depends on how long we expect to have both implementations maintained.

Flags: needinfo?(mgaudet)

My hope is that once we ship DOM streams, it is shipped and we are done. So I don't expect to have to dual maintain both sides for very long at all (essentially, purely while it rides the trains to release, in the event that we find a show-stopping bug in DOM streams).

While the risk-averse part of me would love this to be pref-able, and therefore something we could control/update via remote-settings, I think instead our focus ought to be on aiming for a high-quality flag day.

The whole point of the nightly / beta cycle is to allow us to ship risky things incrementally, so that's what we have to put our faith in i think.

Flags: needinfo?(mgaudet)

I hope after bug 1734873 is finished we can quickly enable ReadableStream and ship it directly. If that isn't possible we could still enable the build flag by default on Nightly and disable it again before the soft freeze. I don't see us having to make any changes to the SpiderMonkey streams implementation though, as that code basically hasn't changed in two years.

Okay, if the plan is to enable it very quick then I guess we can live without the runtime flag, especially when only ReadableStream is affected.

But let me keep this open for now until we actually ship it...

(But since that single ReadableStream affects various components, I still feel it dangerous 🤔, let's hope no catastrophe be happen...)

I think we should let fuzzer to make sure it's not faulty in any security vulnerable way for a while, btw.

Severity: -- → S3
Priority: -- → P3

Another option would be to have a dedicated CI job where --enable-dom-stream is enabled. That'll require a dedicated build job too, but if we think the transition period be short enough, maybe a good option to consider? What do you think?

I think doing so will prevent some sudden regression like bug 1750606.

Flags: needinfo?(mgaudet)

In principle I think this would be a good thing.

My plan is to aim to do the transition early in the Firefox 99 cycle; so we're talking about ~3 weeks before we make that switch.

We could still do that, and then make this build work the opposite way when we ship (ie, switch from building DOM streams to building JS streams to ensure those builds continue running).

Is this something you'd be able to do?

Flags: needinfo?(mgaudet)

I should talk to CI people about this, I'll start some chat on Matrix to make sure this is something we can actually do.

I guess getting a runtime flag would be way too hard, and I found an existing bug for CI thing, so closing this for that one.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.