Closed
Bug 1015783
Opened 10 years ago
Closed 10 years ago
Add a devtools API for Web Audio
Categories
(Core :: Web Audio, defect)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file)
13.04 KB,
patch
|
padenot
:
review+
smaug
:
review+
|
Details | Diff | Splinter Review |
See bug 980506 for an extensive discussion about this. This patch adds three APIs to AudioNode in order for us to be able to build awesome devtools on top of it. * Weak reference API. This patch allows one to hold a weak reference to all AudioNode's using Components.utils.getWeakReference(). That way, the devtool's inspection code would not change the lifetime of AudioNodes. * AudioNode.id This is a chrome-only unique and monotonically incrementing ID for AudioNode objects. It is supposed to be used in order for the devtools to be able to identify a node without having to keep it alive. * webaudio-node-demise This is an observer notification that is called every time an AudioNode gets destroyed inside Gecko. The ID of the corresponding node is passed to this notification.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8428429 [details] [diff] [review] Add a devtools API for Web Audio Over to Paul for the actual review and to smaug for the WebIDL change.
Attachment #8428429 -
Flags: review?(paul)
Attachment #8428429 -
Flags: review?(bugs)
Comment 3•10 years ago
|
||
Comment on attachment 8428429 [details] [diff] [review] Add a devtools API for Web Audio I would probably use unsigned long long / uint64_t, but perhaps uint32_t is enough.
Attachment #8428429 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Yeah, I thought about that, but I think uint32_t should be enough for all practical purposes...
Updated•10 years ago
|
Attachment #8428429 -
Flags: review?(paul) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Jordan, can you please post back here once you've confirmed that this patch does what you want? Thanks!
Flags: needinfo?(jsantell)
Assignee | ||
Comment 6•10 years ago
|
||
We decided to land what I have here. https://hg.mozilla.org/integration/mozilla-inbound/rev/feb56fc5dc01
Flags: needinfo?(jsantell)
Assignee | ||
Comment 7•10 years ago
|
||
Backed out because the test fails intermittently: https://hg.mozilla.org/integration/mozilla-inbound/rev/cd2ef9d646ff https://tbpl.mozilla.org/php/getParsedLog.php?id=40992639&tree=Mozilla-Inbound I suspect that calling cycleCollect twice is not going to guarantee that we kill the cycle collected objects. Olli, Andrew: my test case here wants to cycle collect "enough times" to make sure that the object that I have which participates in the cycle is killed. Is there a robust way of doing that?
Flags: needinfo?(continuation)
Flags: needinfo?(bugs)
Comment 8•10 years ago
|
||
Are you GCing, too? That's important. In general, though, there's no robust way to do it, particularly if a window is involved in the cycle, as all sorts of random half-leaky junk can entrain the window. There's the schedulePreciseGC thing which might help a bit, as it waits until there's no JS on the stack, or something. Then you do a CC inside there.
Flags: needinfo?(continuation)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #8) > Are you GCing, too? That's important. dunno :) I'm calling SpecialPowers.DOMWindowUtils.cycleCollect. > In general, though, there's no > robust way to do it, particularly if a window is involved in the cycle, as > all sorts of random half-leaky junk can entrain the window. There is no window involved here. This is a cycle between an AudioContext and an AudioNode (they each hold a strong reference to the other). >There's the > schedulePreciseGC thing which might help a bit, as it waits until there's no > JS on the stack, or something. Then you do a CC inside there. Hmm, can you please clarify how I'd use that? (FWIW, if you just tell me there is no good way to test this robustly, I'd be happy to remove that part of the test!)
Comment 10•10 years ago
|
||
> dunno :) I'm calling SpecialPowers.DOMWindowUtils.cycleCollect.
Try .garbageCollect instead of cycle collect. That will both GC and CC.
Comment 11•10 years ago
|
||
Oops, hit "save changes" too soon. > Hmm, can you please clarify how I'd use that? Here's an example: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/tests/chrome/test_weakmaps.xul#244 > FWIW, if you just tell me there is no good way to test this robustly I don't think it is impossible. I wrote a number of tests that check that the CC breaks cycles involving weak maps, and they are more or less okay. (There used to be the occasional platform specific intermittent leaks, but I think those aren't a problem any more.)
Assignee | ||
Comment 12•10 years ago
|
||
Thanks a lot! Let's see what the try server thinks about this: https://tbpl.mozilla.org/?tree=Try&rev=6da93bf0ceee
Assignee | ||
Comment 13•10 years ago
|
||
The try server is loving this! https://hg.mozilla.org/integration/mozilla-inbound/rev/50f9f62bebb2
Flags: needinfo?(bugs)
Comment 14•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/50f9f62bebb2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•