WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
150925
createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for invalid numberOfInputs value
https://bugs.webkit.org/show_bug.cgi?id=150925
Summary
createChannelMerger and createChannelSplitter should throw INDEX_SIZE_ERR for...
Hyemi Shin
Reported
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.)
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
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Hyemi Shin
Comment 1
2015-11-05 03:29:32 PST
Created
attachment 264848
[details]
Patch
Hyemi Shin
Comment 2
2015-11-05 03:39:14 PST
Created
attachment 264849
[details]
Patch
Michael Catanzaro
Comment 3
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.
Hyemi Shin
Comment 4
2016-02-03 07:01:50 PST
Created
attachment 270576
[details]
Patch
Hyemi Shin
Comment 5
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.
Build Bot
Comment 6
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
Build Bot
Comment 7
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
Hyemi Shin
Comment 8
2016-02-03 07:49:33 PST
Created
attachment 270578
[details]
Patch
Hyemi Shin
Comment 9
2016-02-04 03:57:52 PST
cq?
WebKit Commit Bot
Comment 10
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
>
WebKit Commit Bot
Comment 11
2016-02-04 10:03:43 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug