[webkit-reviews] review granted: [Bug 75126] Add normalize attribute to ConvolverNode to disable normalization. : [Attachment 121182] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Raymond Toy <rtoy at chromium.org>'s
request for review:
Bug 75126: Add normalize attribute to ConvolverNode to disable normalization.
https://bugs.webkit.org/show_bug.cgi?id=75126

Attachment 121182: Patch
https://bugs.webkit.org/attachment.cgi?id=121182&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=121182&action=review


The code and test look good overall, but I think the test should undergo one
more revision. Feel free to upload another patch. r=me

> LayoutTests/webaudio/convolution-mono-mono.html:19
> +// produce a triangular pulse.  We verify the result is correct we

We verifiy -> To verify

> LayoutTests/webaudio/resources/convolution-testing.js:152
> +	   }

It may be worth factoring all of the code above this point into a separate
function which returns a boolean indicating its success. That way you can use
early returns to abort the test if any step goes wrong rather than setting the
"success" variable in several places throughout the test (and executing code
that's going to fail anyway if earlier portions of the test failed).