Closed Bug 1504464 Opened 10 months ago Closed 7 months ago

Revise ReadableStream implementation to match current standard

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

Attachments

(11 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
There have been many changes since this was implemented. I think they're all fairly minor, but I'd have to go through the standard with a fine-toothed comb to be sure. Probably easier to just do it!
Priority: -- → P1
Depends on: 1508438
Depends on: 1514051
I don't know why it's OK to drop this particular error; my guess is that the
error was already reported previously, when the stream became errored, and
there's no point reporting it again.
In this case, it's likely the user doesn't see this as an error at all.

Depends on D14496
The section headers in the spec that look like JS destructuring are in fact
normative. The methods have to behave just like JS destructuring; see
<https://streams.spec.whatwg.org/#conventions> for details.

This means the getReader method
<https://streams.spec.whatwg.org/#rs-get-reader> must do a full property Get
for options.mode, even if that means querying %ObjectPrototype%, absurd as it
sounds.

Depends on D14499
No change in behavior that I'm aware of.  It should be correct either way,
since the object is guaranteed to be an object created just so by code
elsewhere in the Streams implementation. But the intended purpose of
GetPropertyPure is in a fast path, backstopped by an actual GetProperty, not
for cases like this.

Depends on D14504
Assignee: nobody → jorendorff
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/72b109d30535
Part 1: Mark reader.[[closedPromise]] as handled when creating a reader for an already-errored stream. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/d087b9c8c389
Part 2: Mark reader.[[closedPromise]] as handled in reader.releaseLock(). r=jwalden
https://hg.mozilla.org/integration/autoland/rev/0030c59cdea4
Part 3: Update CreateExternalReadableByteStreamController to the current standard. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/d35260c2032c
Part 4: Comment-only changes. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/de74494a1aa7
Part 5: Fix destructuring behavior in ReadableStream.prototype.getReader. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/7afcd486bdc6
Part 6: Rearrange control flow in ReadableStream_getReader slightly to resemble the standard steps. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/8a450893e90a
Part 7: Stop using GetPropertyPure in Streams.cpp. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/3ff6421bac54
Part 8: Rename the handler called when a tee'd stream becomes errored. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/b07b0b1afe66
Part 9: Rename function that implements ReadableStreamControllerCanCloseOrEnqueue. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/956a058f383c
Part 10: Rename a local variable to follow the `unwrapped` convention. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/60668e84331f
Part 11: Remaining random changes. r=jwalden
You need to log in before you can comment on or make changes to this bug.