Closed Bug 1503006 Opened 3 years ago Closed 3 years ago

ReadableStream cleanup, vol. II

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

Attachments

(12 files, 1 obsolete file)

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
47 bytes, text/x-phabricator-request
Details | Review
*   Add trivial non-static helper methods for getting (and setting, as needed)
    each fixed slot of each class

*   Rename some static methods to be toplevel functions with the exact names
    used in the standard

*   Remove uses of Maybe<AutoRealm>

*   Re-check no non-static methods take a cx argument

*   Get rid of unwrapped1 and unwrapped2

*   Rename UnwrapSlot -> UnwrapPrivateSlot or UnwrapInternalSlot

*   Rename UnwrapThis -> UnwrapThisForNonGenericMethod, with comment
    explaining how that relates to the spec
Oh, and the big one:

*   Finish adding `unwrapped` prefix to variables and arguments that aren't same-compartment with cx.
Blocks: streams-meta
(In reply to Jason Orendorff [:jorendorff] from comment #0)
> *   Get rid of unwrapped1 and unwrapped2
> 
> *   Rename UnwrapSlot -> UnwrapPrivateSlot or UnwrapInternalSlot
> 
> *   Rename UnwrapThis -> UnwrapThisForNonGenericMethod, with comment
>     explaining how that relates to the spec

I went ahead and made these changes in bug 1499813 prior to landing.
Depends on D10374
Attachment #9021543 - Attachment is obsolete: true
(In reply to Jason Orendorff [:jorendorff] from comment #1)
> Oh, and the big one:
> 
> *   Finish adding `unwrapped` prefix to variables and arguments that aren't
> same-compartment with cx.

I filed bug 1503718 for this.
This commit changes ReadableStreamReaderGenericRelease to use
UnwrapInternalSlot when accessing reader.[[closedPromise]]. Before this commit,
the code assumed that there would be a promise in the slot, not a wrapper.
I think that was actually correct, because it's impossible for the stream to
still be readable and the reader to have a [[closedPromise]] that's a wrapper.
Still, it's a bit precarious (and I'm still not 100% sure about that), so I just
changed it to unwrap.

Depends on D10374
In passing, change ReadableStreamControllerPullSteps to use explicit downcasts,
so it's a little more obvious what's going on.

Depends on D10430
This also reorders the remaining members of class ReadableStream a bit.

Depends on D10435
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d3ea04e6d6b
Part 1: Trivial whitespace fixes. r=tcampbell
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98c55ece0f42
Part 2: Trivial slot accessor methods for class ReadableStream. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/3f91b9f1dd38
Part 3: Trivial slot accessor methods for class ReadableStreamReader. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/64f5848790f7
Part 4: Trivial slot accessor methods for class ReadableStreamController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8f0bef2f1dfb
Part 5: Trivial slot accessor methods for class ReadableStreamDefaultController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/b54266154e56
Part 6: Trivial slot accessor methods for class ReadableByteStreamController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d1580501b0d1
Part 7: Clean up remaining slot constants. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/bb238cffc154
Part 8: Move implementation of public Stream API functions from jsapi.cpp to builtin/Stream.cpp. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/4fc5492e340c
Part 9: Code motion only. Move some static Stream methods immediately after their sole callers. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/ca8bd4e2151e
Part 10: Eliminate some static methods that are one-to-one with API entry points. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/5782af4a237c
Part 11: Rename two remaining static methods to toplevel functions. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/f45aeb07cca6
Part 12: Eliminate Maybe<AutoRealm> from Stream.cpp. r=tcampbell
Backout by aciure@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cdee543b1d37
Backed out 11 changesets for landing the wrong version of patches CLOSED TREE
Blocks: 1503867
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b88c95bd8348
Part 2: Trivial slot accessor methods for class ReadableStream. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/992f76656e11
Part 3: Trivial slot accessor methods for class ReadableStreamReader. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/45378da7430c
Part 4: Trivial slot accessor methods for class ReadableStreamController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/302a7eb7a07a
Part 5: Trivial slot accessor methods for class ReadableStreamDefaultController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/536afe592a5b
Part 6: Trivial slot accessor methods for class ReadableByteStreamController. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/2eeb0448e657
Part 7: Clean up remaining slot constants. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/e6ae3beb07bb
Part 8: Move implementation of public Stream API functions from jsapi.cpp to builtin/Stream.cpp. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/21fa1b12f15b
Part 9: Code motion only. Move some static Stream methods immediately after their sole callers. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/902100ba35f6
Part 10: Eliminate some static methods that are one-to-one with API entry points. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c8fdb50621b0
Part 11: Rename two remaining static methods to toplevel functions. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/85facdfd7332
Part 12: Eliminate Maybe<AutoRealm> from Stream.cpp. r=tcampbell
Assignee: nobody → jorendorff
You need to log in before you can comment on or make changes to this bug.