Implement AudioScheduledSourceNode

Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to capture
the common features of these source nodes.

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

BUG=661207
TEST=audio-scheduled-source-basic.html

Committed: https://crrev.com/00971f38908388728f49cd5127b9c6c6761d035f
Cr-Commit-Position: refs/heads/master@{#441277}

[Raymond Toy, 2016-11-03 21:05:42]

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

[commit-bot: I haz the power, 2016-11-03 21:06:16]

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-11-03 22:36:00]

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

[commit-bot: I haz the power, 2016-11-03 22:36:01]

Dry run: Try jobs failed on following builders:
  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-11-15 23:02:24]

Description was changed from

==========
Implement AudioScheduledSourceNode

WIP

BUG=661207
TEST=
==========

to

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture the common features of these source nodes.

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


BUG=661207
TEST=
==========

[Raymond Toy, 2016-11-15 23:02:40]

Description was changed from

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture the common features of these source nodes.

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


BUG=661207
TEST=
==========

to

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=
==========

[Raymond Toy, 2016-11-28 20:53:54]

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

[commit-bot: I haz the power, 2016-11-28 20:54:48]

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-11-28 22:39:33]

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

[commit-bot: I haz the power, 2016-11-28 22:39: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_...)

[Raymond Toy, 2016-11-30 21:58:34]

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

[haraken, 2016-11-30 23:54:06]

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

[haraken, 2016-11-30 23:54:07]

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl:8:
DependentLifetime

Do you know why we need to add [DependentLifetime]?

BTW, [DependentLifetime] is inherited, so you don't need to add it to
sub-classes of AudioScheduledSourceNode.

[hongchan, 2016-12-01 00:11:22]

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html:7:
<script src="resources/audio-testing.js"></script>
I know this might be too late - but can I ask you to try new audit.js here?

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:129:
USING_GARBAGE_COLLECTED_MIXIN(AudioScheduledSourceNode);
Is this needed here? All the children has this.

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl:8:
DependentLifetime
On 2016/11/30 23:54:07, haraken wrote:
> 
> Do you know why we need to add [DependentLifetime]?
> 
> BTW, [DependentLifetime] is inherited, so you don't need to add it to
> sub-classes of AudioScheduledSourceNode.

Yes, according to this description, it seems like we need this.

https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So...

> "For example, if the DOM object has [ActiveScriptWrappable] and implements
hasPendingActivity(), it must be annotated with [DependentLifetime]."

