[webkit-reviews] review denied: [Bug 118405] Update Exception handling in WebAudio : [Attachment 206122] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Praveen Jadhav <praveen.j at samsung.com>'s
request for review:
Bug 118405: Update Exception handling in WebAudio
https://bugs.webkit.org/show_bug.cgi?id=118405

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=206122&action=review


>> Source/WebCore/ChangeLog:8
>> +	    AudioBufferSourceNode, OscillatorNode and AnalyserNode don't
propagate any exceptions specified in WebAudio spec.
> 
> Throwing new exceptions is risky business when it comes to backward
compatibility.

While I usually am a champion for this point, given how little WebAudio content
there is, I am not too worried here.

>> LayoutTests/webaudio/analyser-exception.html:34
>> +	shouldThrow("analyser.minDecibels = -20");
> 
> shouldThrow() can take a second argument with the exception name to validate
that as well. If you expect a specific exception, you probably want to add that
(although the spec does not seem to indicate exception names?)

I agree with Christophe, you should specify what exception you expect.	If the
spec isn't specifying it clearly, we should get it updated.