[Webkit-unassigned] [Bug 76685] Fix ASSERT fail within AudioBus::processWithGainFrom()

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 30 20:44:35 PST 2012


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


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #124633|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #14 from Daniel Bates <dbates at webkit.org>  2012-01-30 20:44:35 PST ---
(From update of attachment 124633)
View in context: https://bugs.webkit.org/attachment.cgi?id=124633&action=review

Thanks Raymond for updating the patch and writing a test. This patch looks good. I have some very minor spelling/grammar nits.

> LayoutTests/webaudio/audionode-connect-order.html:14
> +description("Test AudioNode connection order for checking ASSERT.");

Maybe the following text is more descriptive: "This tests that we don't trigger an assertion failure due to the AudioNode connection order"?

> LayoutTests/webaudio/audionode-connect-order.html:27
> +        data[i] = Math.sin(frequency * 2.0*Math.PI * i / sampleRate);

Nit: "2.0*Math.PI" => "2 * Math.PI"

(Notice the space on either side of the multiplication operator '*'; also, it's unnecessary to write 2 as 2.0 since all JavaScript numbers are represented using double precision floating point format)

> LayoutTests/webaudio/audionode-connect-order.html:54
> +    // We do this becacuse we try to catch the ASSERT which might be fired due to audionode connection order,

I think you meant to say "try to trigger the ASSERT"? I tend to associate the word "catch" with try/catch blocks, which is a construct that provides a way to recover from an exception by transferring control to the catch clause. In contrast, we don't usually have recovery code associated with an ASSERT(). And, in a debug build, an assertion failure causes a crash. Therefore, I suggest using the word "trigger" as in "trigger the ASSERT".

becacuse => because

And

audionode => AudioNode

> LayoutTests/webaudio/audionode-connect-order.html:55
> +    // especiallly when gain node and delay node is involved e.g. https://bugs.webkit.org/show_bug.cgi?id=76685.

especiallly => especially

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



More information about the webkit-unassigned mailing list