Closed Bug 1752206 Opened 4 months ago Closed 4 months ago

Build with DOM ReadableStreams by default.

Categories

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

task

Tracking

()

RESOLVED FIXED
99 Branch
Tracking Status
firefox99 --- fixed

People

(Reporter: mgaudet, Assigned: mgaudet)

References

(Regressed 1 open bug)

Details

Attachments

(3 files)

This is the bug where the actual change in implementation for streams will be landed.

Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6cc91b823c3
Flip the sense of the DOM Streams CI (test JS streams there instead now) r=smaug
https://hg.mozilla.org/integration/autoland/rev/455cd9209987
Enable DOM Streams by default r=smaug,firefox-build-system-reviewers,nalexander

Backed out for causing build bustages on ReadableByteStreamController.cpp

Backout link

Push with failures

Failure log

Failure line(s): /builds/worker/checkouts/gecko/dom/streams/ReadableByteStreamController.cpp:1940:40: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]

Flags: needinfo?(mgaudet)
Depends on: 1754088
Flags: needinfo?(mgaudet)
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e53660b82b9a
Flip the sense of the DOM Streams CI (test JS streams there instead now) r=smaug
https://hg.mozilla.org/integration/autoland/rev/bf7e7a03d4bb
Enable DOM Streams by default r=smaug,firefox-build-system-reviewers,nalexander
Severity: -- → S3
Priority: -- → P1

Backed out for causing failures on testReadableStream.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/9e1168be3b809006570b0f4964482cc27d1d95c0

Push with failures : https://treeherder.mozilla.org/jobs?repo=autoland&duplicate_jobs=visible&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=bf7e7a03d4bb24f0ddc4d436a1874bfa202e943b&selectedTaskRun=T74-Or1LRDiUwHCsRXljFw.0

Link to failure log : https://treeherder.mozilla.org/logviewer?job_id=367088884&repo=autoland

Failure message : TEST-UNEXPECTED-FAIL | testReadableStream_ReadableStreamDefaultReaderRead_CrossCompartment | /builds/worker/checkouts/gecko/js/src/jsapi-tests/testReadableStream.cpp:1169:CHECK failed: stream -- TypeError: ReadableStream is disabled

Flags: needinfo?(mgaudet)
Depends on: 1754251
Depends on: 1754252
Flags: needinfo?(mgaudet)
Flags: needinfo?(mgaudet)
Flags: needinfo?(mgaudet)
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c89f3f10d0e
Flip the sense of the DOM Streams CI (test JS streams there instead now) r=smaug
https://hg.mozilla.org/integration/autoland/rev/bc0d7fc3a472
Enable DOM Streams by default r=smaug,firefox-build-system-reviewers,nalexander
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d706676f161
Add missing RootedDictionary include to make non-unified build work r=evilpie
https://hg.mozilla.org/integration/autoland/rev/02480e689136
Flip the sense of the DOM Streams CI (test JS streams there instead now) r=smaug
https://hg.mozilla.org/integration/autoland/rev/f57513280ac3
Enable DOM Streams by default r=smaug,firefox-build-system-reviewers,nalexander
Flags: needinfo?(mgaudet)
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch
Regressions: 1754977
Regressions: 1755499
Keywords: dev-doc-needed

Hi Matthew, Can you please help me understand the documentation impact of this change, if any?

Looking at ReadableStream we already support that, so my guess is that we've changed the way that it is supported "under the hood". If so, my assumption is probably this is an invisible update for developers. Correct?

PS Further, Remove JS Streams implementation of Writable Stream and PipeTo (bug1755391 ) is the same kind of thing right - replacement of some underlying API with another?

Flags: needinfo?(mgaudet)

Hey Hamish: This change is indeed invisible to developers. Ditto the bug you flagged (we have never shipped WritableStreams on the web)

That should be changing soon though.

Flags: needinfo?(mgaudet)

Thanks Matthew - appreciate the prompt response. I've removed the dev-doc-needed.

Keywords: dev-doc-needed
Regressions: 1761483
Regressions: 1756025
You need to log in before you can comment on or make changes to this bug.