Implement clamping of AudioParam time.

Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html

Committed: https://crrev.com/abc380c92e7b096672618c9cbad28dadf46f1df5
Cr-Commit-Position: refs/heads/master@{#431335}

[Raymond Toy, 2016-10-05 15:20:05]

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

[Raymond Toy, 2016-10-05 15:20:06]

PTAL

[Raymond Toy, 2016-10-07 18:10:11]

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

[commit-bot: I haz the power, 2016-10-07 18:10:35]

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-10-07 19:46:24]

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

[commit-bot: I haz the power, 2016-10-07 19:46:24]

Dry run: This issue passed the CQ dry run.

[hongchan, 2016-10-18 17:34:59]

Mostly nits.

Might be worth naming things with 'time' to clarify we are clamping the time,
not the value.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/audioparam-clamp-time-to-current-time.html
(right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/audioparam-clamp-time-to-current-time.html:32:
success = Should("Output[0-" + (suspendFrame - 1) + "]", result.slice(0,
Wrap this line. (and below)

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp (right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp:460: int n =
m_events.size();
n => numberOfEvents ?

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp:462: if
(m_events.size() > 0) {
Can we use |numberOfEvents| here?

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h (right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:136: bool
needsClampCheck() const { return m_needsClampCheck; }
needsTimeClampCheck()

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:137: void
clearClampCheck() { m_needsClampCheck = false; }
clearTimeClampCheck()

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:163: bool
m_needsClampCheck;
m_needsTimeClampCheck

[Raymond Toy, 2016-10-18 20:32:47]

Description was changed from

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=
==========

to

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html
==========

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

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/audioparam-clamp-time-to-current-time.html
(right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/audioparam-clamp-time-to-current-time.html:32:
success = Should("Output[0-" + (suspendFrame - 1) + "]", result.slice(0,
On 2016/10/18 17:34:59, hoch wrote:
> Wrap this line. (and below)

Done.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp (right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp:460: int n =
m_events.size();
On 2016/10/18 17:34:59, hoch wrote:
> n => numberOfEvents ?

Done.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.cpp:462: if
(m_events.size() > 0) {
On 2016/10/18 17:34:59, hoch wrote:
> Can we use |numberOfEvents| here?

Done.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h (right):

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:136: bool
needsClampCheck() const { return m_needsClampCheck; }
On 2016/10/18 17:34:59, hoch wrote:
> needsTimeClampCheck()

Done.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:137: void
clearClampCheck() { m_needsClampCheck = false; }
On 2016/10/18 17:34:59, hoch wrote:
> clearTimeClampCheck()

Done.

https://codereview.chromium.org/2391893005/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioParamTimeline.h:163: bool
m_needsClampCheck;
On 2016/10/18 17:34:59, hoch wrote:
> m_needsTimeClampCheck

Done.

[Raymond Toy, 2016-10-18 20:34:15]

Perhaps this should wait a bit.  The current spec says clamping only happens for
a few methods. I think this is an oversight. It should happen for all methods.

I'll file a spec issue on this.

[Raymond Toy, 2016-10-31 15:31:23]

On 2016/10/18 at 20:34:15, Raymond Toy wrote:
> Perhaps this should wait a bit.  The current spec says clamping only happens
for a few methods. I think this is an oversight. It should happen for all
methods.
> 
> I'll file a spec issue on this.

I think https://github.com/WebAudio/web-audio-api/issues/713 is pretty clear
that it applies to all.  And https://github.com/WebAudio/web-audio-api/pull/1051
takes care of adding the text to all other methods.

[hongchan, 2016-10-31 20:44:21]

LGTM, but we decided to wait for the official WG decision for the clamping.

[Raymond Toy, 2016-11-10 18:37:40]

On 2016/10/31 20:44:21, hongchan wrote:
> LGTM, but we decided to wait for the official WG decision for the clamping.

Spec now says clamping happens for all audio param automation methods.

[Raymond Toy, 2016-11-10 18:37:48]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2016-11-10 18:38:23]

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-10 20:14:32]

Description was changed from

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html
==========

to

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html
==========

[commit-bot: I haz the power, 2016-11-10 20:14:33]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-10 20:35:50]

Description was changed from

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html
==========

to

==========
Implement clamping of AudioParam time.

The specified time when creating an AudioParam is clamped to the
currentTime.  Because of the thread hop, the clamping needs to be done
on the audio thread, not the main thread, which is always behind the
audio thread.

BUG=622082
TEST=audioparam-clamp-time-to-current-time.html

Committed: https://crrev.com/abc380c92e7b096672618c9cbad28dadf46f1df5
Cr-Commit-Position: refs/heads/master@{#431335}
==========

[commit-bot: I haz the power, 2016-11-10 20:35:51]

Patchset 5 (id:??) landed as
https://crrev.com/abc380c92e7b096672618c9cbad28dadf46f1df5
Cr-Commit-Position: refs/heads/master@{#431335}