feat(ext/web): BYOB support for ReadableStream #12616
Conversation
|
Yes, its not completely done yet. there are a few TODOs that hopefully will get resolved in the morning. once those are solved, I will enable all the WPT tests related to this, and then it should be ready for merging if there are no issues |
| */ | ||
| function canTransferArrayBuffer(O) { | ||
| assert(typeof O === "object"); | ||
| assert(O instanceof ArrayBuffer || O instanceof SharedArrayBuffer); |
There was a problem hiding this comment.
SharedArrayBuffers aren't transferable, and I don't think this spec algorithm is meant to be passed a SAB.
| if (isDetachedBuffer(O)) { | ||
| return false; | ||
| } | ||
| // TODO(@crowlKats): 4. If SameValue(O.[[ArrayBufferDetachKey]], undefined) is false, return false. |
There was a problem hiding this comment.
I believe this check is the same as v8::ArrayBuffer::is_detachable. The reason for this seems to be to make it impossible to detach the buffer that backs a WebAssembly.Memory object other than through its grow method (https://webassembly.github.io/spec/js-api/#create-a-memory-buffer).
There was a problem hiding this comment.
Yes. I suggest we do this in a follow up, as it requires core bindings.
| "ReadableStream with byte source: respondWithNewView() with a zero-length view (in the closed state)", | ||
| "ReadableStream with byte source: respondWithNewView() with a transferred non-zero-length view (in the readable state)", | ||
| "ReadableStream with byte source: respondWithNewView() with a transferred zero-length view (in the closed state)" | ||
| "ReadableStream with byte source: Respond to multiple pull() by separate enqueue()" |
There was a problem hiding this comment.
After investigation I am about 80% confident this test being broken is a result of our broken microtask queuing: #11731
There was a problem hiding this comment.
Thank you @crowlKats. Awesome to have this implemented now!
Other than the one failing test due to microtask queuing, all WPT pass.
Closes #11025