Re: [whatwg/webidl] Fix `BufferSource` algorithms for shared and resizable buffers (PR #1529)

annevk left a comment (whatwg/webidl#1529)

<details>
<summary>AI feedback</summary>

Reviewed at bfe0173. Build passes, no added lines over 100 columns, no trailing whitespace. ECMAScript constant spelling (`Seq-Cst`, `Fixed-Length`, `Preserve-Resizability`) matches existing usage, and the memory orderings follow syg's rules of thumb.

**Blockers**

- [*transfer*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9506-L9507): when |preserveResizability| is true, `ArrayBufferCopyAndDetach` returns a resizable buffer. The last step then converts it to a plain IDL `ArrayBuffer`, and [that conversion](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9179-L9182) throws a `TypeError` when `IsFixedLengthArrayBuffer` is false, so the new option always throws for a resizable input. Consider converting to `[AllowResizable] ArrayBuffer` instead.
- [*get a copy of the bytes held by the buffer source*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9323) and [*ArrayBufferView/write*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9436) now go through [*underlying buffer*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9395-L9405), which converts `[[ViewedArrayBuffer]]` to a plain `ArrayBuffer`/`SharedArrayBuffer`. That conversion throws for resizable/growable buffers, so copying from or writing into a non-empty `[AllowResizable]` view throws. The old *get a copy* never converted the buffer, so this is a regression, and it's what web-platform-tests/wpt#62301 exercises. Consider having *underlying buffer* convert to the `[AllowResizable]` variants, or using `[[ViewedArrayBuffer]]` directly in these algorithms.

**Suggestions**

- [*transfer* note](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9516-L9521): the explanation about the current realm being in a different agent cluster made sense when callers could pass a |targetRealm|. A non-shared `ArrayBuffer` can't be reached from another agent cluster, so that case no longer exists. Consider shortening this to "Sufficient memory cannot be allocated for the new `ArrayBuffer`" and dropping or rewording the agent-cluster sentences.
- Consider updating the [note in *get a copy*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9318-L9319) to say that the early return is also what makes the later `IsDetachedBuffer` assert hold, since that's why the step is required now.
- The tests only cover *get a copy*. The change to the default resizability of *transfer* has no coverage yet, though that may only be testable once Streams uses it.

**Nits**

- [*byte length*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9381-L9382): the `IsTypedArrayOutOfBounds` check is redundant, since `TypedArrayByteLength` already returns 0 for an out-of-bounds typed array. Fine to keep for symmetry with the `DataView` branch, where `GetViewByteLength` does need the check.
- [*get a copy* note](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9318-L9319): "out of bounds" is scoped to `ArrayBufferView`, but |bufferSource| may be a buffer type. Maybe "detached or, for a buffer view type instance, out of bounds".
- [*byte offset*](https://github.com/whatwg/webidl/blob/bfe01731587fffaa7b33787a9d87b2f72e0770f1/index.bs#L9349-L9352) converts |view| to a JavaScript value twice (once inside *out of bounds*). Converting once and calling `IsArrayBufferViewOutOfBounds` directly would be shorter, though reusing the definition is also fine.

</details>


-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/webidl/pull/1529#issuecomment-6384354888
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/webidl/pull/1529/c6384354888@github.com>