[webkit-reviews] review granted: [Bug 197533] WebAudio Node JS wrappers should not be collected if events can be fired : [Attachment 369124] Allow GC for closed AudioContexts

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 197533: WebAudio Node JS wrappers should not be collected if events can be
fired
https://bugs.webkit.org/show_bug.cgi?id=197533

Attachment 369124: Allow GC for closed AudioContexts

https://bugs.webkit.org/attachment.cgi?id=369124&action=review




--- Comment #11 from Jer Noble <jer.noble at apple.com> ---
Comment on attachment 369124
  --> https://bugs.webkit.org/attachment.cgi?id=369124
Allow GC for closed AudioContexts

r=me, with nit:

AudioNode.idl already has a "GenerateIsReachable=Impl", which /should/ keep the
wrapper alive as long as the node itself is alive, so at the very least, your
patch should incorporate those existing isReachableFromOpaqueRoots(). But since
AudioNode is also an EventTarget, it is probably a better idea just to do a
custom isReachable against AudioNode itself, rather than different
implementations for three different subclasses of AudioNode. One implementation
seems like a better idea from a code maintenance standpoint, and the benefit of
having different "reason" information for each subclass doesn't seem worth it
in comparison.