Add constructors for WebAudio nodes

Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support for
new FooNode(context, options).  The options argument is a dictionary
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes
and for testing that the required arguments (if any) are processed correctly.

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuuAAAJ
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html

Committed: https://crrev.com/57909df69be0cf0b2ac42e5c9c018d4a00ee9766
Cr-Commit-Position: refs/heads/master@{#419195}

[Raymond Toy, 2016-07-13 22:02:49]

Description was changed from

==========
Add constructors for WebAudio nodes

BUG=
==========

to

==========
Add constructors for WebAudio nodes

BUG=626449
==========

[Raymond Toy, 2016-07-29 17:09:31]

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

[commit-bot: I haz the power, 2016-07-29 17:09:55]

Dry run: 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, 2016-07-29 17:19:14]

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

[commit-bot: I haz the power, 2016-07-29 17:19:15]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raymond Toy, 2016-08-23 20:24:49]

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

[commit-bot: I haz the power, 2016-08-23 20:25:13]

Dry run: 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, 2016-08-23 20:37:57]

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

[commit-bot: I haz the power, 2016-08-23 20:37:58]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Raymond Toy, 2016-08-23 20:51:38]

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

[commit-bot: I haz the power, 2016-08-23 20:52:30]

Dry run: 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, 2016-08-23 20:56:31]

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

[commit-bot: I haz the power, 2016-08-23 20:56:32]

Dry run: 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, 2016-08-23 21:19:23]

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

[commit-bot: I haz the power, 2016-08-23 21:20:28]

Dry run: 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, 2016-08-23 23:14:06]

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

[commit-bot: I haz the power, 2016-08-23 23:14:07]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Raymond Toy, 2016-09-07 20:37:55]

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

[commit-bot: I haz the power, 2016-09-07 20:38:17]

Dry run: 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, 2016-09-07 20:41:28]

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

[commit-bot: I haz the power, 2016-09-07 20:41:29]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  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, 2016-09-07 22:37:21]

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

[commit-bot: I haz the power, 2016-09-07 22:37:43]

Dry run: 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, 2016-09-08 00:34:30]

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

[commit-bot: I haz the power, 2016-09-08 00:34:31]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Raymond Toy, 2016-09-08 17:23:30]

Description was changed from

==========
Add constructors for WebAudio nodes

BUG=626449
==========

to

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for new
FooNode(context, options).  The options argument is a dictionary specific to the
node and is also defined here.

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

BUG=626449
TEST=
==========

[Raymond Toy, 2016-09-08 17:26:54]

Description was changed from

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for new
FooNode(context, options).  The options argument is a dictionary specific to the
node and is also defined here.

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

BUG=626449
TEST=
==========

to

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for new
FooNode(context, options).  The options argument is a dictionary specific to the
node and is also defined here.

Tests added for the basic functionality for constructing the nodes and for
testing that the required arguments (if any) are processed correctly.

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

