Skip to content

fix: proper gc in the wpt tests - #1246

Open
mdydek wants to merge 6 commits into
mainfrom
fix/unpin-wpt-windows
Open

fix: proper gc in the wpt tests#1246
mdydek wants to merge 6 commits into
mainfrom
fix/unpin-wpt-windows

Conversation

@mdydek

@mdydek mdydek commented Aug 20, 2026

Copy link
Copy Markdown
Member

Closes #

⚠️ Breaking changes ⚠️

Introduced changes

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage
  • Added support for web
  • Updated old arch android spec file

@mdydek
mdydek marked this pull request as ready for review August 20, 2026 17:07
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

WPT non-regression comparison

PASS — no regressions · 2 improved section(s) · overall 2632 → 2642 (+10)

Spec section Base pass Head pass Delta
ConstantSourceNode 59 62 +3
WaveShaperNode 75 82 +7

Per-file data unavailable in one of the reports — category-level comparison only.

Unchanged sections (26)
Spec section Base pass Head pass Delta
Processing model 0 0 0
Other 52 52 0
AnalyserNode 138 138 0
AudioBuffer 140 140 0
AudioBufferSourceNode 208 208 0
AudioContext 58 58 0
AudioNode 261 261 0
AudioParam 599 599 0
BiquadFilterNode 275 275 0
ChannelMergerNode 30 30 0
ChannelSplitterNode 7 7 0
ConvolverNode 170 170 0
DelayNode 104 104 0
DestinationNode 0 0 0
DynamicsCompressorNode 4 4 0
GainNode 15 15 0
IIRFilterNode 87 87 0
MediaElementAudioSourceNode 0 0 0
MediaStreamAudioDestinationNode 1 1 0
MediaStreamAudioSourceNode 0 0 0
OfflineAudioContext 35 35 0
OscillatorNode 104 104 0
PannerNode 75 75 0
PeriodicWave 33 33 0
ScriptProcessorNode 0 0 0
StereoPannerNode 102 102 0

Baseline: f8b57cd46198e0612871fa0f68b27fd7099eec53 · Candidate: ac7f62b4cd276b1488f06b1de5565d5e6a67e75a

Workflow run · this comment is updated on every push.

@mdydek mdydek added the fix Code changes specifically addressing and resolving a bug label Aug 20, 2026
Comment thread packages/react-native-audio-api/src/core/AudioBufferSourceNode.ts Outdated
Comment thread packages/react-native-audio-api/tests/audio-event-subscriptions.test.ts Outdated
Comment thread packages/react-native-audio-api/src/core/BaseAudioContext.ts Outdated
@mdydek
mdydek force-pushed the fix/unpin-wpt-windows branch from dd06a52 to 5bca10f Compare August 21, 2026 13:48
@closetcaiman closetcaiman added the test Adding missing test suites or modifying existing unit/E2E tests label Aug 21, 2026
Comment on lines +59 to 66
/**
* The spec's [[control thread state]]: written synchronously the moment an
* operation is accepted, so the NEXT call validates against what has already
* been requested (e.g. close() right after resume() must see 'running').
* Never exposed — the `state` attribute reports acknowledged reality
* instead.
*/
protected _state: ContextState = 'suspended';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field actually needed if we moved the getter to the host object?

this._state = 'running';
return (this.context as IAudioContext).resume();
this.setControlState('running');
await (this.context as IAudioContext).resume();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the promise rejects the [[control state]] stays running.

Comment on lines -73 to 77
// The driver also starts implicitly, from the first
// `AudioScheduledSourceNode::start()`. Publish RUNNING here so the visible state
// Publish RUNNING here so the visible state
// never reports SUSPENDED while the graph is actually rendering; `resume()`
// reaches the same state through its promise task.
// reaches the same state through its promise task (its dispatch then
// dedupes against this one).
setState(ContextState::RUNNING);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it does not. It only flips the atomic state flag. (?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Code changes specifically addressing and resolving a bug test Adding missing test suites or modifying existing unit/E2E tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants