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

@MattiasBuelens commented on this pull request.



>  <div algorithm>
     The <dfn export for="BufferSource">byte length</dfn> of a [=buffer source type=] instance
     |bufferSource| is the value returned by the following steps:
 
     1.  Let |jsBufferSource| be the result of [=converted to a JavaScript value|converting=]
         |bufferSource| to a JavaScript value.
-    1.  If |jsBufferSource| has a \[[ViewedArrayBuffer]] internal slot, then return
-        |jsBufferSource|.\[[ByteLength]].
-    1.  Return |jsBufferSource|.\[[ArrayBufferByteLength]].
+    1.  If |jsBufferSource| has a \[[ViewedArrayBuffer]] [=/internal slot=]:
+        1.  If |jsBufferSource| has a \[[TypedArrayName]] [=/internal slot=]:
+            1.  [=/Assert=]: |jsBufferSource| is a [=typed array type=] instance.
+            1.  Let |taRecord| be [$MakeTypedArrayWithBufferWitnessRecord$](|jsBufferSource|,
+                Seq-Cst).
+            1.  If [$IsTypedArrayOutOfBounds$](|taRecord|) is true, then return 0.
+            1.  Return [$TypedArrayByteLength$](|taRecord|).
+        1.  Otherwise:
+            1.  [=/Assert=]: |jsBufferSource| is a {{DataView}}.
+            1.  Let |viewRecord| be [$MakeDataViewWithBufferWitnessRecord$](|jsBufferSource|,
+                Seq-Cst).
+            1.  If [$IsViewOutOfBounds$](|viewRecord|) is true, then return 0.

That is quite horrifying! 😱

My intention is to match `(DataView|TypedArray|ArrayBuffer).prototype.byteLength` exactly, so implementors can reuse their existing implementations for those getters. I'll change it to match ECMAScript, and add a note that we're just copying their idiocies. 😅

> +    1.  If |jsBufferSource| has a \[[ViewedArrayBuffer]] [=/internal slot=]:
+        1.  If |jsBufferSource| has a \[[TypedArrayName]] [=/internal slot=]:
+            1.  [=/Assert=]: |jsBufferSource| is a [=typed array type=] instance.
+            1.  Let |taRecord| be [$MakeTypedArrayWithBufferWitnessRecord$](|jsBufferSource|,
+                Seq-Cst).
+            1.  If [$IsTypedArrayOutOfBounds$](|taRecord|) is true, then return 0.
+            1.  Return [$TypedArrayByteLength$](|taRecord|).
+        1.  Otherwise:
+            1.  [=/Assert=]: |jsBufferSource| is a {{DataView}}.
+            1.  Let |viewRecord| be [$MakeDataViewWithBufferWitnessRecord$](|jsBufferSource|,
+                Seq-Cst).
+            1.  If [$IsViewOutOfBounds$](|viewRecord|) is true, then return 0.
+            1.  Return [$GetViewByteLength$](|viewRecord|).
+    1.  Otherwise:
+        1.  [=/Assert=]: |jsBufferSource| is an {{ArrayBuffer}} or a {{SharedArrayBuffer}}.
+        1.  Return [$ArrayBufferByteLength$](|jsBufferSource|, Seq-Cst).

Right, [ArrayBuffer.prototype.byteLength](https://tc39.es/ecma262/multipage/structured-data.html#sec-get-arraybuffer.prototype.bytelength) does a detached check before reading the byte length slot. I'll add that check here too and return 0. 👍

> -    1.  Let |length| be 0.
-    1.  If |jsBufferSource| has a \[[ViewedArrayBuffer]] [=/internal slot=], then:
-        1.  Set |jsArrayBuffer| to |jsBufferSource|.\[[ViewedArrayBuffer]].
-        1.  Set |offset| to |jsBufferSource|.\[[ByteOffset]].
-        1.  Set |length| to |jsBufferSource|.\[[ByteLength]].
-    1.  Otherwise:
-        1.  Assert: |jsBufferSource| is an {{ArrayBuffer}} or
-            {{SharedArrayBuffer}} object.
-        1.  Set |length| to |jsBufferSource|.\[[ArrayBufferByteLength]].
+    1.  Let |length| be |bufferSource|'s [=BufferSource/byte length=].
+    1.  If |bufferSource| is a [=buffer view type=] instance:
+        1.  Set |arrayBuffer| to |bufferSource|'s [=underlying buffer=].
+        1.  Set |offset| to |bufferSource|'s [=ArrayBufferView/byte offset=].
+    1.  [=/Assert=]: |arrayBuffer| is an {{ArrayBuffer}} or {{SharedArrayBuffer}} object.
+    1.  Let |jsArrayBuffer| be the result of [=converted to a JavaScript value|converting=]
+        |arrayBuffer| to a JavaScript value.
     1.  If [$IsDetachedBuffer$](|jsArrayBuffer|) is true, then return the empty

You're right, we should check if *bufferSource*'s underlying buffer is detached *before* we try to read *bufferSource*'s byte offset.

I'll move this to the preceding "if *bufferSource* is a buffer view type instance" step.

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

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