Add and improve operations on BufferSources #987

domenic · 2021-06-02T21:06:46Z

Introduces algorithms for creating ArrayBuffers and ArrayBufferViews from byte sequences
Introduces algorithms for writing into ArrayBuffers and ArrayBufferViews, replacing "get a reference to the bytes held by the buffer source".
Introduces "transfer" for an ArrayBuffer.
Introduces "byte length", "underlying buffer", and "detached" for introspecting a BufferSource.
Improves the specification for "get a copy of the bytes held by a buffer source" and "detach".

Updates to dependent specs:

I'd like review here from @syg and/or other ES editors, especially for the "write" operation.

syg

Glad to see ABs and TAs having more solid foundations here!

index.bs

annevk

Nice!

I wonder how I would use this for https://encoding.spec.whatwg.org/#dom-textencoder-encodeinto.

Part of the problem I guess is https://heycam.github.io/webidl/#dfn-get-buffer-source-reference not being very clear. Perhaps we should aim to replace that completely by having a copy operation that returns a byte sequence? And instead of obtaining a reference you'd use the algorithms you have here (though we'd need to add write for views).

index.bs

domenic · 2021-06-03T15:03:17Z

Oh yeah, I like the idea of getting rid of "get a reference" in favor of having people write directly into it. And then making the "get a copy" operation more detailed using this same sort of primitive. I'll investigate.

domenic · 2021-06-03T19:17:57Z

OK, I expanded and updated this PR's scope a bit. The OP has been edited with more details. A re-review would be lovely!

I will work on an Encoding spec PR to show how the "write" concepts would be used.

Encoding wants this

syg

lgtm modulo the [[Realm]] question for ABs.

index.bs

syg · 2021-06-03T22:42:00Z

index.bs

+    1.  Let |arrayBufferData| be |arrayBuffer|.\[[ArrayBufferData]].
+    1.  Let |arrayBufferByteLength| be |arrayBuffer|.\[[ArrayBufferByteLength]].
+    1.  Perform [=?=] [$DetachArrayBuffer$](|arrayBuffer|).
+    1.  If |realm| is not given, let |realm| be |arrayBuffer|.\[[Realm]].


I don't think ABs have a [[Realm]], only functions do.

Hmm, right. I guess that is why streams uses the current Realm. Probably that's the right fallback here.

They do have a realm per IDL, see https://heycam.github.io/webidl/#dfn-associated-realm. See also tc39/ecma262#1333.

domenic · 2021-06-23T19:43:40Z

Can I get an editor review? I'm not sure merging this will work given the travis-ci.org shutdown, but I think we should at least try.

TimothyGu

Suggestions for a few more concepts to define:

BufferSource/is detached: basically an IDL version of IsDetachedBuffer that also supports views.
ArrayBufferView/get the underlying ArrayBuffer: spares specs from having to step into ES land in order to transfer the input buffer.
(possibly) ArrayBufferView/transfer

index.bs

TimothyGu · 2021-06-23T21:13:26Z

Is "get a reference to the bytes held by the buffer source" used anywhere?

domenic · 2021-06-24T22:39:49Z

I added BufferSource/is detached for now.

On the others, it occured to me that we have two directions here: super-polymorphism, where everything is defined on BufferSources, or something more true to the model where ArrayBuffers are special. I was curious for your thoughts.

Super-polymorphism:

Maybe merge ArrayBuffer/create and ArrayBufferView/create into BufferSource/create (with ArrayBuffer just being one of the types you can specify)
Merge ArrayBuffer/write and ArrayBufferView/write
Replace ArrayBuffer/detach with BufferSource/detach
Replace ArrayBuffer/transfer BufferSource/transfer

ArrayBuffers are special:

