Repository navigation
script: Invoke decodeAudioData error callback for detached buffers - #46870
Merged
Merged
Conversation
|
🔨 Triggering try run (#30515789168) for Linux (WPT) |
|
Test results for linux-wpt from try job (#30515789168): Flaky unexpected result (39)
Stable unexpected results that are known to be intermittent (18)
|
|
✨ Try run (#30515789168) succeeded. |
|
🔨 Triggering try run (#30689210043) for Linux (WPT) |
|
Test results for linux-wpt from try job (#30689210043): Flaky unexpected result (36)
Stable unexpected results that are known to be intermittent (12)
|
|
✨ Try run (#30689210043) succeeded. |
jdm
approved these changes
Aug 12, 2026
jdm
left a comment
Member
There was a problem hiding this comment.
Great work! Just a couple small changes before we merge this.
Member
|
Also, the fact that no tests changed results is very suspicious. Can we add a new test to tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface that passes the error callback and verifies that it is invoked? i don't see any tests in https://github.com/search?q=repo%3Aservo%2Fservo+decodeaudiodata+language%3AHTML&type=code&l=HTML that pass either the success or error callbacks. https://book.servo.org/contributing/testing.html#add-a-new-test has instructions on this. |
amittenak47
marked this pull request as draft
August 16, 2026 06:40
Contributor
Author
Ok, I marked it as a draft so I can work on it Sorry about that; I didn't get any Github notifications! |
amittenak47
force-pushed
the
issue-46602
branch
2 times, most recently
from
August 25, 2026 21:32
d84da86 to
b089e20
Compare
Collaborator
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#62205) with upstreamable changes. |
Collaborator
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#62205). |
amittenak47
force-pushed
the
issue-46602
branch
from
August 26, 2026 00:10
a070827 to
d5f7ab1
Compare
amittenak47
marked this pull request as ready for review
August 26, 2026 00:11
Collaborator
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#62205). |
Previously, the detached ArrayBuffer branch only rejected the promise and the decodeErrorCallback was never invoked. Now, we detect if the ArrayBuffer is detached with IsDetachedArrayBufferObject, and synchronously reject the promise with DataCloneError before enqueuing a media element task to invoke the callback. We use a uuid clone to insert the callback in decode_resolvers and use the uuid in the task to find the callback. Because decode_error_callback is Rc and not Send, which Task expects, it cannot be moved into the queued closure. Added Step 1 from the spec to reject with InvalidStateError when the document is not fully active. Updated the decodeAudioData success callback and error callback to be optional nullable to match the Web Audio IDL, so we can pass null for an unused callback. Fixes servo#46602. Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Previously, the detached ArrayBuffer branch only rejected the promise and the decodeErrorCallback was never invoked. Now, we detect if the ArrayBuffer is detached with IsDetachedArrayBufferObject, and synchronously reject the promise with DataCloneError before enqueuing a media element task to invoke the callback. We use a uuid clone to insert the callback in decode_resolvers and use the uuid in the task to find the callback. Because decode_error_callback is Rc and not Send, which Task expects, it cannot be moved into the queued closure. Added Step 1 from the spec to reject with InvalidStateError when the document is not fully active. Updated the decodeAudioData success callback and error callback to be optional nullable to match the Web Audio IDL, so we can pass null for an unused callback. Fixes servo#46602. Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Co-authored-by: Josh Matthews <josh@joshmatthews.net> Signed-off-by: amittenak47 <55721729+amittenak47@users.noreply.github.com>
Reuse the same DOMException when rejecting the promise and invoking the error callback, wrapping it in a Trusted::new(). Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Pass the error callback and assert it is invoked with DataCloneError when the ArrayBuffer is detached. Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu>
amittenak47
force-pushed
the
issue-46602
branch
from
August 28, 2026 23:22
d5f7ab1 to
2ee348b
Compare
Contributor
Author
Thank you, I updated the harness to not expect the tests to time out. Should expect PASS now |
Collaborator
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#62205). |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 29, 2026
Collaborator
|
⛔ Failed to properly merge the upstream pull request (web-platform-tests/wpt#62205). Please address any CI issues and try to merge manually. |
Taym95
reviewed
Sep 19, 2026
| // Reads the JSObject pointer to check if the array buffer is detached. | ||
| #[expect(unsafe_code)] | ||
| fn is_detached_array_buffer(array_buffer: &ArrayBuffer) -> bool { | ||
| unsafe { IsDetachedArrayBufferObject(*array_buffer.underlying_object()) } |
Member
There was a problem hiding this comment.
we should avoid calling IsDetachedArrayBufferObject directly and use buffer_source.rs
RichardTjokroutomo
pushed a commit
to RichardTjokroutomo/servo
that referenced
this pull request
Oct 6, 2026
…ervo#46870) Previously, the detached ArrayBuffer branch only rejected the promise and the `decodeErrorCallback` was never invoked. Now, we detect if the ArrayBuffer is detached with `IsDetachedArrayBufferObject`, and synchronously reject the promise with `DataCloneError` before enqueuing a media element task to invoke the callback. We use a uuid clone to insert the callback in `decode_resolvers` and use the uuid in the task to find the callback. Because `decode_error_callback` is Rc and not Send, which Task expects, it cannot be moved into the queued closure (so we store the callback in the map and enqueue the Task). Added Step 1 from the spec to reject with `InvalidStateError` when the document is not fully active. Updated the `decodeAudioData` success callback and error callback to be optional nullable to match the Web Audio IDL, so we can pass null for an unused callback. Testing: - WebIDL and code changes compiles in release build: `.\mach build -r` - Call `decodeAudioData` after detaching `AudioContext` iframe: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-detached-execution-context.html` - Call `decodeAudioData` after detaching `OfflineAudioContext` iframe: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-offlineaudiocontext-interface/offlineaudiocontext-detached-execution-context.html` - Call `decodeAudioData`on a closed context: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-suspend-resume-close.html` - Manually detach buffer with null success callback: Using console in FireFox Dev Tools: `.\mach run -r --devtools=6080` ```js var ctx = new AudioContext(); var ab = new ArrayBuffer(4); structuredClone({}, { transfer: [ab] }); var called = false; ctx.decodeAudioData(ab, null, function () { called = true; }) .catch(function (e) { console.log("reject", e.name, e.message); }); setTimeout(function () { console.log("callback called?", called); }, 500); ``` Fixes servo#46602. --------- Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu> Signed-off-by: amittenak47 <55721729+amittenak47@users.noreply.github.com> Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Psychpsyo
pushed a commit
to Psychpsyo/servo
that referenced
this pull request
Oct 12, 2026
…ervo#46870) Previously, the detached ArrayBuffer branch only rejected the promise and the `decodeErrorCallback` was never invoked. Now, we detect if the ArrayBuffer is detached with `IsDetachedArrayBufferObject`, and synchronously reject the promise with `DataCloneError` before enqueuing a media element task to invoke the callback. We use a uuid clone to insert the callback in `decode_resolvers` and use the uuid in the task to find the callback. Because `decode_error_callback` is Rc and not Send, which Task expects, it cannot be moved into the queued closure (so we store the callback in the map and enqueue the Task). Added Step 1 from the spec to reject with `InvalidStateError` when the document is not fully active. Updated the `decodeAudioData` success callback and error callback to be optional nullable to match the Web Audio IDL, so we can pass null for an unused callback. Testing: - WebIDL and code changes compiles in release build: `.\mach build -r` - Call `decodeAudioData` after detaching `AudioContext` iframe: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-detached-execution-context.html` - Call `decodeAudioData` after detaching `OfflineAudioContext` iframe: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-offlineaudiocontext-interface/offlineaudiocontext-detached-execution-context.html` - Call `decodeAudioData`on a closed context: `.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-suspend-resume-close.html` - Manually detach buffer with null success callback: Using console in FireFox Dev Tools: `.\mach run -r --devtools=6080` ```js var ctx = new AudioContext(); var ab = new ArrayBuffer(4); structuredClone({}, { transfer: [ab] }); var called = false; ctx.decodeAudioData(ab, null, function () { called = true; }) .catch(function (e) { console.log("reject", e.name, e.message); }); setTimeout(function () { console.log("callback called?", called); }, 500); ``` Fixes servo#46602. --------- Signed-off-by: Amit Anilkumar <aanilkumar30@gatech.edu> Signed-off-by: amittenak47 <55721729+amittenak47@users.noreply.github.com> Co-authored-by: Josh Matthews <josh@joshmatthews.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the detached ArrayBuffer branch only rejected the promise and the
decodeErrorCallbackwas never invoked.Now, we detect if the ArrayBuffer is detached with
IsDetachedArrayBufferObject, and synchronously reject the promise withDataCloneErrorbefore enqueuing a media element task to invoke the callback. We use a uuid clone to insert the callback indecode_resolversand use the uuid in the task to find the callback. Becausedecode_error_callbackis Rc and not Send, which Task expects, it cannot be moved into the queued closure (so we store the callback in the map and enqueue the Task).Added Step 1 from the spec to reject with
InvalidStateErrorwhen the document is not fully active.Updated the
decodeAudioDatasuccess callback and error callback to be optional nullable to match the Web Audio IDL, so we can pass null for an unused callback.Testing:
WebIDL and code changes compiles in release build:
.\mach build -rCall
decodeAudioDataafter detachingAudioContextiframe:.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-detached-execution-context.htmlCall
decodeAudioDataafter detachingOfflineAudioContextiframe:.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-offlineaudiocontext-interface/offlineaudiocontext-detached-execution-context.htmlCall
decodeAudioDataon a closed context:.\mach test-wpt -r tests/wpt/tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-suspend-resume-close.htmlManually detach buffer with null success callback:
Using console in FireFox Dev Tools:
.\mach run -r --devtools=6080Fixes #46602.