Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite to use Web IDL, and generally modernize #1035

Merged
merged 30 commits into from
Jun 11, 2020
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Apr 13, 2020

Closes #963.

Normative changes to widely-implemented features, roughly in order of most disruptive to least-disruptive:

  • For the queuing strategy classes, their size and highWaterMark properties are now getters on the prototype, instead of data properties on the prototype and instance (respectively). Closes Specifying QueuingStrategies in WebIDL #1005. In particular this means that attempts to set either of them post-creation will throw a TypeError. Chromium already ships these semantics.

  • Functions which take a dictionary no longer accept non-objects.

  • For the queuing strategy classes, their highWaterMark property will no longer return a non-number from their highWaterMark properties, if one was passed to the constructor. Instead, NaN will be returned.

  • All methods and accessors are now enumerable, per Web IDL defaults, instead of non-enumerable, per ECMAScript defaults.

  • All classes are now exposed globally. Formerly, ReadableStreamDefaultReader, ReadableStreamBYOBReader, ReadableStreamDefaultController, ReadableByteStreamController, WritableStreamDefaultWriter, WritableStreamDefaultController, and TransformStreamDefaultController were not exposed. Closes Should ReadableStreamDefaultReader be exposed? #586.

  • All classes now have [Symbol.toStringTag] properties. Closes Question: Should Streams have the prototype for Symbol.toStringTag and toString declared? #952.

  • Some functions have changed their length property value.

  • Some exceptions are thrown earlier, at argument-conversion time.

  • Property lookup in options arguments now happens earlier, at argument-conversion time, and in alphabetical order, per dictionary rules.

Normative changes to unimplemented features:

  • ReadableStream's getIterator() method has been renamed to values() as part of adopting Web IDL's infrastructure for async iterators.

  • The byobRequest property on ReadableByteStreamController now returns null when there is no BYOB request, instead of returning undefined.

  • The view property on ReadableStreamBYOBRequest now returns null when the view cannot be written into, instead of returning undefined.

  • Various byte-stream-related APIs that used to specifically prohibit detached buffers now check for zero-length views or buffers, which is a more general category.

  • The async iterator's next() and return() methods now behave more like async generators, e.g. returning promises fulfilled with { value: undefined, done: true } after return()ing the iterator, instead of returning a rejected promise.

Editorial changes:

Other bug fixes:


Note to reviewers: this isn't really a fun thing to review. I started by trying to do this incrementally, e.g. removing emu-algify first, then reformatting to WHATWG conventions/modern Bikeshed syntax, then Web IDL-ifying, to make it more reviewable. But this way turned out to be a lot easier for me, and I don't think this would have gotten done if I'd taken the harder route.

The best review strategy I can think of is just to scroll around randomly in the output preview and see if anything catches your eye.


TODOs:

  • Add back async iterators, after Add async value iterators and async iterator arguments webidl#808 arrives.
  • Update the reference implementation. Probably use webidl2js. (In progress)
  • See what tests break. Update them all. (In progress)
  • Add idlharness tests.
  • Add tests for the per-global-ness of the queuing strategy functions.
  • (Optional) Consider removing the hand-curated IDs for non-exported abstract operations. They are painful to maintain and don't buy us much.
  • (Optional) Consider renaming internal slot names to be less long-winded now that they are no longer used for brand checks
  • (Optional) Consider renaming internal slot names to be uppercase to solve Use [[UpperCamelCase]] in internal slots #825.

Here's the pull request template, which we should update as we complete the above list of TODOs. I think in particular asking for implementer interest will be more feasible after we have the test changes up, although the "normative changes" list above is reasonable by itself.

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

Normative changes, all stemming from the Web IDL adaptation:

* All classes are now exposed globally. Formerly, ReadableStreamDefaultReader, ReadableStreamBYOBReader, ReadableStreamDefaultController, ReadableByteStreamController, WritableStreamDefaultWriter, WritableStreamDefaultController, and TransformStreamDefaultController were not exposed. Closes #586.

