ConvolverNode.buffer sample rate must match AudioContext rate

ConvolverNode.buffer sample rate must match AudioContext rate

Throw an exception as required when the sample rate of the convolver
node buffer does not match the sample rate of the audio context.

Added test for this case.

BUG=352776
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169501

[Raymond Toy, 2014-03-14 21:34:17]

PTAL

[Ken Russell (switch to Gerrit), 2014-03-14 23:54:11]

The code looks fine, but there's a bug in the tests.

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
File LayoutTests/webaudio/dom-exceptions.html (right):

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:172: oc = new
webkitOfflineAudioContext(1, 44100, 44100);
Possible to get this line into the debug output?

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:173: conv = context.createConvolver();
This isn't referencing "oc". Is it supposed to?

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:174: shouldThrow("conv.buffer =
context.createBuffer(1, 100, 22050)");
Same here.

[Raymond Toy, 2014-03-17 18:09:19]

Thanks for catching these stupid mistakes.

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
File LayoutTests/webaudio/dom-exceptions.html (right):

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:172: oc = new
webkitOfflineAudioContext(1, 44100, 44100);
On 2014/03/14 23:54:12, Ken Russell wrote:
> Possible to get this line into the debug output?

Added as a test so that the expected output contains this.

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:173: conv = context.createConvolver();
On 2014/03/14 23:54:12, Ken Russell wrote:
> This isn't referencing "oc". Is it supposed to?

Yes, it should use oc. Fixed.

https://codereview.chromium.org/200543005/diff/1/LayoutTests/webaudio/dom-exc...
LayoutTests/webaudio/dom-exceptions.html:174: shouldThrow("conv.buffer =
context.createBuffer(1, 100, 22050)");
On 2014/03/14 23:54:12, Ken Russell wrote:
> Same here.

Done.

[Ken Russell (switch to Gerrit), 2014-03-17 18:21:38]

LGTM if it passes on the bots. Looks like there are some severe problems with
the try servers right now.

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
File LayoutTests/webaudio/dom-exceptions.html (right):

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
LayoutTests/webaudio/dom-exceptions.html:173: conv = oc.createConvolver();
Maybe this should be wrapped in a shouldNotThrow too for clarity.

[Raymond Toy, 2014-03-17 19:42:13]

Thanks for the review and the note about the trybots.

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
File LayoutTests/webaudio/dom-exceptions.html (right):

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
LayoutTests/webaudio/dom-exceptions.html:173: conv = oc.createConvolver();
On 2014/03/17 18:21:38, Ken Russell wrote:
> Maybe this should be wrapped in a shouldNotThrow too for clarity.

Done.

Do you think this should be done in general in this file? (Not for this CL, of
course.)

[Ken Russell (switch to Gerrit), 2014-03-17 21:35:30]

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
File LayoutTests/webaudio/dom-exceptions.html (right):

https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
LayoutTests/webaudio/dom-exceptions.html:173: conv = oc.createConvolver();
On 2014/03/17 19:42:13, Raymond Toy wrote:
> On 2014/03/17 18:21:38, Ken Russell wrote:
> > Maybe this should be wrapped in a shouldNotThrow too for clarity.
> 
> Done.
> 
> Do you think this should be done in general in this file? (Not for this CL, of
> course.)

If you think it will improve clarity of the test, then yes.

[Raymond Toy, 2014-03-17 23:49:02]

On 2014/03/17 21:35:30, Ken Russell wrote:
>
https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
> File LayoutTests/webaudio/dom-exceptions.html (right):
> 
>
https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
> LayoutTests/webaudio/dom-exceptions.html:173: conv = oc.createConvolver();
> On 2014/03/17 19:42:13, Raymond Toy wrote:
> > On 2014/03/17 18:21:38, Ken Russell wrote:
> > > Maybe this should be wrapped in a shouldNotThrow too for clarity.
> > 
> > Done.
> > 
> > Do you think this should be done in general in this file? (Not for this CL,
of
> > course.)
> 
> If you think it will improve clarity of the test, then yes.

Thanks.

In addition, the mac_blink_rel failure is real; I can reproduce it on my mac.
Curiously, running the test in a full browser with this CL applied passes.

[Raymond Toy, 2014-03-18 23:13:27]

On 2014/03/17 23:49:02, Raymond Toy wrote:
> On 2014/03/17 21:35:30, Ken Russell wrote:
> >
>
https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
> > File LayoutTests/webaudio/dom-exceptions.html (right):
> > 
> >
>
https://codereview.chromium.org/200543005/diff/10001/LayoutTests/webaudio/dom...
> > LayoutTests/webaudio/dom-exceptions.html:173: conv = oc.createConvolver();
> > On 2014/03/17 19:42:13, Raymond Toy wrote:
> > > On 2014/03/17 18:21:38, Ken Russell wrote:
> > > > Maybe this should be wrapped in a shouldNotThrow too for clarity.
> > > 
> > > Done.
> > > 
> > > Do you think this should be done in general in this file? (Not for this
CL,
> of
> > > course.)
> > 
> > If you think it will improve clarity of the test, then yes.
> 
> Thanks.
> 
> In addition, the mac_blink_rel failure is real; I can reproduce it on my mac.
> Curiously, running the test in a full browser with this CL applied passes.

Fixed.

[Raymond Toy, 2014-03-18 23:14:27]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2014-03-18 23:15:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtoy@google.com/200543005/50001

[commit-bot: I haz the power, 2014-03-18 23:21:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-18 23:21:10]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Raymond Toy, 2014-03-18 23:26:03]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2014-03-18 23:26:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtoy@google.com/200543005/50001

[commit-bot: I haz the power, 2014-03-19 00:04:12]

Change committed as 169501

[tkent, 2015-04-08 04:52:35]

tkent@chromium.org changed reviewers:
+ tkent@chromium.org

[tkent, 2015-04-08 04:52:35]

https://codereview.chromium.org/200543005/diff/50001/Source/modules/webaudio/...
File Source/modules/webaudio/ConvolverNode.cpp (right):

https://codereview.chromium.org/200543005/diff/50001/Source/modules/webaudio/...
Source/modules/webaudio/ConvolverNode.cpp:125: }
Should we do |return| here?

[Raymond Toy, 2015-04-08 15:49:44]

https://codereview.chromium.org/200543005/diff/50001/Source/modules/webaudio/...
File Source/modules/webaudio/ConvolverNode.cpp (right):

https://codereview.chromium.org/200543005/diff/50001/Source/modules/webaudio/...
Source/modules/webaudio/ConvolverNode.cpp:125: }
On 2015/04/08 04:52:35, tkent wrote:
> Should we do |return| here?

Yes!

I'll fix this right away.