1341549 - Label runnables under dom/media/webaudio

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[JW Wang [:jwwang] [:jw_wang]]

See https://wiki.mozilla.org/Quantum/DOM for the motivation.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Hi Paul,
Can you find someone to take this bug? Thanks!

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1341549 - Label runnables in dom/media/webaudio/

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

Comment 3

[Paul Adenot (:padenot)]

Bill, I didn't know who to request this from, so I picked you, feel free to delegate. I'm not too familiar with all this, but what I did seem to work.

One question I have, though, is that sometimes, in Web Audio, we want to dispatch something to the main thread, but at this time, the global has gone away already (it's highly asynchronous, and depends in system API that take some time, and all, and people can close time at any time).

I don't check that the global is valid when we're in a stack frame that is called _synchronously_ from js or from another main thread runnable, my assumption being that the global cannot go away during the execution of the current runnable. Does that sound correct ?

For example, in AudioContext::OnStateChanged, we're here from deep down inside the MSG, so we don't have any guarantee that the document hasn't gone away, so I nullcheck, but in AsyncDecodeWebAudio, we have js code up the stack so I think it's safe.

Also, what I'm not sure about is the change from `NS_DispatchToMainThread(this);`, is that a correct way to rewrite it ?

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8854889 [details]
Bug 1341549 - Label runnables in dom/media/webaudio/

https://reviewboard.mozilla.org/r/126838/#review129662

Thanks Paul. Everything in the patch looks good. I just have two comments:
- I think it might be a bit cleaner to add a Dispatch method to AudioContext. That would reduce the amount of boilerplate that we have here for getting the AbstractMainThread and calling Dispatch on it.
- You can use do_AddRef instead of assigning to a RefPtr and then forget()ing it. This won't always fit on one line, so use at your discretion. But it helps sometimes.

Comment 5

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c78ccd43636caabe56fd8c86748e6e9f3f80b3f

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a436c8af1ba2
Label runnables in dom/media/webaudio/ r=billm

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a436c8af1ba2