<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for invalid numberOfInputs value"
href="https://bugs.webkit.org/show_bug.cgi?id=150925#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for invalid numberOfInputs value"
href="https://bugs.webkit.org/show_bug.cgi?id=150925">bug 150925</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=264849&action=diff" name="attach_264849" title="Patch">attachment 264849</a> <a href="attachment.cgi?id=264849&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=264849&action=review">https://bugs.webkit.org/attachment.cgi?id=264849&action=review</a>
Thanks for this patch, Hyemi, and sorry for the delay in getting a review. It looks good, but note that AudioContext::createPeriodicWave has the same problem. r=me if you could fix that too.
<span class="quote">> Source/WebCore/Modules/webaudio/AudioContext.cpp:611
> + ec = INDEX_SIZE_ERR;</span >
Hm, I see that this change is correct, because ChannelSplitterNode::create (and also ChannelMergerNode::create) will only ever fail if the numberOfInputs parameter is invalid. I'm a bit uncomfortable with relying on a null return value from create to signal that, since it's not at all clear from reading AudioContext.cpp why the ExceptionCode should be INDEX_SIZE_ERROR. I was going to ask you to pass the ExceptionCode down into the create function, but I see this pattern is established throughout AudioContext.cpp, so the code you have here is best.
<span class="quote">> LayoutTests/webaudio/audiochannelmerger-basic-expected.txt:6
> +PASS IndexSizeError was thrown for numberOfInputs > 32.</span >
Ah, I see 32 is valid; good catch. I would mention this fix separately in the changelog, since it is noteworthy.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>