createPeriodicWave parameters are sequence<float>

createPeriodicWave parameters are sequence<float>

Change the parameter types for createPeriodicWave from Float32Array to
sequence<float>.  this makes createPeriodicWave signal errors if
non-finite Fourier coefficients are used and matches what the
PeriodicWave constructor behavior.

Also periodic-lengths.html was actually generating Infinity for some
coefficient values, so the test is updated to correct for that.  And
the selected waveform is changed from square to sawtooth because the
Fourier series for a sawtooth is non-zero everywhere.  This fixes the
problem where the wave with 8192 coefficients was the same as for
8191.  Which has to be true because a square wave only has non-zero
coefficients for the odd terms, so the arrays for 8192 and 8191 were
the same.  With a sawtooth, these are now different.

Add new test to verify that we throw errors if non-finite values are
used for the Fourier coefficients.

BUG=709512
TEST=periodicwave-lengths.html, periodicwave-exceptions.html

Review-Url: https://codereview.chromium.org/2807673002
Cr-Commit-Position: refs/heads/master@{#464441}
Committed: https://chromium.googlesource.com/chromium/src/+/445c9544f743c5fb1dee4ba1594360bb10366f92

[Raymond Toy, 2017-04-11 15:06:10]

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

[Raymond Toy, 2017-04-11 15:06:11]

PTAL.  The spec issue is finalized.

[hongchan, 2017-04-11 17:15:02]

lgtm with nit

https://codereview.chromium.org/2807673002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/PeriodicWave/periodicwave-exceptions.html
(right):

https://codereview.chromium.org/2807673002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/PeriodicWave/periodicwave-exceptions.html:15:
audit.define('non-finite values', (task, should) => {
Can we have a brief comment what this task does?

[Raymond Toy, 2017-04-11 20:21:15]

https://codereview.chromium.org/2807673002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/PeriodicWave/periodicwave-exceptions.html
(right):

https://codereview.chromium.org/2807673002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/PeriodicWave/periodicwave-exceptions.html:15:
audit.define('non-finite values', (task, should) => {
On 2017/04/11 17:15:02, hongchan wrote:
> Can we have a brief comment what this task does?

Done.

[Raymond Toy, 2017-04-11 20:22:58]

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

[Raymond Toy, 2017-04-11 20:22:59]

PTAL as API Owner.  We're changing the parameters for createPeriodicWave from
Float32Array to sequence<float>.  This makes the behavior compatible with
PeriodicWaveOptions dictionary that takes sequence<float>

[tkent, 2017-04-12 13:47:04]

On 2017/04/11 at 20:22:59, rtoy wrote:
> PTAL as API Owner.  We're changing the parameters for createPeriodicWave from
Float32Array to sequence<float>.  This makes the behavior compatible with
PeriodicWaveOptions dictionary that takes sequence<float>

lgtm

[Raymond Toy, 2017-04-12 14:39:24]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-04-12 14:39:25]

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

[commit-bot: I haz the power, 2017-04-12 14:39:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 14:42:19]

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

[commit-bot: I haz the power, 2017-04-12 14:42:20]

Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Raymond Toy, 2017-04-12 15:08:34]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-04-12 15:08:34]

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

[commit-bot: I haz the power, 2017-04-12 15:09:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 17:12:57]

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

[commit-bot: I haz the power, 2017-04-12 17:12:58]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raymond Toy, 2017-04-12 17:56:28]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-04-12 17:56:28]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2807673002/#ps120001
(title: "Rebaseline test result")

[commit-bot: I haz the power, 2017-04-12 17:57:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 19:46:43]

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

[commit-bot: I haz the power, 2017-04-12 19:46:44]

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

[Raymond Toy, 2017-04-12 21:35:17]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-04-12 21:35:17]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2807673002/#ps140001
(title: "Update tests and results (no SharedArrayBuffer for
createPeriodicWave)")

[commit-bot: I haz the power, 2017-04-12 21:36:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raymond Toy, 2017-04-12 21:42:28]

The CQ bit was unchecked by rtoy@chromium.org

[Raymond Toy, 2017-04-12 21:48:23]

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

[Raymond Toy, 2017-04-12 21:48:23]

binji:  You added some tests for createPeriodicWave for SharedArrayBuffers.  In
this CL, createPeriodicWave no longer takes Float32Array's but takes
sequence<float>.

I've deleted the tests for this, but is that right?  Should I leave the test but
change the expectation that it does not throw for SharedArrayBuffer's?

