1503006 - ReadableStream cleanup, vol. II

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jason Orendorff [:jorendorff]]

*   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

Comment 1

[Jason Orendorff [:jorendorff]]

Oh, and the big one:

*   Finish adding `unwrapped` prefix to variables and arguments that aren't same-compartment with cx.

Comment 2

[Jason Orendorff [:jorendorff]]

(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.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 2: Trivial slot accessor methods for class ReadableStream. r?tcampbell

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 2 alternate universe

Depends on D10374

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 1: Trivial whitespace fixes. r?tcampbell

Comment 6

[Jason Orendorff [:jorendorff]]

(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.

Comment 7

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 3: Trivial slot accessor methods for class ReadableStreamReader. r?tcampbell

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

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 4: Trivial slot accessor methods for class ReadableStreamController. r?tcampbell

Depends on D10428

Comment 9

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 5: Trivial slot accessor methods for class ReadableStreamDefaultController. r?tcampbell

Depends on D10429

Comment 10

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 6: Trivial slot accessor methods for class ReadableByteStreamController. r?tcampbell

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

Depends on D10430

Comment 11

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 7: Clean up remaining slot constants. r?tcampbell

Depends on D10431

Comment 12

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 8: Move implementation of public Stream API functions from jsapi.cpp to builtin/Stream.cpp. r?tcampbell

Depends on D10432

Comment 13

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 9: Code motion only. Move some static Stream methods immediately after their sole callers. r?tcampbell

Depends on D10433

Comment 14

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 10: Eliminate some static methods that are one-to-one with API entry points. r?tcampbell

Depends on D10434

Comment 15

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 11: Rename two remaining static methods to toplevel functions. r?tcampbell

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

Depends on D10435

Comment 16

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1503006 - Part 12: Eliminate Maybe<AutoRealm> from Stream.cpp. r?tcampbell

Depends on D10436

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d3ea04e6d6b
Part 1: Trivial whitespace fixes. r=tcampbell

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

Comment 19

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdee543b1d37
Backed out 11 changesets for landing the wrong version of patches CLOSED TREE

Comment 20

[Pulsebot]

Pushed by [e-mail]:
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

Comment 21

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b88c95bd8348
https://hg.mozilla.org/mozilla-central/rev/992f76656e11
https://hg.mozilla.org/mozilla-central/rev/45378da7430c
https://hg.mozilla.org/mozilla-central/rev/302a7eb7a07a
https://hg.mozilla.org/mozilla-central/rev/536afe592a5b
https://hg.mozilla.org/mozilla-central/rev/2eeb0448e657
https://hg.mozilla.org/mozilla-central/rev/e6ae3beb07bb
https://hg.mozilla.org/mozilla-central/rev/21fa1b12f15b
https://hg.mozilla.org/mozilla-central/rev/902100ba35f6
https://hg.mozilla.org/mozilla-central/rev/c8fdb50621b0
https://hg.mozilla.org/mozilla-central/rev/85facdfd7332