Implement ConstantSourceNode (issue 2134813002 by rtoy@chromium.org)

1 view
Skip to first unread message

rtoy@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 12:09:58 PM9/30/16
to hongchan...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Reviewers: hoch
CL: https://codereview.chromium.org/2134813002/

Message:
PTAL

Description:
Implement ConstantSourceNode

Add implementation of ConstantSourceNode, including a factory method
and a constructor.

This serves as a very useful constant source node, and, because it has
an AudioParam, it can be used as a constructible AudioParam.

Feature: https://www.chromestatus.com/features/5647701588836352
Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/9uoSMZftWfY/GJaqkCUNAAAJ

BUG=644438
TEST=constant-source-basic.html, constant-source-output.html

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+1707, -31 lines):
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
M third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range.html
M third_party/WebKit/LayoutTests/webaudio/audioparam-nominal-range-expected.txt
A third_party/WebKit/LayoutTests/webaudio/constant-source-basic.html
A third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html
A third_party/WebKit/LayoutTests/webaudio/constant-source-output.html
A third_party/WebKit/LayoutTests/webaudio/constructor/constantsource.html
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
M third_party/WebKit/Source/core/frame/UseCounter.h
A third_party/WebKit/Source/modules/modules.gypi
M third_party/WebKit/Source/modules/modules_idl_files.gni
M third_party/WebKit/Source/modules/webaudio/AudioNode.h
M third_party/WebKit/Source/modules/webaudio/AudioParam.h
M third_party/WebKit/Source/modules/webaudio/AudioParam.cpp
M third_party/WebKit/Source/modules/webaudio/BUILD.gn
M third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h
M third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
M third_party/WebKit/Source/modules/webaudio/BaseAudioContext.idl
A third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.h
A third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.cpp
A third_party/WebKit/Source/modules/webaudio/ConstantSourceNode.idl
A + third_party/WebKit/Source/modules/webaudio/ConstantSourceOptions.idl
M third_party/WebKit/Source/modules/webaudio/WindowWebAudio.idl
M tools/metrics/histograms/histograms.xml


rtoy@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 12:24:30 PM9/30/16
to hongchan...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org

https://codereview.chromium.org/2134813002/diff/340001/third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html
File
third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html
(right):

https://codereview.chromium.org/2134813002/diff/340001/third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html#newcode21
third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html:21:
audit.defineTask("onended", function (taskDone) {
Maybe we don't want to use Audit for this?

https://codereview.chromium.org/2134813002/

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 5, 2016, 11:23:32 AM10/5/16
to hongchan...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Ping. Should I get another reviewer?

https://codereview.chromium.org/2134813002/

hong...@chromium.org

unread,
Oct 6, 2016, 2:10:24 PM10/6/16
to rt...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, rt...@chromium.org
lgtm

lgtm with some nits in layout tests.


https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html
File
third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html
(right):

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html#newcode32
third_party/WebKit/LayoutTests/webaudio/constant-source-onended.html:32:
tester.done();
This and taskDone() at the bottom. Which one ends first? Can you
guarantee their order without using suspend()?
I think we need to make sure tester.don() ends first no matter what.

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html
File third_party/WebKit/LayoutTests/webaudio/constant-source-output.html
(right):

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode33
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:33:
Should("ConstantSourceNode({offset: 0.5})",
actual).beEqualToArray(expected);
wrap this line.

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode101
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:101:
var expected = createLinearRampArray(0, rampEndTime, 0.5, 1,
context.sampleRate);
wrap line.

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode154
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:154:
success = Should("ConstantSourceNode frames [0, " + sourceStartFrame +
")",
wrap line.

https://codereview.chromium.org/2134813002/

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 7, 2016, 11:24:08 AM10/7/16
to hongchan...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org
On 2016/10/06 18:10:23, hoch wrote:
> This and taskDone() at the bottom. Which one ends first? Can you
guarantee their
> order without using suspend()?
> I think we need to make sure tester.don() ends first no matter what.

Yeah, I think we should just not use Audit here in favor of plain
testharness.

https://codereview.chromium.org/2134813002/

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 7, 2016, 11:49:11 AM10/7/16
to hongchan...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html
File third_party/WebKit/LayoutTests/webaudio/constant-source-output.html
(right):

https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode33
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:33:
Should("ConstantSourceNode({offset: 0.5})",
actual).beEqualToArray(expected);
On 2016/10/06 18:10:23, hoch wrote:
> wrap this line.

Done.


https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode101
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:101:
var expected = createLinearRampArray(0, rampEndTime, 0.5, 1,
context.sampleRate);
On 2016/10/06 18:10:23, hoch wrote:
> wrap line.

Done.


https://codereview.chromium.org/2134813002/diff/380001/third_party/WebKit/LayoutTests/webaudio/constant-source-output.html#newcode154
third_party/WebKit/LayoutTests/webaudio/constant-source-output.html:154:
success = Should("ConstantSourceNode frames [0, " + sourceStartFrame +
")",
On 2016/10/06 18:10:23, hoch wrote:

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 10, 2016, 11:37:06 AM10/10/16
to hongchan...@chromium.org, tk...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, har...@chromium.org, hongchan...@chromium.org

tk...@chromium.org

unread,
Oct 11, 2016, 9:58:44 AM10/11/16
to rt...@chromium.org, hongchan...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org, rt...@chromium.org

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 11, 2016, 12:30:14 PM10/11/16
to tk...@chromium.org, hongchan...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
On 2016/10/11 13:58:43, tkent wrote:
>
https://codereview.chromium.org/2134813002/diff/400001/third_party/WebKit/Source/modules/modules.gypi
> File third_party/WebKit/Source/modules/modules.gypi (right):
>
>
https://codereview.chromium.org/2134813002/diff/400001/third_party/WebKit/Source/modules/modules.gypi#newcode1
> third_party/WebKit/Source/modules/modules.gypi:1: {
> I guess you added this file by mistake.

Oops. It used to be required, but has been removed recently.

Fixed.

https://codereview.chromium.org/2134813002/

tk...@chromium.org

unread,
Oct 11, 2016, 6:22:36 PM10/11/16
to rt...@chromium.org, hongchan...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 12, 2016, 11:13:25 AM10/12/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 12, 2016, 11:25:46 AM10/12/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Try jobs failed on following builders:
chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/279314)

https://codereview.chromium.org/2134813002/

rtoy@chromium.org via codereview.chromium.org

unread,
Oct 12, 2016, 11:45:01 AM10/12/16
to tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
isherman@ PTAL at histograms.xml changes.

https://codereview.chromium.org/2134813002/

ishe...@chromium.org

unread,
Oct 12, 2016, 8:37:39 PM10/12/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 10:56:05 AM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 11:05:17 AM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Try jobs failed on following builders:
chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 11:22:38 AM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 3:09:37 PM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Try jobs failed on following builders:

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 4:24:57 PM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 6:14:59 PM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Committed patchset #24 (id:460001)

https://codereview.chromium.org/2134813002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 13, 2016, 6:16:47 PM10/13/16
to rt...@chromium.org, tk...@chromium.org, hongchan...@chromium.org, isherman...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, har...@chromium.org, hongchan...@chromium.org
Patchset 24 (id:??) landed as
https://crrev.com/f61f1e00fc29d3e88ef4e3d7b1c63d5b469d92f2
Cr-Commit-Position: refs/heads/master@{#425184}

https://codereview.chromium.org/2134813002/
Reply all
Reply to author
Forward
0 new messages