[binji, 2017-04-12 21:58:28]

On 2017/04/12 at 21:48:23, rtoy wrote:
> binji:  You added some tests for createPeriodicWave for SharedArrayBuffers. 
In this CL, createPeriodicWave no longer takes Float32Array's but takes
sequence<float>.
> 
> I've deleted the tests for this, but is that right?  Should I leave the test
but change the expectation that it does not throw for SharedArrayBuffer's?

I think it makes sense to remove the test. The concern with SharedArrayBuffer is
that the C++ code would use the buffer directly thinking that it could not be
modified, but would be modified concurrently by a worker thread. I assume an
ArrayBufferView is implicitly converted into a sequence<>? If so, it should be
fine because a copy of the data would be made first.

[Raymond Toy, 2017-04-12 22:16:31]

On 2017/04/12 21:58:28, binji wrote:
> On 2017/04/12 at 21:48:23, rtoy wrote:
> > binji:  You added some tests for createPeriodicWave for SharedArrayBuffers. 
> In this CL, createPeriodicWave no longer takes Float32Array's but takes
> sequence<float>.
> > 
> > I've deleted the tests for this, but is that right?  Should I leave the test
> but change the expectation that it does not throw for SharedArrayBuffer's?
> 
> I think it makes sense to remove the test. The concern with SharedArrayBuffer
is
> that the C++ code would use the buffer directly thinking that it could not be
> modified, but would be modified concurrently by a worker thread. I assume an
> ArrayBufferView is implicitly converted into a sequence<>? If so, it should be
> fine because a copy of the data would be made first.

I think it's copied.  Whatever ToImplArray<Vector<float>, IDLFloat> does. I
think that does a copy.

[Raymond Toy, 2017-04-13 14:46:07]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-04-13 14:46:07]

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

[commit-bot: I haz the power, 2017-04-13 14:46:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 16:46:17]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1492094767521000,
"parent_rev": "2f6d8f01d9087e8bebab5b7d2d25b28d657dbbb7", "commit_rev":
"445c9544f743c5fb1dee4ba1594360bb10366f92"}

[commit-bot: I haz the power, 2017-04-13 16:47:07]

Description was changed from

==========
createPeriodicWave parameters are sequence<float>

Change the parameter types for createPeriodicWave from Float32Array to
sequence<float>.  this makes createPeriodicWave signal errors if
non-finite Fourier coefficients are used and matches what the
PeriodicWave constructor behavior.

Also periodic-lengths.html was actually generating Infinity for some
coefficient values, so the test is updated to correct for that.  And
the selected waveform is changed from square to sawtooth because the
Fourier series for a sawtooth is non-zero everywhere.  This fixes the
problem where the wave with 8192 coefficients was the same as for
8191.  Which has to be true because a square wave only has non-zero
coefficients for the odd terms, so the arrays for 8192 and 8191 were
the same.  With a sawtooth, these are now different.

Add new test to verify that we throw errors if non-finite values are
used for the Fourier coefficients.

BUG=709512
TEST=periodicwave-lengths.html, periodicwave-exceptions.html
==========

to

==========
createPeriodicWave parameters are sequence<float>

Change the parameter types for createPeriodicWave from Float32Array to
sequence<float>.  this makes createPeriodicWave signal errors if
non-finite Fourier coefficients are used and matches what the
PeriodicWave constructor behavior.

Also periodic-lengths.html was actually generating Infinity for some
coefficient values, so the test is updated to correct for that.  And
the selected waveform is changed from square to sawtooth because the
Fourier series for a sawtooth is non-zero everywhere.  This fixes the
problem where the wave with 8192 coefficients was the same as for
8191.  Which has to be true because a square wave only has non-zero
coefficients for the odd terms, so the arrays for 8192 and 8191 were
the same.  With a sawtooth, these are now different.

Add new test to verify that we throw errors if non-finite values are
used for the Fourier coefficients.

BUG=709512
TEST=periodicwave-lengths.html, periodicwave-exceptions.html

Review-Url: https://codereview.chromium.org/2807673002
Cr-Commit-Position: refs/heads/master@{#464441}
Committed:
https://chromium.googlesource.com/chromium/src/+/445c9544f743c5fb1dee4ba15943...
==========

[commit-bot: I haz the power, 2017-04-13 16:47:08]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/445c9544f743c5fb1dee4ba15943...