Bug 150925 - createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for invalid numberOfInputs value
Summary: createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://www.w3.org/TR/2013/WD-webaudio...
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-05 03:14 PST by Hyemi Shin
Modified: 2016-02-04 10:03 PST (History)
4 users (show)

See Also:


Attachments
Patch (7.55 KB, patch)
2015-11-05 03:29 PST, Hyemi Shin
no flags Details | Formatted Diff | Diff
Patch (7.55 KB, patch)
2015-11-05 03:39 PST, Hyemi Shin
no flags Details | Formatted Diff | Diff
Patch (11.51 KB, patch)
2016-02-03 07:01 PST, Hyemi Shin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (751.63 KB, application/zip)
2016-02-03 07:48 PST, Build Bot
no flags Details
Patch (11.50 KB, patch)
2016-02-03 07:49 PST, Hyemi Shin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hyemi Shin 2015-11-05 03:14:32 PST
From http://www.w3.org/TR/2013/WD-webaudio-20131010,
exception is specified as INDEX_SIZE_ERROR for invalid numberOfInputs value.
(It was just 'An exception' in previous version.)
Comment 1 Hyemi Shin 2015-11-05 03:29:32 PST
Created attachment 264848 [details]
Patch
Comment 2 Hyemi Shin 2015-11-05 03:39:14 PST
Created attachment 264849 [details]
Patch
Comment 3 Michael Catanzaro 2016-01-27 21:02:57 PST
Comment on attachment 264849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264849&action=review

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.

> Source/WebCore/Modules/webaudio/AudioContext.cpp:611
> +        ec = INDEX_SIZE_ERR;

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.

> LayoutTests/webaudio/audiochannelmerger-basic-expected.txt:6
> +PASS IndexSizeError was thrown for numberOfInputs > 32.

Ah, I see 32 is valid; good catch. I would mention this fix separately in the changelog, since it is noteworthy.
Comment 4 Hyemi Shin 2016-02-03 07:01:50 PST
Created attachment 270576 [details]
Patch
Comment 5 Hyemi Shin 2016-02-03 07:07:19 PST
I added the fix for createPeriodicWave also.

Because I'm not good at english that much, my change-log could be hard to understand or not clear. Feel free to point out wrong sentence which needs to be changed.
Comment 6 Build Bot 2016-02-03 07:48:46 PST
Comment on attachment 270576 [details]
Patch

Attachment 270576 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/777381

New failing tests:
webaudio/audiochannelsplitter.html
Comment 7 Build Bot 2016-02-03 07:48:49 PST
Created attachment 270577 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Hyemi Shin 2016-02-03 07:49:33 PST
Created attachment 270578 [details]
Patch
Comment 9 Hyemi Shin 2016-02-04 03:57:52 PST
cq?
Comment 10 WebKit Commit Bot 2016-02-04 10:03:39 PST
Comment on attachment 270578 [details]
Patch

Clearing flags on attachment: 270578

Committed r196130: <http://trac.webkit.org/changeset/196130>
Comment 11 WebKit Commit Bot 2016-02-04 10:03:43 PST
All reviewed patches have been landed.  Closing bug.