Add min/max values for AudioParams

Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFSBwAJ

BUG=594594
TEST=audioparam-nominal-range.html

Committed: https://crrev.com/71df40e1bab3e8d1bb48f539af9ae9cbb03f9bba
Cr-Commit-Position: refs/heads/master@{#393861}

[Raymond Toy, 2016-03-18 22:54:24]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-18 22:54:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1803153002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1803153002/20001

[commit-bot: I haz the power, 2016-03-19 00:13:37]

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

[commit-bot: I haz the power, 2016-03-19 00:13:37]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Raymond Toy, 2016-03-30 16:25:47]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-30 16:26:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1803153002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1803153002/40001

[commit-bot: I haz the power, 2016-03-30 18:56:46]

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

[commit-bot: I haz the power, 2016-03-30 18:56:47]

Dry run: This issue passed the CQ dry run.

[Raymond Toy, 2016-04-26 18:15:41]

Description was changed from

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

BUG=594594
TEST=audioparam-nominal-range.html
==========

to

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

[Raymond Toy, 2016-05-03 16:52:53]

rtoy@chromium.org changed reviewers:
+ hongchan@chromium.org

[Raymond Toy, 2016-05-03 16:52:54]

PTAL

[Raymond Toy, 2016-05-03 16:54:17]

Description was changed from

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

to

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

[hongchan, 2016-05-03 20:30:12]

I raised the issue on min/max attribute names:
https://github.com/WebAudio/web-audio-api/issues/762

We can wait until this gets resolved.

[Raymond Toy, 2016-05-03 20:35:33]

On 2016/05/03 20:30:12, hoch wrote:
> I raised the issue on min/max attribute names:
> https://github.com/WebAudio/web-audio-api/issues/762
> 
> We can wait until this gets resolved.

You can still review everything though. It's just a minor name change. :-)

[hongchan, 2016-05-03 20:40:57]

On 2016/05/03 20:35:33, Raymond Toy wrote:
> On 2016/05/03 20:30:12, hoch wrote:
> > I raised the issue on min/max attribute names:
> > https://github.com/WebAudio/web-audio-api/issues/762
> > 
> > We can wait until this gets resolved.
> 
> You can still review everything though. It's just a minor name change. :-)

Yes, other than user-exposed attribute names, I think it looks good. The
internal code should use 'minValue' and 'maxValue' according to the IDL change.

[hongchan, 2016-05-06 16:54:37]

On 2016/05/03 20:40:57, hoch wrote:
> On 2016/05/03 20:35:33, Raymond Toy wrote:
> > On 2016/05/03 20:30:12, hoch wrote:
> > > I raised the issue on min/max attribute names:
> > > https://github.com/WebAudio/web-audio-api/issues/762
> > > 
> > > We can wait until this gets resolved.
> > 
> > You can still review everything though. It's just a minor name change. :-)
> 
> Yes, other than user-exposed attribute names, I think it looks good. The
> internal code should use 'minValue' and 'maxValue' according to the IDL
change.

According the change in the spec, we have to change the name to xxxValue.

https://github.com/WebAudio/web-audio-api/issues/762

[Raymond Toy, 2016-05-12 20:46:49]

On 2016/05/06 16:54:37, hoch wrote:
> On 2016/05/03 20:40:57, hoch wrote:
> > On 2016/05/03 20:35:33, Raymond Toy wrote:
> > > On 2016/05/03 20:30:12, hoch wrote:
> > > > I raised the issue on min/max attribute names:
> > > > https://github.com/WebAudio/web-audio-api/issues/762
> > > > 
> > > > We can wait until this gets resolved.
> > > 
> > > You can still review everything though. It's just a minor name change. :-)
> > 
> > Yes, other than user-exposed attribute names, I think it looks good. The
> > internal code should use 'minValue' and 'maxValue' according to the IDL
> change.
> 
> According the change in the spec, we have to change the name to xxxValue.
> 
> https://github.com/WebAudio/web-audio-api/issues/762

Done.

PTAL.

[hongchan, 2016-05-13 01:30:15]

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:191:
getter max
maxValue?

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:192:
getter min
minValue?

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt:19:
CONSOLE WARNING: Oscillator.frequency value 48001 outside nominal range [0,
24000]; value will be clamped.
Same here. Not sure we should keep these warnings here. We should talk about
this.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/stereopannernode-panning-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/stereopannernode-panning-expected.txt:12:
CONSOLE WARNING: StereoPanner.pan value 1.0625 outside nominal range [-1, 1];
value will be clamped.
Is this change intentional? I thought console warnings are not something we want
to see in the test result.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:231:
getter max
maxValue

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:232:
getter min
minValue

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:586: //
Arbitrarily choose a limit of +/- Nyquist frequency for playback
Not arbitrarily - it is based on the spec. Right?

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioParam.cpp (right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.cpp:278: float min, float
max)
Here as well.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioParam.h (right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:89: float max)
Even for internal variables - I suggest we should use minValue/maxValue.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:91: return adoptRef(new
AudioParamHandler(context, paramType, defaultValue, min, max));
Ditto.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:174: static AudioParam*
create(AbstractAudioContext&, AudioParamType, double defaultValue, float min,
float max);
Here as well.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:200:
AudioParam(AbstractAudioContext&, AudioParamType, double defaultValue, float
min, float max);
Ditto.

[Raymond Toy, 2016-05-13 17:06:15]

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt:19:
CONSOLE WARNING: Oscillator.frequency value 48001 outside nominal range [0,
24000]; value will be clamped.
On 2016/05/13 01:30:14, hoch wrote:
> Same here. Not sure we should keep these warnings here. We should talk about
> this.

We decided that we wanted to generate warnings when the AudioParam is set
outside the nominal range.  How else will we know the warnings are generated if
they aren't in the expected output?

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:586: //
Arbitrarily choose a limit of +/- Nyquist frequency for playback
On 2016/05/13 01:30:14, hoch wrote:
> Not arbitrarily - it is based on the spec. Right?

Forgot to update it after it was decided to be unlimited.

[Raymond Toy, 2016-05-13 18:40:32]

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:191:
getter max
On 2016/05/13 01:30:14, hoch wrote:
> maxValue?

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:192:
getter min
On 2016/05/13 01:30:14, hoch wrote:
> minValue?

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:231:
getter max
On 2016/05/13 01:30:14, hoch wrote:
> maxValue

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:232:
getter min
On 2016/05/13 01:30:14, hoch wrote:
> minValue

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:586: //
Arbitrarily choose a limit of +/- Nyquist frequency for playback
On 2016/05/13 01:30:14, hoch wrote:
> Not arbitrarily - it is based on the spec. Right?

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioParam.cpp (right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.cpp:278: float min, float
max)
On 2016/05/13 01:30:15, hoch wrote:
> Here as well.

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioParam.h (right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:89: float max)
On 2016/05/13 01:30:15, hoch wrote:
> Even for internal variables - I suggest we should use minValue/maxValue.

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:91: return adoptRef(new
AudioParamHandler(context, paramType, defaultValue, min, max));
On 2016/05/13 01:30:15, hoch wrote:
> Ditto.

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:174: static AudioParam*
create(AbstractAudioContext&, AudioParamType, double defaultValue, float min,
float max);
On 2016/05/13 01:30:15, hoch wrote:
> Here as well.

Done.

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.h:200:
AudioParam(AbstractAudioContext&, AudioParamType, double defaultValue, float
min, float max);
On 2016/05/13 01:30:15, hoch wrote:
> Ditto.

Done.

[hongchan, 2016-05-13 21:26:23]

lgtm

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt
(right):

https://codereview.chromium.org/1803153002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt:19:
CONSOLE WARNING: Oscillator.frequency value 48001 outside nominal range [0,
24000]; value will be clamped.
On 2016/05/13 17:06:15, Raymond Toy wrote:
> On 2016/05/13 01:30:14, hoch wrote:
> > Same here. Not sure we should keep these warnings here. We should talk about
> > this.
> 
> We decided that we wanted to generate warnings when the AudioParam is set
> outside the nominal range.  How else will we know the warnings are generated
if
> they aren't in the expected output?

Okay.

[Raymond Toy, 2016-05-13 21:55:11]

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

[Raymond Toy, 2016-05-13 21:55:11]

tkent: PTAL as API owner.  We're just adding minValue and maxValue to the
AudioParam.  (Do you still need to review this if the intent to implement and
ship was approved?)

[tkent, 2016-05-15 23:50:10]

lgtm

An API owner needs to review this because LayoutTests/virtual/stable/webexposed/
is owned by API owners.

https://codereview.chromium.org/1803153002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioParam.cpp (right):

https://codereview.chromium.org/1803153002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioParam.cpp:324: DCHECK(minValue
<= maxValue);
DCHECK -> DCHECK_LE

[Raymond Toy, 2016-05-16 16:13:50]

On 2016/05/15 23:50:10, tkent wrote:
> lgtm
> 
> An API owner needs to review this because
LayoutTests/virtual/stable/webexposed/
> is owned by API owners.

Oh, right.  Forgot about that file.
> 
>
https://codereview.chromium.org/1803153002/diff/280001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AudioParam.cpp (right):
> 
>
https://codereview.chromium.org/1803153002/diff/280001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AudioParam.cpp:324: DCHECK(minValue
> <= maxValue);
> DCHECK -> DCHECK_LE

Done.

[Raymond Toy, 2016-05-16 16:14:32]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-05-16 16:14:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
hongchan@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1803153002/#ps300001
(title: "Address comments")

[commit-bot: I haz the power, 2016-05-16 16:14:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1803153002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1803153002/300001

[commit-bot: I haz the power, 2016-05-16 18:03:16]

Description was changed from

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

to

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

[commit-bot: I haz the power, 2016-05-16 18:03:17]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-05-16 18:04:18]

Description was changed from

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html
==========

to

==========
Add min/max values for AudioParams

Each AudioParam has a nominal range given by the min and max
attributes for the AudioParam.  Add these new attributes and set them
appropriately for each AudioParam.

Print a warning if the value is set (via the setter) outside the nominal range.

Spec: http://webaudio.github.io/web-audio-api/#AudioParam
WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/673

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CVOAEqoa2dc/OBPHcGFS...

BUG=594594
TEST=audioparam-nominal-range.html

Committed: https://crrev.com/71df40e1bab3e8d1bb48f539af9ae9cbb03f9bba
Cr-Commit-Position: refs/heads/master@{#393861}
==========

[commit-bot: I haz the power, 2016-05-16 18:04:19]

Patchset 16 (id:??) landed as
https://crrev.com/71df40e1bab3e8d1bb48f539af9ae9cbb03f9bba
Cr-Commit-Position: refs/heads/master@{#393861}