Define ArrayBufferView/get the underlying ArrayBuffer
Don't define ArrayBufferView/transfer or ArrayBufferView/detach; make it clear that you're reaching below the view level and messing with the buffer beneath
Maybe change BufferSource/is detached to just ArrayBuffer/is detached, for similar reasons (although it's less important for introspection)
Probably keep ArrayBuffer/write and ArrayBufferView/write separate, to make it clear when you're writing into a view that you only have influence over the view's range

Is "get a reference to the bytes held by the buffer source" used anywhere?

The one place I was aware of was Encoding. But I just thought to check a bit more and Web Audio is a heavy consumer. I should prepare a PR for them before we merge this.

annevk · 2021-06-25T07:52:46Z

I think it's fine to to go with "super-polymorphism" as long as those algorithms are appropriately detailed. I don't really see a downside in terms of what is possible (especially since we can provide a way to get the underlying buffer once we identify a need for that) and it makes it easier to write prose. It also shouldn't get in the way of #865 which is still something we need to do I think.

TimothyGu

On the others, it occured to me that we have two directions here: super-polymorphism, where everything is defined on BufferSources, or something more true to the model where ArrayBuffers are special. I was curious for your thoughts.

Here's an alternative: super-polymorphism for read-like operations (byte length, is detached, get a copy), ArrayBuffers are special for write-like operations (create, write, detach, transfer). The name "BufferSource" is fairly explicit about being the source, so I don't think it's worthwhile to make write operations work for any BufferSource.

Come to think of it, this layout is what you have already!

index.bs

annevk · 2021-06-25T17:52:33Z

We need write for all BufferSource types, no? How else would you write into a Uint8Array respecting the length and such?

TimothyGu · 2021-06-25T18:28:34Z

@annevk Yeah, it’s more like we define it separately from ArrayBuffer so we’d have ArrayBuffer/write and ArrayBufferView/write, rather than a combined BufferSource/write.

See whatwg/webidl#987.

domenic · 2021-06-25T19:48:29Z

Alright, we'll merge, but TBD if it'll actually deploy given the Travis situation...

bathos · 2021-06-28T12:19:52Z

Is "get a reference to the bytes held by the buffer source" used anywhere?

The one place I was aware of was Encoding. But I just thought to check a bit more and Web Audio is a heavy consumer. I should prepare a PR for them before we merge this.

Another is in WebAuthn: https://w3c.github.io/webauthn/#sctn-createCredential

* Use new Web IDL buffer primitives See whatwg/webidl#987. * Fix #2361: Use new Web IDL buffer primitives Updates spec to use new Web IDL buffer primitives. The changes are marked as candidate corrections, but we use several correction amendments to make it easy to see what the new and old text will be. Using just one makes it super-hard to figure out what controls the text, and the text sometimes doesn't always clearly show what's deleted and what's inserted because the links are colored correctly, and struck-through text doesn't strike through links. (A bikeshed issue?) * Remove link that no longer exists * Update changelog * Add script to create list of corrections for a change * Add comments * Change text slightly Change "write" to "allow writing into" so reflects better what getChannelData allows. * Add links to the previous and next change in the amendment box. * Inline the JS file * Fix up script and add event listener Fix up the script so bikeshed doesn't mangle it. Add an onload event listener so that when the doc is loaded we can run ListAmendments to add a list of the related changes for the change log. * Fix typo * Inline JS into index.bs Adjust so bikeshed doesn't mangle it. Move the listener code into the same script segment so it's easy to find. * Add some comments * Refine markup * The changes should should have the "proposed" class too. * Update text to say "Proposed", as shown in https://fantasai.inkedblade.net/style/design/w3c-restyle/2020/readme#proposed-changes * Update buttons to say "Previous Change" and "Next Change" to make it clearer what the buttons do. * Refine amendment boxes * Limit the scope of each change to a paragraph or item in an algorithm. * Move the changed section to be in the `<div>` for the amendment box to it's easier to see where the changes could be. * Fix bad numbering. The id had the correct value, but the link for the issue used the wrong change number (should be 3 instead of 2). * Remove amendents.js. Not needed because we've basically inlined it all so that the preview shows the prev/next buttons and the list of changes for the change log. Co-authored-by: Domenic Denicola <d@domenic.me> Co-authored-by: Raymond Toy <rtoy@chromium.org>

syg reviewed Jun 2, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

annevk reviewed Jun 3, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

domenic added 3 commits June 3, 2021 15:12

Add some operations bridging BufferSources and byte sequences

590b3bb

Review comments

701e3df

Expand and fix up

fce17fd

domenic force-pushed the wrapping-byte-sequences branch from a6bf834 to fce17fd Compare June 3, 2021 19:12

Add "byte length"

fb7c29e

Encoding wants this

domenic changed the title ~~Add some operations bridging BufferSources and byte sequences~~ Add and improve operations on BufferSources Jun 3, 2021

domenic mentioned this pull request Jun 3, 2021

Editorial: use new Web IDL primitives for Uint8Array writing whatwg/encoding#262

Merged

syg approved these changes Jun 3, 2021

View reviewed changes

domenic mentioned this pull request Jun 4, 2021

Allow other specifications to create readable byte streams whatwg/streams#1130

Merged

domenic added 2 commits June 4, 2021 09:15

Fix in response to review

1190c5d

Fix linking error

f935bf4

TimothyGu reviewed Jun 23, 2021

View reviewed changes

domenic added 2 commits June 24, 2021 18:29

Conversions and other fixes

be4f854

Add BufferSource/is detached

145a4cd

TimothyGu approved these changes Jun 25, 2021

View reviewed changes

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 Show resolved Hide resolved

domenic added 2 commits June 25, 2021 15:14

Review comments and Bikeshed warnings

1c98981

Add "underlying buffer"

d48fe0b

domenic added a commit to domenic/web-audio-api that referenced this pull request Jun 25, 2021

Use new Web IDL buffer primitives

d47c3d7

See whatwg/webidl#987.

domenic mentioned this pull request Jun 25, 2021

Use new Web IDL buffer primitives WebAudio/web-audio-api#2358

Closed

domenic merged commit d6e71e5 into master Jun 25, 2021

domenic deleted the wrapping-byte-sequences branch June 25, 2021 19:49

domenic mentioned this pull request Jun 25, 2021

Meta: move to GitHub Actions #992

Merged

rtoy mentioned this pull request Jun 25, 2021

Use new Web IDL buffer primitives WebAudio/web-audio-api#2361

Closed

This was referenced Aug 2, 2021

get-buffer-source-{reference,copy} ought to point to the algorithm #229

Closed

Who uses the detach concept for ArrayBuffer? #724

Closed

ExE-Boss mentioned this pull request Oct 5, 2021

Editorial: Update ECMA‑262 links #1033

Merged

domenic mentioned this pull request Jan 24, 2022

Specify realm for object creation whatwg/streams#1213

Open

domenic mentioned this pull request Mar 3, 2022

Editorial: update use of, or stop using, completion values? whatwg/streams#1224

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add and improve operations on BufferSources #987

Add and improve operations on BufferSources #987

domenic commented Jun 2, 2021 •

edited

Loading

syg left a comment •

edited

Loading

annevk left a comment

domenic commented Jun 3, 2021

domenic commented Jun 3, 2021

syg left a comment

syg Jun 3, 2021

domenic Jun 4, 2021

annevk Jun 4, 2021

domenic commented Jun 23, 2021

TimothyGu left a comment

TimothyGu commented Jun 23, 2021

domenic commented Jun 24, 2021

annevk commented Jun 25, 2021

TimothyGu left a comment

annevk commented Jun 25, 2021

TimothyGu commented Jun 25, 2021

domenic commented Jun 25, 2021

bathos commented Jun 28, 2021

Add and improve operations on BufferSources #987

Add and improve operations on BufferSources #987

Conversation

domenic commented Jun 2, 2021 • edited Loading

syg left a comment • edited Loading

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

domenic commented Jun 3, 2021

domenic commented Jun 3, 2021

syg left a comment

Choose a reason for hiding this comment

syg Jun 3, 2021

Choose a reason for hiding this comment

domenic Jun 4, 2021

Choose a reason for hiding this comment

annevk Jun 4, 2021

Choose a reason for hiding this comment

domenic commented Jun 23, 2021

TimothyGu left a comment

Choose a reason for hiding this comment

TimothyGu commented Jun 23, 2021

domenic commented Jun 24, 2021

annevk commented Jun 25, 2021

TimothyGu left a comment

Choose a reason for hiding this comment

annevk commented Jun 25, 2021

TimothyGu commented Jun 25, 2021

domenic commented Jun 25, 2021

bathos commented Jun 28, 2021

domenic commented Jun 2, 2021 •

edited

Loading

syg left a comment •

edited

Loading