Bug 113121

Summary: AudioNode should be an EventTarget
Product: WebKit Reporter: Russell McClellan <russell.mcclellan>
Component: Web AudioAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, crogers, dominicc, ehsan, eric.carlson, esprehn+autocc, feature-media-reviews, haraken, jer.noble, kling, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 113274    
Attachments:
Description Flags
Patch none

Russell McClellan
Reported 2013-03-22 17:33:31 PDT
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).
Attachments
Patch (16.08 KB, patch)
2013-03-22 18:22 PDT, Russell McClellan
no flags
Russell McClellan
Comment 1 2013-03-22 18:22:49 PDT
Russell McClellan
Comment 2 2013-03-22 18:29:23 PDT
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
Comment 3 2013-04-01 12:01:28 PDT
Dominic, could you please have a look regarding the EventTarget changes?
Andreas Kling
Comment 4 2015-09-07 13:24:01 PDT
Looks like this was merged back from Blink in bug 116871.
Note You need to log in before you can comment on or make changes to this bug.