Fix smooth scroll animation flake on janky pages

Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call to
AnimateState ticks an animation and UpdateState adds a new animation to be ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale start time
(last_tick_time_) and a call to AnimateState will jump and "finish" the animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous
mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/04da03ce765e60b6081ed755b834b9e7e555a64f
Cr-Commit-Position: refs/heads/master@{#412898}

[ymalik, 2016-08-17 07:49:37]

Description was changed from

==========
ElementAnimations::UpdateState shouldn't do anything unless Animate is called

BUG=622553
==========

to

==========
ElementAnimations::UpdateState shouldn't do anything unless Animate is called

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ymalik, 2016-08-17 07:50:43]

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

[commit-bot: I haz the power, 2016-08-17 07:50:58]

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-17 09:14:12]

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

[commit-bot: I haz the power, 2016-08-17 09:14:13]

Dry run: This issue passed the CQ dry run.

[ymalik, 2016-08-18 07:25:02]

Description was changed from

==========
ElementAnimations::UpdateState shouldn't do anything unless Animate is called

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
call UpdateState. In this case, the animation gets started with a stale start
time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ymalik, 2016-08-18 07:34:09]

Description was changed from

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
call UpdateState. In this case, the animation gets started with a stale start
time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale
start time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[ymalik, 2016-08-18 07:34:09]

ymalik@chromium.org changed reviewers:
+ ajuma@chromium.org, skobes@chromium.org

[ymalik, 2016-08-18 07:34:20]

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
File cc/animation/scroll_offset_animations_impl.cc (right):

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
cc/animation/scroll_offset_animations_impl.cc:96:
curve->UpdateTarget(trimmed.InSecondsF(), new_target);
The observed failure mode is that when you do two mouse wheel ticks on a janky
page, the first one would instant scroll and the second one would smooth scroll.
Here is what's happening:

The first tick will create a new scroll offset animation in the
Animation::WAITING_FOR_TARGET_AVAILABILITY state. Before a call to UpdateTarget
can put the animation in Animation::STARTING state, we get the scroll from the
second mouse wheel tick. The second tick will call
ScrollOffsetAnimationCurve::UpdateTarget with an incorrect delta (delta should
be 0 because the first animation hasn't ticked yet). This is because the call to
Animation::TrimTimeToCurrentIteration only returns 0 if the animation is in
Animation::STARTING state (see
https://cs.chromium.org/chromium/src/cc/animation/animation.cc?rcl=1471447605...).

[ajuma, 2016-08-18 14:16:25]

lgtm

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
File cc/animation/scroll_offset_animations_impl.cc (right):

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
cc/animation/scroll_offset_animations_impl.cc:90: // for run_state ==
Animation::WAITING_FOR_TARGET_AVAILABILITY.
Yeah, Animation::ConvertToActiveTime should probably treat this run state the
same as Animation::STARTING. But that can be done in a follow-up.

[ymalik, 2016-08-18 17:20:20]

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

[commit-bot: I haz the power, 2016-08-18 17:20:47]

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

[ymalik, 2016-08-18 17:23:33]

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
File cc/animation/scroll_offset_animations_impl.cc (right):

https://codereview.chromium.org/2251933002/diff/40001/cc/animation/scroll_off...
cc/animation/scroll_offset_animations_impl.cc:90: // for run_state ==
Animation::WAITING_FOR_TARGET_AVAILABILITY.
On 2016/08/18 14:16:25, ajuma wrote:
> Yeah, Animation::ConvertToActiveTime should probably treat this run state the
> same as Animation::STARTING. But that can be done in a follow-up.

Yeah. I tried to do it in this CL and a bunch of tests fail. This is because
Animation::WAITING_FOR_TARGET_AVAILABILTY is the default value, and a lot of the
tests are simply checking expectations after calling TrimTimeToCurrentIteration
without changing the default run state of the animation (so it returns 0 and not
their expectation).

[commit-bot: I haz the power, 2016-08-18 18:18:06]

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

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

Dry run: This issue passed the CQ dry run.

[skobes, 2016-08-18 18:22:06]

lgtm

I verified this fixes http://crbug.com/622553.

[ymalik, 2016-08-18 18:33:40]

The CQ bit was checked by ymalik@chromium.org

[commit-bot: I haz the power, 2016-08-18 18:34:18]

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-18 18:38:41]

Description was changed from

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale
start time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale
start time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-08-18 18:38:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-18 18:41:10]

Description was changed from

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when 
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale
start time 
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous

mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Fix smooth scroll animation flake on janky pages

Some background:
In each frame, we have AnimateLayers and after we draw we do UpdateState. A call
to
AnimateState ticks an animation and UpdateState adds a new animation to be
ticked
in subsequent frames.

This CL fixes the following two cases:
1) An animation can get added at an unusual point. This can happen when
GestureScrollUpdate gets delivered after we AnimateLayers but before draw and
a call to UpdateState. In this case, the animation gets started with a stale
start time
(last_tick_time_) and a call to AnimateState will jump and "finish" the
animation. To
prevent this, we reset last_tick_time_ when element_animations becomes inactive.

2) An animation gets updated while it hasn't started and is in the
WAITING_FOR_TARGET_AVAILABILITY state. This happens when you do two simultaneous
mouse wheel ticks on a janky page. See inline comment for more details.

BUG=622553
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/04da03ce765e60b6081ed755b834b9e7e555a64f
Cr-Commit-Position: refs/heads/master@{#412898}
==========

[commit-bot: I haz the power, 2016-08-18 18:41:11]

Patchset 3 (id:??) landed as
https://crrev.com/04da03ce765e60b6081ed755b834b9e7e555a64f
Cr-Commit-Position: refs/heads/master@{#412898}