Closed Bug 1341549 Opened 3 years ago Closed 3 years ago

Label runnables under dom/media/webaudio


(Core :: Web Audio, defect, P1)




Tracking Status
firefox55 --- fixed


(Reporter: jwwang, Assigned: padenot)


(Blocks 1 open bug)


(Whiteboard: [QDL][BACKLOG][MEDIA])


(1 file)

See for the motivation.
Blocks: 1341539
Rank: 19
Priority: -- → P1
Hi Paul,
Can you find someone to take this bug? Thanks!
Flags: needinfo?(padenot)
Assignee: nobody → padenot
Flags: needinfo?(padenot)
Whiteboard: [QDL][BACKLOG][MEDIA]
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 on attachment 8854889 [details]
Bug 1341549 - Label runnables in dom/media/webaudio/

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.
Attachment #8854889 - Flags: review?(wmccloskey) → review+
Pushed by
Label runnables in dom/media/webaudio/ r=billm
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.