[Webkit-unassigned] [Bug 80681] [JSC] AudioDestinationNode event notification error on AudioContext

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=80681


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #135325|review?                     |review-
               Flag|                            |




--- Comment #5 from Geoffrey Garen <ggaren at apple.com>  2012-04-03 09:39:54 PST ---
(From update of attachment 135325)
View in context: https://bugs.webkit.org/attachment.cgi?id=135325&action=review

> Source/WebCore/Modules/webaudio/AudioContext.h:84
> +        bool running(destination() ? destination()->running() : false);

WebKit style is to use "=" instead of constructor syntax for "running" here. But this probably won't matter in the end -- see below.

> Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:88
>          m_startedRendering = true;
> +        m_running = true;
>          ref(); // See corresponding deref() call in notifyCompleteDispatch().

I think "m_running = false" should go wherever we call "deref()" and destroy our audio thread, rather than in notifyComplete(). I'm not familiar with this code, but from the outside it seems like some chains of function calls could cancel the audio without calling notifyComplete(), which would leave a zombie audio node.

Even better: ActiveDOMObject packages up these concepts for you. Instead of manually calling ref() and maintaining a separate boolean, you can just call "setPendingActivity". That will ref() your object and cause hasPendingActivity() to start returning true. No need to keep an m_running variable, no need to override hasPendingActivity(). Then, call "unsetPendingActivity" wherever you stop playing and cancel the audio thread.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.