BUG=626449
TEST=constructor/*.html
==========

[Raymond Toy, 2016-09-08 17:27:26]

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

[Raymond Toy, 2016-09-08 17:27:28]

PTAL

[hongchan, 2016-09-08 21:41:11]

Some nits for mechanical stuff.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html:44:
taskDone();
I think we should add 'summarize' to the task instead of passing 'success' from
top to the bottom, but perhaps in a different CL.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html
(right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html:14:
test(function () {
Are you keeping the test in this way? This is okay, but is better with the Audit
style.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:150: {
DCHECK(isMainThread());

Perhaps?

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:154: return node;
Wouldn't it be better to return nullptr?

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.idl (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.idl:26:
Constructor(AudioContext context, optional AnalyserOptions options),
The spec says BaseAudioContext instead of AudioContext, but it's sort of
nonsensical to use Analyser on the OAC. Still, we should think about this.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBuffer.idl (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBuffer.idl:30:
Constructor(AudioContext context, AudioBufferOptions options),
Same thing - this should be BaseAudioContext.

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

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:619: {
I prefer to have a DCHECK here.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:623: return
node;
nullptr?

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.idl
(right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.idl:27:
Constructor(AudioContext context, optional AudioBufferSourceOptions options),
BaseAudioContext?

[Raymond Toy, 2016-09-08 21:58:00]

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html:44:
taskDone();
On 2016/09/08 21:41:11, hoch wrote:
> I think we should add 'summarize' to the task instead of passing 'success'
from
> top to the bottom, but perhaps in a different CL.

Yeah, I'd love to have task do that for me.  But yeah, probably best done
separately.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html
(right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html:14:
test(function () {
On 2016/09/08 21:41:11, hoch wrote:
> Are you keeping the test in this way? This is okay, but is better with the
Audit
> style.

Oops. I forgot to rewrite these.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:150: {
On 2016/09/08 21:41:11, hoch wrote:
> DCHECK(isMainThread());
> 
> Perhaps?

Yes!

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.idl (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.idl:26:
Constructor(AudioContext context, optional AnalyserOptions options),
On 2016/09/08 21:41:11, hoch wrote:
> The spec says BaseAudioContext instead of AudioContext, but it's sort of
> nonsensical to use Analyser on the OAC. Still, we should think about this.

I think you're looking at the old version of this file. I updated them all to
say BaseAudioContext.

But with suspend/resume, AnalyserNodes work just fine on offline contexts.

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

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:619: {
On 2016/09/08 21:41:11, hoch wrote:
> I prefer to have a DCHECK here.

DCHECK what?

[Raymond Toy, 2016-09-08 21:58:44]

Description was changed from

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for new
FooNode(context, options).  The options argument is a dictionary specific to the
node and is also defined here.

Tests added for the basic functionality for constructing the nodes and for
testing that the required arguments (if any) are processed correctly.

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

BUG=626449
TEST=constructor/*.html
==========

to

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

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

BUG=626449
TEST=constructor/*.html
==========

[hongchan, 2016-09-08 22:03:12]

I think these changes need to be applied to all the constructors in the CL.

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

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:619: {
DCHECK(isMainThread());

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:623: return
node;
return nullptr;

[haraken, 2016-09-09 00:57:08]

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

[haraken, 2016-09-09 00:57:10]

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl (right):

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl:4: 

Add a link to the spec. The same comment for other IDL files in this CL.

[haraken, 2016-09-09 00:58:14]

haraken@chromium.org changed reviewers:
+ bashi@chromium.org

[haraken, 2016-09-09 00:58:16]

+bashi: FYI, this CL introduces a heavy dictionary use.

[bashi, 2016-09-09 01:16:21]

On 2016/09/09 00:58:16, haraken wrote:
> +bashi: FYI, this CL introduces a heavy dictionary use.

I think we should prioritize crbug.com/625295. It requires some design changes
in the code generator though.

[Raymond Toy, 2016-09-09 17:19:10]

On 2016/09/09 01:16:21, bashi1 wrote:
> On 2016/09/09 00:58:16, haraken wrote:
> > +bashi: FYI, this CL introduces a heavy dictionary use.
> 
> I think we should prioritize crbug.com/625295. It requires some design changes
> in the code generator though.

Should I wait on this CL until crbug.com/625295 lands?

[Raymond Toy, 2016-09-09 17:32:53]

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:150: {
On 2016/09/08 21:41:11, hoch wrote:
> DCHECK(isMainThread());
> 
> Perhaps?

Done.

https://codereview.chromium.org/2102133002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:154: return node;
On 2016/09/08 21:41:11, hoch wrote:
> Wouldn't it be better to return nullptr?

Done.

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl (right):

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl:4: 
On 2016/09/09 00:57:10, haraken wrote:
> 
> Add a link to the spec. The same comment for other IDL files in this CL.

Done, for the dictionaries.

Did you want links for all other IDL files that were modified too?

[Raymond Toy, 2016-09-09 18:33:20]

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

[commit-bot: I haz the power, 2016-09-09 18:33:43]

Dry run: 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, 2016-09-09 18:37:13]

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

[commit-bot: I haz the power, 2016-09-09 18:37:15]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  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, 2016-09-09 22:09:32]

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

[commit-bot: I haz the power, 2016-09-09 22:10:03]

Dry run: 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, 2016-09-10 00:58:32]

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

[commit-bot: I haz the power, 2016-09-10 00:58:34]

Dry run: 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_...)

[haraken, 2016-09-10 01:31:10]

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl (right):

https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl:4: 
On 2016/09/09 17:32:53, Raymond Toy wrote:
> On 2016/09/09 00:57:10, haraken wrote:
> > 
> > Add a link to the spec. The same comment for other IDL files in this CL.
> 
> Done, for the dictionaries.
> 
> Did you want links for all other IDL files that were modified too?

Yes, we want to have the spec link in all IDL files.

[Raymond Toy, 2016-09-12 16:11:12]

On 2016/09/10 01:31:10, haraken wrote:
>
https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl (right):
> 
>
https://codereview.chromium.org/2102133002/diff/360001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AnalyserOptions.idl:4: 
> On 2016/09/09 17:32:53, Raymond Toy wrote:
> > On 2016/09/09 00:57:10, haraken wrote:
> > > 
> > > Add a link to the spec. The same comment for other IDL files in this CL.
> > 
> > Done, for the dictionaries.
> > 
> > Did you want links for all other IDL files that were modified too?
> 
> Yes, we want to have the spec link in all IDL files.

Done.

[Raymond Toy, 2016-09-12 16:14:13]

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

[commit-bot: I haz the power, 2016-09-12 16:14:28]

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

[Raymond Toy, 2016-09-12 16:17:10]

The CQ bit was unchecked by rtoy@chromium.org

[Raymond Toy, 2016-09-12 16:22:35]

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

[commit-bot: I haz the power, 2016-09-12 16:22:56]

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

[hongchan, 2016-09-12 18:56:33]

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:156: return node;
return nullptr;

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

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:625: return
node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.cpp:75: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp:169: return
node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp:124: return
node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp:194: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/DelayNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/DelayNode.cpp:93: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/DynamicsCompressorNode.cpp
(right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/DynamicsCompressorNode.cpp:165:
return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/GainNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/GainNode.cpp:147: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/OscillatorNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/OscillatorNode.cpp:362: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/PannerNode.cpp:677: return node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:162: return
node;
return nullptr;

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/WaveShaperNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/WaveShaperNode.cpp:61: return node;
return nullptr;

[Raymond Toy, 2016-09-12 21:04:58]

On 2016/09/12 18:56:33, hoch wrote:
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:156: return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
> (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:625:
return
> node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.cpp:75: return
node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp:169: return
> node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp
(right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp:124: return
> node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp:194: return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/DelayNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/DelayNode.cpp:93: return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/DynamicsCompressorNode.cpp
> (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/DynamicsCompressorNode.cpp:165:
> return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/GainNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/GainNode.cpp:147: return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/OscillatorNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/OscillatorNode.cpp:362: return
node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/PannerNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/PannerNode.cpp:677: return node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:162: return
> node;
> return nullptr;
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/WaveShaperNode.cpp (right):
> 
>
https://codereview.chromium.org/2102133002/diff/460001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/WaveShaperNode.cpp:61: return node;
> return nullptr;

Done.  Sorry about not fixing these all earlier!

[bashi, 2016-09-12 22:39:49]

On 2016/09/09 17:19:10, Raymond Toy wrote:
> On 2016/09/09 01:16:21, bashi1 wrote:
> > On 2016/09/09 00:58:16, haraken wrote:
> > > +bashi: FYI, this CL introduces a heavy dictionary use.
> > 
> > I think we should prioritize crbug.com/625295. It requires some design
changes
> > in the code generator though.
> 
> Should I wait on this CL until crbug.com/625295 lands?

Please go ahead without waiting for crbug.com/625295. We don't have a plan to
support multiple dictionaries in one IDL file at this point.

[haraken, 2016-09-13 00:13:21]

LGTM

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp:166: node =
create(*context, options.numberOfInputs(), exceptionState);

ChannelMergerNode* node = create(...)

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp:121: node =
create(*context, options.numberOfOutputs(), exceptionState);

ChannelSplitterNode* node = create()

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/IIRFilterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/IIRFilterNode.cpp:140: IIRFilterNode*
node = create(*context, options.feedforward(), options.feedback(),
exceptionState);

You need to check if(!node).

[Raymond Toy, 2016-09-13 15:44:49]

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

[Raymond Toy, 2016-09-13 15:44:57]

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp:166: node =
create(*context, options.numberOfInputs(), exceptionState);
On 2016/09/13 00:13:20, haraken wrote:
> 
> ChannelMergerNode* node = create(...)

Done.

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ChannelSplitterNode.cpp:121: node =
create(*context, options.numberOfOutputs(), exceptionState);
On 2016/09/13 00:13:20, haraken wrote:
> 
> ChannelSplitterNode* node = create()

Done.

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/IIRFilterNode.cpp (right):

https://codereview.chromium.org/2102133002/diff/480001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/IIRFilterNode.cpp:140: IIRFilterNode*
node = create(*context, options.feedforward(), options.feedback(),
exceptionState);
On 2016/09/13 00:13:21, haraken wrote:
> 
> You need to check if(!node).

Done.

[commit-bot: I haz the power, 2016-09-13 15:45:19]

Dry run: 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, 2016-09-13 17:21:42]

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

[commit-bot: I haz the power, 2016-09-13 17:21:44]

Dry run: This issue passed the CQ dry run.

[hongchan, 2016-09-13 22:13:23]

The first pass on the layout tests.

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

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:3572:
interface PannerNode : AudioNode
Hmm, why panner node?

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:5247:
interface StereoPannerNode : AudioNode
and stereo panner node?

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html:161: success =
Should("node = new AnalyserNode(..., {minDecibels: -10, maxDecibels: 20})",
var opts = { minDecibels: -10, maxDecibels: 20 };
success = Should("node = new AnalyserNode(... ," + JSON.stringify(opts),
function () {
  node = new AnalyserNode(context, opts);
}).notThrow() && success;

I think this reads better. Also you can reuse |opts|.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html:89: success
= Should("new AudioBuffer(context, {numberOfChannels: 0})", function () {
We can apply the same technique of JSON.stringify here. Then they will match in
the string and the actual arguments.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html:145: var
message = "new AudioBuffer(context, {";
var message = "..."
  + "..." + val1
  + "..." + val2
  + "...";

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html:99: }
missing ;

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html:101:
message = "node = new AudioBufferSourceNode(context, {";
Let's not repeat |message| here and on.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/channelmerger.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/channelmerger.html:112:
success = Should("new ChannelMergerNode(..., { numberOfInputs: 99 }",
We can use the same technique for { numberOfInputs: 99 }.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:85: message
+= "{ buffer: <buf>, " + "disableNormalization: ";
Replace:

; message += "..."

into

  + "..."

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:104: message
+= "{ buffer: " + options.buffer + ", " + "disableNormalization: ";
Ditto.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:115: message
+= "{ " + "disableNormalization: ";
Ditto.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/delay.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/delay.html:49: success =
success = Should("node = new DelayNode(context)", function () {
Not sure what you want here? Why repeating |success|?

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:57:
success = success = Should("node.threshold.value", node.threshold.value)
success = success

This seems to be wrong...

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:108:
success = success = Should("node.threshold.value", node.threshold.value)
Ditto.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/gain.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:50: success =
success = Should("node = new GainNode(context)", function () {
Two |success| here as well.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:90: success =
success = Should("node instanceof GainNode",
Ditto.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/iirfilter.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/iirfilter.html:61: success =
Should("node = new IIRFilterNode(context, {feedback: [1, .5]})", function () {
Better to use JSON.stringify() here.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/stereopanner.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/stereopanner.html:75:
success = Should("new StereoPannerNode(c, {channelCount: 1})",
Let's use JSON.stringify() for the options.

[hongchan, 2016-09-14 17:40:07]

The second pass.

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html (right):

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:112: message
= "node = new ConvolverNode(context, " + JSON.stringify(options) + "";
Perhaps this empty quote is a mistake?

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html
(right):

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:106:
success = success = Should("node instanceof DynamicsCompressorNode",
We still have duplicated |success|s.

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/gain.html (right):

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:50: success =
success = Should("node = new GainNode(context)", function () {
It's still here!

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:90: success =
success = Should("node instanceof GainNode",
Here as well!

[Raymond Toy, 2016-09-14 18:02:24]

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

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt:3572:
interface PannerNode : AudioNode
On 2016/09/13 22:13:22, hoch wrote:
> Hmm, why panner node?

Good question. I don't know why it's suddenly showing up now. The other nodes
are already there.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/analyser.html:161: success =
Should("node = new AnalyserNode(..., {minDecibels: -10, maxDecibels: 20})",
On 2016/09/13 22:13:22, hoch wrote:
> var opts = { minDecibels: -10, maxDecibels: 20 };
> success = Should("node = new AnalyserNode(... ," + JSON.stringify(opts),
> function () {
>   node = new AnalyserNode(context, opts);
> }).notThrow() && success;
> 
> I think this reads better. Also you can reuse |opts|.

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffer.html:89: success
= Should("new AudioBuffer(context, {numberOfChannels: 0})", function () {
On 2016/09/13 22:13:22, hoch wrote:
> We can apply the same technique of JSON.stringify here. Then they will match
in
> the string and the actual arguments.

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html:99: }
On 2016/09/13 22:13:22, hoch wrote:
> missing ;

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/audiobuffersource.html:101:
message = "node = new AudioBufferSourceNode(context, {";
On 2016/09/13 22:13:22, hoch wrote:
> Let's not repeat |message| here and on.

Using JSON.stringify instead.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/channelmerger.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/channelmerger.html:112:
success = Should("new ChannelMergerNode(..., { numberOfInputs: 99 }",
On 2016/09/13 22:13:22, hoch wrote:
> We can use the same technique for { numberOfInputs: 99 }.

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:85: message
+= "{ buffer: <buf>, " + "disableNormalization: ";
On 2016/09/13 22:13:22, hoch wrote:
> Replace:
> 
> ; message += "..."
> 
> into
> 
>   + "..."

Used JSON.stringify instead.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/delay.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/delay.html:49: success =
success = Should("node = new DelayNode(context)", function () {
On 2016/09/13 22:13:22, hoch wrote:
> Not sure what you want here? Why repeating |success|?

Too much copy and paste.  Fixed.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:57:
success = success = Should("node.threshold.value", node.threshold.value)
On 2016/09/13 22:13:22, hoch wrote:
> success = success
> 
> This seems to be wrong...

Typo. Fixed.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:108:
success = success = Should("node.threshold.value", node.threshold.value)
On 2016/09/13 22:13:22, hoch wrote:
> Ditto.

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/gain.html (right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:50: success =
success = Should("node = new GainNode(context)", function () {
On 2016/09/13 22:13:22, hoch wrote:
> Two |success| here as well.

Fixed.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/gain.html:90: success =
success = Should("node instanceof GainNode",
On 2016/09/13 22:13:22, hoch wrote:
> Ditto.

Done.

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/stereopanner.html
(right):

https://codereview.chromium.org/2102133002/diff/500001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/stereopanner.html:75:
success = Should("new StereoPannerNode(c, {channelCount: 1})",
On 2016/09/13 22:13:22, hoch wrote:
> Let's use JSON.stringify() for the options.

Done.

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html (right):

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/convolver.html:112: message
= "node = new ConvolverNode(context, " + JSON.stringify(options) + "";
On 2016/09/14 17:40:07, hoch wrote:
> Perhaps this empty quote is a mistake? 

Fixed. It should be ")"

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html
(right):

https://codereview.chromium.org/2102133002/diff/520001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/constructor/dynamicscompressor.html:106:
success = success = Should("node instanceof DynamicsCompressorNode",
On 2016/09/14 17:40:07, hoch wrote:
> We still have duplicated |success|s.

Found and fixed them all, everywhere.

[hongchan, 2016-09-14 18:03:46]

lgtm

[Raymond Toy, 2016-09-14 18:03:55]

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

[commit-bot: I haz the power, 2016-09-14 18:04:29]

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

[Raymond Toy, 2016-09-14 18:23:00]

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

[Raymond Toy, 2016-09-14 18:23:02]

mkwst@chromium.org: Please review changes in histograms.xml, UseCounter.h,
modules_idl_files.gni.

I'm not sure about the use counter names.



tkent@: Please review as API owner.

[commit-bot: I haz the power, 2016-09-14 22:38:15]

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

[commit-bot: I haz the power, 2016-09-14 22:38:17]

Dry run: This issue passed the CQ dry run.

[tkent, 2016-09-14 23:05:11]

webexposed lgtm

[Mike West, 2016-09-16 05:09:03]

On 2016/09/14 at 18:23:02, rtoy wrote:
> mkwst@chromium.org: Please review changes in histograms.xml, UseCounter.h,
modules_idl_files.gni.
> 
> I'm not sure about the use counter names.

LGTM.

I'd suggest prefixing the names with `WebAudio*` so that you can easily find
them on `chromestatus.com`, but there's no hard and fast rule to follow for
counter naming schemes.

[Raymond Toy, 2016-09-16 15:09:47]

On 2016/09/16 05:09:03, Mike West wrote:
> On 2016/09/14 at 18:23:02, rtoy wrote:
> > mailto:mkwst@chromium.org: Please review changes in histograms.xml,
UseCounter.h,
> modules_idl_files.gni.
> > 
> > I'm not sure about the use counter names.
> 
> LGTM.
> 
> I'd suggest prefixing the names with `WebAudio*` so that you can easily find
> them on `chromestatus.com`, but there's no hard and fast rule to follow for
> counter naming schemes.

Good idea.  Done.

[Raymond Toy, 2016-09-16 15:11:36]

Description was changed from

==========
Add constructors for WebAudio nodes

In addition to createFoo() to create an AudioNode, we add support for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

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

BUG=626449
TEST=constructor/*.html
==========

to

==========
Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support
for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuu...
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html
==========

[Raymond Toy, 2016-09-16 15:12:17]

Thanks, everyone, for your review!

[Raymond Toy, 2016-09-16 15:12:35]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-09-16 15:12:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, tkent@chromium.org, mkwst@chromium.org,
hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2102133002/#ps560001
(title: "Rebase and prefix use counter names with WebAudio")

[commit-bot: I haz the power, 2016-09-16 15:13:05]

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, 2016-09-16 17:07:15]

Description was changed from

==========
Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support
for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuu...
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html
==========

to

==========
Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support
for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuu...
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html
==========

[commit-bot: I haz the power, 2016-09-16 17:07:17]

Committed patchset #29 (id:560001)

[commit-bot: I haz the power, 2016-09-16 17:08:49]

Description was changed from

==========
Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support
for 
new FooNode(context, options).  The options argument is a dictionary 
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes 
and for testing that the required arguments (if any) are processed correctly.

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuu...
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html
==========

to

==========
Add constructors for WebAudio nodes

In addition to the existing createFoo() to create an AudioNode, we add support
for
new FooNode(context, options).  The options argument is a dictionary
specific to the node and is also defined here.

Tests added for the basic functionality for constructing the nodes
and for testing that the required arguments (if any) are processed correctly.

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/QpOwpM15YRA/SQJbPmuu...
Feature: https://www.chromestatus.com/features/6409704485945344

BUG=626449
TEST=constructor/*.html

Committed: https://crrev.com/57909df69be0cf0b2ac42e5c9c018d4a00ee9766
Cr-Commit-Position: refs/heads/master@{#419195}
==========

[commit-bot: I haz the power, 2016-09-16 17:08:51]

Patchset 29 (id:??) landed as
https://crrev.com/57909df69be0cf0b2ac42e5c9c018d4a00ee9766
Cr-Commit-Position: refs/heads/master@{#419195}