Closed Bug 1015783 Opened 6 years ago Closed 5 years ago

Add a devtools API for Web Audio

Categories

(Core :: Web Audio, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

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: nobody → ehsan
Blocks: 980506
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 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+
Yeah, I thought about that, but I think uint32_t should be enough for all practical purposes...
Attachment #8428429 - Flags: review?(paul) → review+
Jordan, can you please post back here once you've confirmed that this patch does what you want?  Thanks!
Flags: needinfo?(jsantell)
We decided to land what I have here.

https://hg.mozilla.org/integration/mozilla-inbound/rev/feb56fc5dc01
Flags: needinfo?(jsantell)
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)
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)
(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!)
> dunno :)  I'm calling SpecialPowers.DOMWindowUtils.cycleCollect.
Try .garbageCollect instead of cycle collect.  That will both GC and CC.
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.)
Thanks a lot!  Let's see what the try server thinks about this: https://tbpl.mozilla.org/?tree=Try&rev=6da93bf0ceee
The try server is loving this!

https://hg.mozilla.org/integration/mozilla-inbound/rev/50f9f62bebb2
Flags: needinfo?(bugs)
https://hg.mozilla.org/mozilla-central/rev/50f9f62bebb2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.