Closed
Bug 1341549
Opened 8 years ago
Closed 8 years ago
Label runnables under dom/media/webaudio
Categories
(Core :: Web Audio, defect, P1)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jwwang, Assigned: padenot)
References
Details
(Whiteboard: [QDL][BACKLOG][MEDIA])
Attachments
(1 file)
See https://wiki.mozilla.org/Quantum/DOM for the motivation.
Updated•8 years ago
|
Rank: 19
Priority: -- → P1
Reporter | ||
Comment 1•8 years ago
|
||
Hi Paul,
Can you find someone to take this bug? Thanks!
Flags: needinfo?(padenot)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → padenot
Flags: needinfo?(padenot)
Updated•8 years ago
|
Whiteboard: [QDL][BACKLOG][MEDIA]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•8 years ago
|
||
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•8 years ago
|
||
mozreview-review |
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.
Attachment #8854889 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a436c8af1ba2
Label runnables in dom/media/webaudio/ r=billm
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•