AudioScheduledSource has the |onended| callback (event handler) and also its own
lifetime with hasPendingActivity(). Doesn't it fit the bill?

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.h (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.h:46:
USING_GARBAGE_COLLECTED_MIXIN(ConstantSourceNode);
Just curious - so this only can be set up in the terminal class?

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.idl (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.idl:10:
DependentLifetime
You can remove ActiveScriptWrappable, DependentLifetime since they are
inherited.

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/OscillatorNode.idl (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/OscillatorNode.idl:40:
DependentLifetime
You can remove ActiveScriptWrappable, DependentLifetime since they are
inherited.

[Raymond Toy, 2016-12-01 00:12:11]

On 2016/11/30 23:54:07, haraken wrote:
>
https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl
> (right):
> 
>
https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl:8:
> DependentLifetime
> 
> Do you know why we need to add [DependentLifetime]?
> 
> BTW, [DependentLifetime] is inherited, so you don't need to add it to
> sub-classes of AudioScheduledSourceNode.

I vaguely remember the IDL compiler complaining about something and this is how
I shut it up.

Let me go back an undo some of these and see....

[Raymond Toy, 2016-12-01 00:13:36]

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html:7:
<script src="resources/audio-testing.js"></script>
On 2016/12/01 00:11:22, hongchan wrote:
> I know this might be too late - but can I ask you to try new audit.js here?

Maybe.  The test file is short enough to be converted.

[Raymond Toy, 2016-12-01 16:35:16]

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.idl:8:
DependentLifetime
On 2016/11/30 23:54:07, haraken wrote:
> 
> Do you know why we need to add [DependentLifetime]?
> 
> BTW, [DependentLifetime] is inherited, so you don't need to add it to
> sub-classes of AudioScheduledSourceNode.

On removing this (and no other changes), the IDL compiler complains with:

  File
"/media/rtoy/ssd/chromium/src/third_party/WebKit/Source/bindings/scripts/v8_interface.py",
line 240, in interface_context
    '[DependentLifetime]: %s' % interface.name)
Exception: [ActiveScriptWrappable] interface must also specify
[DependentLifetime]: AudioScheduledSourceNode

[Raymond Toy, 2016-12-01 18:48:32]

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audio-scheduled-source-basic.html:7:
<script src="resources/audio-testing.js"></script>
On 2016/12/01 00:13:36, Raymond Toy wrote:
> On 2016/12/01 00:11:22, hongchan wrote:
> > I know this might be too late - but can I ask you to try new audit.js here?
> 
> Maybe.  The test file is short enough to be converted.

Done.

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h
(right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:129:
USING_GARBAGE_COLLECTED_MIXIN(AudioScheduledSourceNode);
On 2016/12/01 00:11:22, hongchan wrote:
> Is this needed here? All the children has this.

This appears to be needed, if I remove this from the subclasses.

haraken@  What is the right way?

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.h (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.h:46:
USING_GARBAGE_COLLECTED_MIXIN(ConstantSourceNode);
On 2016/12/01 00:11:22, hongchan wrote:
> Just curious - so this only can be set up in the terminal class?

Removed here and for OscillatorNode.

I will have to let haraken tell me if that's right or now.  It compiles and runs
the tests fine, FWIW.

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.idl (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.idl:10:
DependentLifetime
On 2016/12/01 00:11:22, hongchan wrote:
> You can remove ActiveScriptWrappable, DependentLifetime since they are
> inherited.

Done.

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/OscillatorNode.idl (right):

https://codereview.chromium.org/2471353004/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/OscillatorNode.idl:40:
DependentLifetime
On 2016/12/01 00:11:22, hongchan wrote:
> You can remove ActiveScriptWrappable, DependentLifetime since they are
> inherited.

Done.

[hongchan, 2016-12-02 17:55:07]

lgtm for WebAudio side. Need opinions from haraken@ for GC and IDL lifetime
attributes.

[haraken, 2016-12-03 02:06:58]

LGTM

https://codereview.chromium.org/2471353004/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/2471353004/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:169:
USING_GARBAGE_COLLECTED_MIXIN(AudioBufferSourceNode);

We won't need this.

[Raymond Toy, 2016-12-05 16:44:54]

https://codereview.chromium.org/2471353004/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/2471353004/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:169:
USING_GARBAGE_COLLECTED_MIXIN(AudioBufferSourceNode);
On 2016/12/03 02:06:58, haraken wrote:
> 
> We won't need this.

Done.

[Raymond Toy, 2016-12-05 16:51:02]

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

[Raymond Toy, 2016-12-05 16:51:03]

tkent: PTAL at the expected test results and as an API OWNER.

[tkent, 2016-12-05 23:34:46]

lgtm

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

https://codereview.chromium.org/2471353004/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:144:
DEFINE_INLINE_VIRTUAL_TRACE() { AudioNode::trace(visitor); }
Looks this function is unnecessary.

[Raymond Toy, 2016-12-06 00:04:00]

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

https://codereview.chromium.org/2471353004/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:144:
DEFINE_INLINE_VIRTUAL_TRACE() { AudioNode::trace(visitor); }
On 2016/12/05 23:34:45, tkent wrote:
> Looks this function is unnecessary.

Removing this causes the compiler to complain about the use of
USING_GARBAGE_COLLECTED_MIXIN(AudioScheduledSourceNode)

Does this mean I don't need USING_GARBAGE_COLLECTED_MIXIN either?

[tkent, 2016-12-06 00:25:03]

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

https://codereview.chromium.org/2471353004/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:144:
DEFINE_INLINE_VIRTUAL_TRACE() { AudioNode::trace(visitor); }
On 2016/12/06 at 00:04:00, Raymond Toy wrote:
> On 2016/12/05 23:34:45, tkent wrote:
> > Looks this function is unnecessary.
> 
> Removing this causes the compiler to complain about the use of
USING_GARBAGE_COLLECTED_MIXIN(AudioScheduledSourceNode)
> 
> Does this mean I don't need USING_GARBAGE_COLLECTED_MIXIN either?

ActiveScriptWrappable is a GarbageCollectionMixin, and
USING_GARBAGE_COLLECTED_MIXIN is necessary.
I still don't understand why the compiler complains. Anyway it's ok to land this
as is.  I'm sorry for disturbing you.

[Raymond Toy, 2016-12-06 16:05:40]

On 2016/12/06 00:25:03, tkent wrote:
>
https://codereview.chromium.org/2471353004/diff/220001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h
> (right):
> 
>
https://codereview.chromium.org/2471353004/diff/220001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/webaudio/AudioScheduledSourceNode.h:144:
> DEFINE_INLINE_VIRTUAL_TRACE() { AudioNode::trace(visitor); }
> On 2016/12/06 at 00:04:00, Raymond Toy wrote:
> > On 2016/12/05 23:34:45, tkent wrote:
> > > Looks this function is unnecessary.
> > 
> > Removing this causes the compiler to complain about the use of
> USING_GARBAGE_COLLECTED_MIXIN(AudioScheduledSourceNode)
> > 
> > Does this mean I don't need USING_GARBAGE_COLLECTED_MIXIN either?
> 
> ActiveScriptWrappable is a GarbageCollectionMixin, and
> USING_GARBAGE_COLLECTED_MIXIN is necessary.
> I still don't understand why the compiler complains. Anyway it's ok to land
this
> as is.  I'm sorry for disturbing you.

No problem.  It's always good to have someone look over these GC things.

[Raymond Toy, 2016-12-06 16:05:49]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-12-06 16:05:49]

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

[commit-bot: I haz the power, 2016-12-06 16:06:10]

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-12-06 16:09:01]

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

[commit-bot: I haz the power, 2016-12-06 16:09:03]

Try jobs failed on following builders:
  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-12-06 16:28:59]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-12-06 16:29:00]

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

[commit-bot: I haz the power, 2016-12-06 16:29:16]

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-12-06 18:23:42]

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

[commit-bot: I haz the power, 2016-12-06 18:23:43]

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-12-19 17:51:17]

Description was changed from

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=
==========

to

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=audio-scheduled-source-basic.html
==========

[Raymond Toy, 2017-01-03 23:35:47]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-01-03 23:35:48]

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

[commit-bot: I haz the power, 2017-01-03 23:36:10]

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-01-04 01:20:40]

CQ is committing da patch.

Bot data: {"patchset_id": 500001, "attempt_start_ts": 1483486547742550,
"parent_rev": "6d51fc91e999a28297af846de0c9084a980dd3da", "commit_rev":
"769c3624debb561d2da058955a8b18262be2d77b"}

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

Description was changed from

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=audio-scheduled-source-basic.html
==========

to

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=audio-scheduled-source-basic.html

Review-Url: https://codereview.chromium.org/2471353004
==========

[commit-bot: I haz the power, 2017-01-04 01:21:13]

Committed patchset #26 (id:500001)

[commit-bot: I haz the power, 2017-01-04 01:23:33]

Description was changed from

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture 
the common features of these source nodes.

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


BUG=661207
TEST=audio-scheduled-source-basic.html

Review-Url: https://codereview.chromium.org/2471353004
==========

to

==========
Implement AudioScheduledSourceNode

Add AudioScheduledSourceNode as a base class of schedulable source nodes to
capture
the common features of these source nodes.

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

BUG=661207
TEST=audio-scheduled-source-basic.html

Committed: https://crrev.com/00971f38908388728f49cd5127b9c6c6761d035f
Cr-Commit-Position: refs/heads/master@{#441277}
==========

[commit-bot: I haz the power, 2017-01-04 01:23:35]

Patchset 26 (id:??) landed as
https://crrev.com/00971f38908388728f49cd5127b9c6c6761d035f
Cr-Commit-Position: refs/heads/master@{#441277}