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

[bugzilla-daemon at webkit.org]

Raymond Toy <rtoy at chromium.org> has asked  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 Raymond Toy <rtoy at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121182&action=review


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

Fixed.

>>> 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).
> 
> Putting these in a separate function is a good idea.	I will do that. 
However, I think the only test worth exiting early is the initial latency test.
 If that's wrong, everything after will be wrong.  The other tests  have
experimentally determined thresholds, so I think it's worth continuing since
it's not much more work.  Failing the triangle doesn't imply the tail will
fail, and vice versa.

Each test is now in its own function.