Bug 113121 – AudioNode should be an EventTarget

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Russell McClellan]

Recently the w3 web audio group decided to make AudioNode an EventTarget

https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#AudioNode-section

In the discussion for https://bugs.webkit.org/show_bug.cgi?id=112521, it was decided to convert AudioNode into an EventTarget before we'd fix that issue, as all AudioNode wrappers shouldn't be garbage collected if they have active audio connections if events can be triggered from the C++ side (as one would expect).

[Russell McClellan]

Created attachment 194677 [details]
Patch

[Russell McClellan]

There was some discussion here [1] about whether or not ActiveDOMObject was the right way to go on this.  The other obvious option would be a custom binding.  I don't feel strongly either way, but my first patch rejected my first patch for [2] because I used a custom binding.

It is true there's precedence for using ActiveDOMObject only for its GC benefits (e.g. DatabaseContext, AudioContext, MediaSource), so I don't know if this is the place to change strategies.  Maybe at some point it would be good to split out the GC meaning of ActiveDOMObject from the suspendable meaning.

The only perhaps questionable part of this patch is calling suspendIfNeeded directly from the AudioNode constructor.  This fixes some asserts and I don't think this is a big deal, and we can change it if we ever need suspend behavior from AudioNodes.  It seems silly to add a suspendIfNeeded call to every create function in the hierarchy, when the call doesn't do anything.

[1]: https://bugs.webkit.org/show_bug.cgi?id=112858
[2]: https://bugs.webkit.org/show_bug.cgi?id=112521

[Chris Rogers]

Dominic, could you please have a look regarding the EventTarget changes?

[Andreas Kling]

Looks like this was merged back from Blink in bug 116871.