1466500 - WebAudio context resumes unconditionally

Status: RESOLVED INVALID

(Core :: Web Audio, defect, P2)

Description

[Kirjah Salys]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20100101

Steps to reproduce:

I found several webpages that were using audioContext.resume() on every webpage click to work around Chrome's auto-play audio block.


Actual results:

WebAudio's context resumed unconditionally. Websites took so much time resuming the audio context that firefox lost an overwhelming majority of click events.


Expected results:

WebAudio's context, as specified in https://webaudio.github.io/web-audio-api/#dom-baseaudiocontext-resume must abort further action if the context is already running. According to Firefox source code, it is unconditionally resumed.

Comment 1

[Andreas Pehrson [:pehrsons]]

Thanks for filing! I don't think we had such abuse in mind when we implemented the api. It sounds like a potential issue indeed.

Paul, can you check this when you have time?

Kirjah, do you have an example of such a website so we have something to confirm a fix with?

Comment 2

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b17e549877080af6f3efd580630f7b4ce9677c6

Looks green on try. I had a to adjust a test as well, that was relying on the old behaviour.

I think this can make some code simpler. I had a look, and Chrome implements this more or less correctly (they return early). More or less, because they don't actually wait for the audio to be flowing before resolving `resume`, but I _think_ with this patch we should be more compatible.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1466500 - Update AudioContext.{suspend, resume} to match the spec.

Review commit: https://reviewboard.mozilla.org/r/250676/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250676/

Comment 4

[Paul Adenot (:padenot)]

Link to the standard: https://webaudio.github.io/web-audio-api/#dom-baseaudiocontext-resume (similar for suspend).

Comment 5

[Paul Adenot (:padenot)]

I think it should be possible to write a test for this, to make sure that the thenable for, for example, a `resume()` Promise is called exactly on the next microtask checkpoint if the context is already running, but we need to be able to reliably send an normal event loop task, and make sure the event queue is empty prior to sending the event and resolving the promise. I'll think about it.

Comment 6

[Karl Tomlinson (:karlt)]

I filed an issue re the specified behavior: https://github.com/WebAudio/web-audio-api/issues/1674

We don't understand why very slightly delayed promise resolution would cause loss of click events.

We need more information about why this is causing problems before changing the behavior.

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8984905 [details]
Bug 1466500 - Update AudioContext.{suspend, resume} to match the spec.

https://reviewboard.mozilla.org/r/250676/#review257530

Returning early would leave the rendering suspended if the context had recently
been suspended, which would be inconsistent with Chrome's behaviour.

Perhaps it would be possible to resolve immediately in some situations, but
https://bugzilla.mozilla.org/show_bug.cgi?id=1269741#c6 seems to indicate this
would be complicated.

Comment 8

[Paul Adenot (:padenot)]

The spec has been changed to unconditionally try to resume, this is now invalid. Please file another, more focused bug if there still is a problem with Gecko's implementation.