* All classes now have [Symbol.toStringTag] properties. (At least, pending whatwg/webidl#357 resolution.) Closes #952.

* All methods and accesors are now enumerable, per Web IDL defaults, instead of non-enumerable, per ECMAScript defaults.

* For the queuing strategy classes, their size and highWaterMark properties are now getters on the prototype, instead of data properties on the prototype and instance (respectively). Closes #1005. Note that the size function is not settable anymore, but highWaterMark has a setter.

* Some functions have changed their length property value.

* Some exceptions are thrown earlier, at argument-conversion time.

Editorial changes:

* All APIs are specified to using Web IDL now, instead of using a modified version of the ECMAScript specification conventions. We continue using abstract operations and completion records for now, and we have to drop down to the ECMAScript level in a couple places (notably for dealing with %ObjectPrototype% vs. null-prototype iteration result objects, and transferring array buffers). But overall this removes a lot of type-checking and conversion boilerplate from the specification.

* Individual abstract operations, constructors, methods, and properties no longer have their own heading. They are instead lumped together in sections. Closes #885.

* The constructors, methods, and properties are now documented in a per-class block, using the usual WHATWG "domintro" style. Closes #907.

* Abstract operations are now consistently alphabetized within their section. Closes #684.

* By using Bikeshed's <div algorithm> feature, we now get automatic identifier highlighting. Closes #687.

* Switched to 100-character line limits, 1-space indents, and omitting end tags, per WHATWG conventions.

* Removed usage of emu-algify in favor of using some more of Bikeshed's built-in features, plus manually annotating a few things.

* Switched to concise Bikeshed linking syntax, e.g. [=term=] and [$AbstractOp$].

* Eliminated a number of utility abstract operations, especially around calling functions, by better using Web IDL.

Other bug fixes:

* Web IDL makes constructor behavior clear, so this closes #965.
@ricea
Copy link
Collaborator

ricea commented Apr 14, 2020

Wow. It might take me a little time to review this.

I assume we don't have to mention minor normative changes like the order of lookup of dictionary fields changing?

@domenic
Copy link
Member Author

domenic commented Apr 14, 2020

I'd love to have the list of normative changes as exhaustive as possible, at least to the extent of including general points like that. (It seems similar to my "Some exceptions are thrown earlier, at argument-conversion time".) I'll edit the OP to include that, and if you can think of any other such changes, just let me know.

@ricea
Copy link
Collaborator

ricea commented Apr 14, 2020

Possible breaking change: the first argument of the constructors of ReadableStream, WritableStream and TransformStream is now compulsory.

Methods which take a dictionary will be stricter about what types they accept. For example readable.getReader(true) will no longer work.

An additional irrelevant change for queuing strategies: currently new CountQueuingStrategy({highWaterMark: 'sandwich'}).highWaterMark returns "sandwich". After this change, it will return NaN.

@domenic
Copy link
Member Author

domenic commented Apr 14, 2020

Possible breaking change: the first argument of the constructors of ReadableStream, WritableStream and TransformStream is now compulsory.

Note that this is relatively easy to reverse, and maybe we should do so to minimize unnecessary breaking changes. On the other hand, I have a hard time imagining code usefully using no-argument versions of these...

Oh wait, no-arg TransformStream is actually really important. OK, well, we should definitely allow that. I think I'll just allow it for all of them.

- Update dependencies
- Update WPT submodule
- Re-do the queuing strategies in the new style. (Tests not updated yet.)
@ricea
Copy link
Collaborator

ricea commented Apr 15, 2020

Reference implementation changes look good so far.

domenic added a commit to web-platform-tests/wpt that referenced this pull request Apr 15, 2020
But tests are failing all over the place so there are probably some dumb mistakes in the conversion still
@domenic
Copy link
Member Author

domenic commented Apr 17, 2020

The reference implemenatation is mostly ported. I want to wait for the following webidl2js PRs to land and then reassess the test pass rate:

Copy link
Collaborator

@MattiasBuelens MattiasBuelens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through the new spec text. Incredible work! I'm glad to see that almost all of the manual type checks have now been obsoleted by WebIDL. 👏

I'll go through the reference implementation another time. 😅

index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
Copy link
Collaborator

@ricea ricea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've so far reviewed the preview up to the end of ReadableStream. It's very high quality. I haven't checked that all the links go to the right places. I think we might need to trust in Bikeshed for that.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated
promises returned by {{WritableStreamDefaultWriter/write()}}; in this case we'll be notified about any failures
<code>await</code>ing the {{WritableStreamDefaultWriter/ready}} promise.
(These are defined as internal methods, instead of as abstract operations, so that they can be
called polymorphically by the {{Readab
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: manipualtion

@ricea
Copy link
Collaborator

ricea commented Apr 21, 2020

The preview lgtm.

I'm going to review the reference implementation changes next. After that I will do a light review of the markup.

@domenic domenic deleted the webidl-rewrite branch June 11, 2020 15:42
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 17, 2020
…sion, a=testonly

Automatic update from web-platform-tests
Streams: update tests for Web IDL conversion

Follows whatwg/streams#1035. Notable changes:

- Updates for the normative changes listed there.
- Introduce an idlharness test
- Remove various tests for things that are covered by idlharness, such as brand checks, the order of getting values from dictionaries, etc.
- Updated for the fact that everything is now globally exposed, so some roundabout code to get constructors can be removed.
- Slight timing updates: the pattern of returning a promise from an underlying sink/source/transformer `start()` function, then waiting on that before doing asserts, does not work with Web IDL's "promise resolved with". So instead we use `flushAsyncEvents()` to wait a little longer.
- Consolidated queuing strategy tests into a single file, since after deleting the things covered by idlharness, most of the tests are shared.
- Added tests that underlyingSink/underlyingSource are converted after queuingStrategy, since those dictionary conversions are done in prose.
- Added a test for various updates to the Web IDL async iterators specification.
--

wpt-commits: 887350c2f46def5b01c4dd1f8d2eee35dfb9c5bb
wpt-pr: 22982
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jun 18, 2020
…sion, a=testonly

Automatic update from web-platform-tests
Streams: update tests for Web IDL conversion

Follows whatwg/streams#1035. Notable changes:

- Updates for the normative changes listed there.
- Introduce an idlharness test
- Remove various tests for things that are covered by idlharness, such as brand checks, the order of getting values from dictionaries, etc.
- Updated for the fact that everything is now globally exposed, so some roundabout code to get constructors can be removed.
- Slight timing updates: the pattern of returning a promise from an underlying sink/source/transformer `start()` function, then waiting on that before doing asserts, does not work with Web IDL's "promise resolved with". So instead we use `flushAsyncEvents()` to wait a little longer.
- Consolidated queuing strategy tests into a single file, since after deleting the things covered by idlharness, most of the tests are shared.
- Added tests that underlyingSink/underlyingSource are converted after queuingStrategy, since those dictionary conversions are done in prose.
- Added a test for various updates to the Web IDL async iterators specification.
--

wpt-commits: 887350c2f46def5b01c4dd1f8d2eee35dfb9c5bb
wpt-pr: 22982
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Oct 21, 2020
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from readable streams.

Bug: 1093862
Change-Id: I2cab1814a7d34beeca16b15fd6c64339d296952f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485986
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Cr-Commit-Position: refs/heads/master@{#819180}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Oct 21, 2020
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from writable streams.

Bug: 1093862
Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Cr-Commit-Position: refs/heads/master@{#819181}
pull bot pushed a commit to FreddyZeng/chromium that referenced this pull request Oct 21, 2020
This reverts commit a2505c5.

Reason for revert: Broken bots, possibly caused by CQ collision. See https://crbug.com/1140836

Original change's description:
> Remove NotEnumerable from Writable Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from writable streams.
>
> Bug: 1093862
> Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Cr-Commit-Position: refs/heads/master@{#819181}

TBR=ricea@chromium.org,nidhijaju@google.com

Change-Id: I3f49640931f5968e1127664f6d755525dcdb9543
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1093862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489062
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819225}
pull bot pushed a commit to FreddyZeng/chromium that referenced this pull request Oct 21, 2020
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from transform streams.

Bug: 1093862
Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819230}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Oct 21, 2020
This reverts commit b52b97c.

Reason for revert: It is breaking external/wpt/streams/idlharness.any.html based on bisection.

Original change's description:
> Remove NotEnumerable from Transform Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from transform streams.
>
> Bug: 1093862
> Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#819230}

TBR=ricea@chromium.org,nidhijaju@google.com

Change-Id: I7176391031e4bbf89960fe6d7f358a520d06dd21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1093862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487745
Reviewed-by: Fergal Daly <fergal@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819275}
pull bot pushed a commit to Mu-L/chromium that referenced this pull request Oct 21, 2020
This is a reland of b52b97c

Original change's description:
> Remove NotEnumerable from Transform Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from transform streams.
>
> Bug: 1093862
> Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#819230}

Bug: 1093862
Change-Id: Ic3d119e1c5d2192a3159fb672a5782fa2af8aedc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2488946
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819380}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Oct 22, 2020
This is a reland of a2505c5

Original change's description:
> Remove NotEnumerable from Writable Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from writable streams.
>
> Bug: 1093862
> Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Cr-Commit-Position: refs/heads/master@{#819181}

Bug: 1093862
Change-Id: Iaf54f395cc649b85cbc834dc18baf94855d53613
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489110
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819563}
gsnedders added a commit to gsnedders/web-platform-tests that referenced this pull request May 17, 2021
See whatwg/streams#1035. No new test added for
its exposure as this is covered by streams/idlharness.any.js.
gsnedders added a commit to web-platform-tests/wpt that referenced this pull request May 17, 2021
See whatwg/streams#1035. No new test added for
its exposure as this is covered by streams/idlharness.any.js.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 20, 2021
…ow be exposed, a=testonly

Automatic update from web-platform-tests
WritableStreamDefaultController should now be exposed

See whatwg/streams#1035. No new test added for
its exposure as this is covered by streams/idlharness.any.js.

--

wpt-commits: 59873545965cc737221c40c4004f2a3c9ce23f28
wpt-pr: 29015
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from readable streams.

Bug: 1093862
Change-Id: I2cab1814a7d34beeca16b15fd6c64339d296952f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485986
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Cr-Commit-Position: refs/heads/master@{#819180}
GitOrigin-RevId: f7cdb8cbb21d8c199fd5cd9fef499895b7fdfa8f
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from writable streams.

Bug: 1093862
Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Cr-Commit-Position: refs/heads/master@{#819181}
GitOrigin-RevId: a2505c5b931d8e19121da681ae02eed9391be37a
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This reverts commit a2505c5b931d8e19121da681ae02eed9391be37a.

Reason for revert: Broken bots, possibly caused by CQ collision. See https://crbug.com/1140836

Original change's description:
> Remove NotEnumerable from Writable Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from writable streams.
>
> Bug: 1093862
> Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Cr-Commit-Position: refs/heads/master@{#819181}

TBR=ricea@chromium.org,nidhijaju@google.com

Change-Id: I3f49640931f5968e1127664f6d755525dcdb9543
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1093862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489062
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819225}
GitOrigin-RevId: ba0e000e46284dc6e7badb690fc976e721f28b83
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
According to whatwg/streams#1035, all methods
and accessors are now enumerable, per Web IDL defaults, instead of
non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
be removed from the Streams WebIDL files. This CL specifically
removes them from transform streams.

Bug: 1093862
Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819230}
GitOrigin-RevId: b52b97cb36de5ae9a366d76548b6f9aa32e234d7
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This reverts commit b52b97cb36de5ae9a366d76548b6f9aa32e234d7.

Reason for revert: It is breaking external/wpt/streams/idlharness.any.html based on bisection.

Original change's description:
> Remove NotEnumerable from Transform Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from transform streams.
>
> Bug: 1093862
> Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#819230}

TBR=ricea@chromium.org,nidhijaju@google.com

Change-Id: I7176391031e4bbf89960fe6d7f358a520d06dd21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1093862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487745
Reviewed-by: Fergal Daly <fergal@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819275}
GitOrigin-RevId: be460e57593cdad10ec14f1411f40c3a41d17ab8
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This is a reland of b52b97cb36de5ae9a366d76548b6f9aa32e234d7

Original change's description:
> Remove NotEnumerable from Transform Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from transform streams.
>
> Bug: 1093862
> Change-Id: I509b470722039ebad20b8caeaa7f34189c24b1ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486006
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#819230}

Bug: 1093862
Change-Id: Ic3d119e1c5d2192a3159fb672a5782fa2af8aedc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2488946
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819380}
GitOrigin-RevId: 04529c04d8c3a3f947386b271013390102dfb055
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This is a reland of a2505c5b931d8e19121da681ae02eed9391be37a

Original change's description:
> Remove NotEnumerable from Writable Streams WebIDL
>
> According to whatwg/streams#1035, all methods
> and accessors are now enumerable, per Web IDL defaults, instead of
> non-enumerable, per ECMAScript defaults. Hence, 'NotEnumerable' can
> be removed from the Streams WebIDL files. This CL specifically
> removes them from writable streams.
>
> Bug: 1093862
> Change-Id: I935a9f8726b3cb1dc4efb5b4d6db71384c8efe37
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485664
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
> Cr-Commit-Position: refs/heads/master@{#819181}

Bug: 1093862
Change-Id: Iaf54f395cc649b85cbc834dc18baf94855d53613
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489110
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819563}
GitOrigin-RevId: 39275616594253c3840977a1d1b390bc679db3e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment