WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
216972
Calling suspend() on audio context with no node does not prevent auto transition to running when first node is created
https://bugs.webkit.org/show_bug.cgi?id=216972
Summary
Calling suspend() on audio context with no node does not prevent auto transit...
Francois Daoust
Reported
2020-09-25 06:42:20 PDT
Created
attachment 409686
[details]
Test HTML page to reproduce the bug If one calls suspend() on an audio context before any node has been created, the call succeeds (and state right after is indeed "suspended") but the intent is ignored: the audio context still switches to "running" automatically when a first node gets created. This is counter-intuitive. Steps to Reproduce: Run the following code in response to some user gesture. See attached file for an HTML page that contains the test. const AudioContext = window.AudioContext || window.webkitAudioContext; const audioContext = new AudioContext(); await audioContext.suspend(); const node = audioContext.createOscillator(); Actual state after creation of the oscillator node is "running". Since the code explicitly called suspend(), I would rather expect the audio context to remain in a "suspended" state. Working around this behavior is easy, one can either: 1. call suspend() after creating the first node 2. call resume() before suspend() leading to the following (weird) workaround: const AudioContext = window.AudioContext || window.webkitAudioContext; const audioContext = new AudioContext(); await audioContext.resume(); await audioContext.suspend(); const node = audioContext.createOscillator(); The attached HTML page renders 3 buttons. First button illustrates the bug. Second and third button illustrate the workarounds. I note that this may warrant an update to the Web Audio API specification, as it does not explicitly state that the initial transition to running (delayed in Webkit as allowed by the spec) needs to take the [[suspended by user]] flag into account. It seems reasonable to assume that when the developer calls suspend(), they actually mean it, though!
Attachments
Test HTML page to reproduce the bug
(4.72 KB, text/html)
2020-09-25 06:42 PDT
,
Francois Daoust
no flags
Details
Patch
(7.67 KB, patch)
2020-10-02 13:39 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(8.21 KB, patch)
2020-10-02 14:16 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-10-02 06:43:15 PDT
<
rdar://problem/69878834
>
Chris Dumez
Comment 2
2020-10-02 08:12:35 PDT
Seems reasonable indeed. I will look into it shortly. Thank you for the bug report.
Chris Dumez
Comment 3
2020-10-02 13:39:39 PDT
Created
attachment 410359
[details]
Patch
Darin Adler
Comment 4
2020-10-02 13:49:16 PDT
Comment on
attachment 410359
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=410359&action=review
> Source/WebCore/Modules/webaudio/BaseAudioContext.h:467 > + bool m_wasSuspendedByUser { false };
What is "user" here? Code in the webpage? Not sure the word "user" is quite right for that, unless it’s guarded by "in response to user event" or that sort of thing.
Chris Dumez
Comment 5
2020-10-02 13:57:26 PDT
(In reply to Darin Adler from
comment #4
)
> Comment on
attachment 410359
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=410359&action=review
> > > Source/WebCore/Modules/webaudio/BaseAudioContext.h:467 > > + bool m_wasSuspendedByUser { false }; > > What is "user" here? Code in the webpage? Not sure the word "user" is quite > right for that, unless it’s guarded by "in response to user event" or that > sort of thing.
I can rename to m_wasSuspendedByScript for clarity. The reason I had used that name was to match the wording in the spec.
Darin Adler
Comment 6
2020-10-02 14:02:24 PDT
(In reply to Chris Dumez from
comment #5
)
> The reason I had used > that name was to match the wording in the spec.
That seems like a reason to stick with that wording, but I must say that calling the webpage code "the user" is peculiar.
Chris Dumez
Comment 7
2020-10-02 14:13:59 PDT
(In reply to Darin Adler from
comment #6
)
> (In reply to Chris Dumez from
comment #5
) > > The reason I had used > > that name was to match the wording in the spec. > > That seems like a reason to stick with that wording, but I must say that > calling the webpage code "the user" is peculiar.
Yes, I agree the naming is peculiar. Thinking about this more, I think my preference would be to use a better name in our implementation. I can add a comment next to the data member explaining which flag in the specification it corresponds to.
Chris Dumez
Comment 8
2020-10-02 14:16:47 PDT
Created
attachment 410371
[details]
Patch
EWS
Comment 9
2020-10-02 18:26:18 PDT
Committed
r267911
: <
https://trac.webkit.org/changeset/267911
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 410371
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug