Correctly reset ScrollAnimator animation state after cancelling animation

Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
Committed: https://crrev.com/d7b5be44530835f2b1f3c3cfc8d6d7ea18a50553
Cr-Commit-Position: refs/heads/master@{#383854}

[ymalik, 2016-03-29 18:00:59]

Description was changed from

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
==========

to

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
==========

[ymalik, 2016-03-29 18:00:59]

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

[ajuma, 2016-03-29 18:41:57]

https://codereview.chromium.org/1842623003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp (right):

https://codereview.chromium.org/1842623003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp:116:
resetAnimationState();
What happens to the actual cleanup (specifically, the removal of the main thread
scrolling reason)?

[ymalik, 2016-03-29 19:42:17]

https://codereview.chromium.org/1842623003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp (right):

https://codereview.chromium.org/1842623003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp:116:
resetAnimationState();
On 2016/03/29 18:41:57, ajuma wrote:
> What happens to the actual cleanup (specifically, the removal of the main
thread
> scrolling reason)?

That's a good point (though that's a separate issue from this).

This is the special case when we get a user scroll while we are in the
PostAnimationCleanup state after an animation is cancelled. Note that to remove
the main thread scrolling reason, we need to be in compositing clean.

After the call to willAnimateToOffset below, we will either start a new
animation (or update the current one) or not do anything. If we start a new
animation, we will add main thread scrolling reason anyway, so it's probably
okay to not clear it here. We were not handling the latter, which I think is
fixed if we stay in the PostAnimationCleanup state. Does that make sense?

[ajuma, 2016-03-29 20:22:29]

Thanks, lgtm.

[ymalik, 2016-03-29 20:23:34]

ymalik@chromium.org changed reviewers:
+ jbroman@chromium.org

[ymalik, 2016-03-29 20:23:35]

+Jeremy for Source/platform

[jbroman, 2016-03-29 20:28:02]

rs lgtm

[ymalik, 2016-03-29 20:40:13]

The CQ bit was checked by ymalik@chromium.org

[commit-bot: I haz the power, 2016-03-29 20:40:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1842623003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1842623003/20001

[commit-bot: I haz the power, 2016-03-29 23:03:02]

Description was changed from

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
==========

to

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
==========

[commit-bot: I haz the power, 2016-03-29 23:03:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-29 23:04:22]

Description was changed from

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548
==========

to

==========
Correctly reset ScrollAnimator animation state after cancelling animation

We were calling willAnimaterToOffset with the updated target position before
clearing the animation state when in PostAnimationCleanup. As a result, we
didn't take into account the potentially modified scroll offset by a
programmatic scroll.

BUG=598548

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

[commit-bot: I haz the power, 2016-03-29 23:04:23]

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