-
Notifications
You must be signed in to change notification settings - Fork 167
Meta: implement TransferArrayBuffer correctly in reference implementation
#1353
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
Meta: implement TransferArrayBuffer correctly in reference implementation
#1353
Conversation
| const byteOffset = view.byteOffset; | ||
| const byteLength = view.byteLength; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reference implementation was doing steps 8 and 9 after calling TransferArrayBuffer, which meant that view.byteLength was already 0. We didn't notice this, because we weren't actually transferring before. 😛
Fortunately, all "real" implementations already do this correctly.
| O[isFakeDetached] = true; | ||
|
|
||
| return transferredIshVersion; | ||
| return ArrayBufferPrototypeTransferToFixedLength.call(O); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using transferToFixedLength() since we don't (yet) support resizable ArrayBuffers.
I think we can loosen that restriction, but we should do that in an actual spec change. 😉
852ded4 to
5cde2af
Compare
domenic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I guess you might want to roll in web-platform-tests/wpt#54350 before merging?
|
Done, ready to land. 🛬 |
Previously, the reference implementation did not implement the
TransferArrayBufferabstract op correctly, because we didn't have a way of doing that synchronously in user-land code. Today, we can useArrayBuffer.prototype.transfer()for that.At least two implementers are interested (and none opposed)ArrayBuffer.transfer()in Streams tests web-platform-tests/wpt#54350Implementation bugs are filedMDN issue is filed(See WHATWG Working Mode: Changes for more details.)
Preview | Diff