Closed Bug 1504464 Opened 6 years ago Closed 6 years 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)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
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
Depends on D14498
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
Depends on D14508
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.

Attachment

General

Created:
Updated:
Size: