[Webkit-unassigned] [Bug 48875] Add JavaScriptAudioNode files

[bugzilla-daemon at webkit.org]

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


Kenneth Russell <kbr at google.com> changed:

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




--- Comment #3 from Kenneth Russell <kbr at google.com>  2010-11-08 16:41:01 PST ---
(From update of attachment 72747)
View in context: https://bugs.webkit.org/attachment.cgi?id=72747&action=review

This looks good overall; I went through the bounds checking assertions carefully and they all look correct. Because of the potential deletion issue before the callback is called, I'm marking it r-.

> WebCore/webaudio/JavaScriptAudioNode.cpp:101
> +    m_outputBuffers.append(AudioBuffer::create(2, bufferSize(), sampleRate));

This could be a loop to reduce duplicated code.

> WebCore/webaudio/JavaScriptAudioNode.cpp:200
> +            callOnMainThread(fireProcessEventDispatch, this);

Some sort of guard is needed to ensure that this node isn't deallocated by the time fireProcessEventDispatch is called. Perhaps you could increment one of the node's reference counts here and decrement it in the callback.

> WebCore/webaudio/JavaScriptAudioNode.h:57
> +        return adoptRef(new JavaScriptAudioNode(context, sampleRate, bufferSize, numberOfInputs, numberOfOutputs));

Unless there's a strong reason to inline this, move the body to the .cpp.

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