Bug 65276 – Add audionode.html and gain.html web audio layout tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add audionode.html and gain.html web audio layout tests

[Chris Rogers]

Created attachment 102170 [details]
Patch

[Tony Chang]

Comment on attachment 102170 [details]
Patch

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

r- for the missing file.

> LayoutTests/webaudio/audionode.html:5
> +<!--
> +Performs basic tests on the AudioNode API.
> +-->

Nit: I would remove this since it's in the description() below.

> LayoutTests/webaudio/gain.html:12
> +<script type="text/javascript" src="resources/buffer-loader.js"></script>

Did you mean to include this file in the change?

> LayoutTests/webaudio/gain.html:22
> +var bufferDuration = 0.125;

Nit: I like to include the units in my variable names, e.g., bufferDurationSeconds.

> LayoutTests/webaudio/gain.html:63
> +    context = new webkitAudioContext(2, sampleRate * lengthInSeconds, sampleRate);

Can you get someone familiar with the audio API to review this code?  I'm happy to forward on their LGTM, but I don't really know the API.

[Chris Rogers]

Created attachment 102190 [details]
Patch

[Chris Rogers]

Comment on attachment 102170 [details]
Patch

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

>> LayoutTests/webaudio/audionode.html:5
>> +-->
> 
> Nit: I would remove this since it's in the description() below.

FIXED

>> LayoutTests/webaudio/gain.html:12
>> +<script type="text/javascript" src="resources/buffer-loader.js"></script>
> 
> Did you mean to include this file in the change?

Added buffer-loader.js

>> LayoutTests/webaudio/gain.html:22
>> +var bufferDuration = 0.125;
> 
> Nit: I like to include the units in my variable names, e.g., bufferDurationSeconds.

FIXED

>> LayoutTests/webaudio/gain.html:63
>> +    context = new webkitAudioContext(2, sampleRate * lengthInSeconds, sampleRate);
> 
> Can you get someone familiar with the audio API to review this code?  I'm happy to forward on their LGTM, but I don't really know the API.

Sure, no problem, adding kbr

[Tony Chang]

Comment on attachment 102190 [details]
Patch

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

> LayoutTests/webaudio/gain.html:12
> +<script type="text/javascript" src="resources/buffer-loader.js"></script>

It doesn't look like this test uses BufferLoader.  Can we remove it from this change?

[Chris Rogers]

Created attachment 102542 [details]
Patch

[Chris Rogers]

Comment on attachment 102190 [details]
Patch

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

>> LayoutTests/webaudio/gain.html:12
>> +<script type="text/javascript" src="resources/buffer-loader.js"></script>
> 
> It doesn't look like this test uses BufferLoader.  Can we remove it from this change?

FIXED

[Tony Chang]

LGTM, maybe kbr can do the r+ after reviewing the test contents.

[Kenneth Russell]

Comment on attachment 102542 [details]
Patch

Looks good, in particular after our offline conversation indicating that these tests are already disabled for platforms not supporting the Web Audio API yet.

[Chris Rogers]

Committed r92144: <http://trac.webkit.org/changeset/92144>