Remove a frame of delay on main-thread FlingStart. [2nd land]

Remove a frame of delay on main-thread FlingStart. [2nd land]

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964
Review-Url: https://codereview.chromium.org/2908073008
Cr-Original-Commit-Position: refs/heads/master@{#475984}
Committed: https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41
Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#476558}
Committed: https://chromium.googlesource.com/chromium/src/+/8d929084e3659b06f6ba30f334541bdde21866a6

[aelias_OOO_until_Jul13, 2017-05-30 23:57:06]

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

[aelias_OOO_until_Jul13, 2017-05-30 23:57:52]

aelias@chromium.org changed reviewers:
+ dtapuska@chromium.org

[aelias_OOO_until_Jul13, 2017-05-30 23:57:58]

Hi dtapuska@, PTAL.

[commit-bot: I haz the power, 2017-05-30 23:57:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2017-05-31 00:53:11]

https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebViewImpl.cpp (left):

https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebViewImpl.cpp:702: gesture_animation_ =
WebActiveGestureAnimation::CreateAtAnimationStart(
Do we want this to be the event creation time or Now()?

It seems that if we have a janky main thread you could end up missing a few
frames of the fling animation. Is that anticipated?

[aelias_OOO_until_Jul13, 2017-05-31 01:07:06]

https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebViewImpl.cpp (left):

https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebViewImpl.cpp:702: gesture_animation_ =
WebActiveGestureAnimation::CreateAtAnimationStart(
On 2017/05/31 at 00:53:11, dtapuska wrote:
> Do we want this to be the event creation time or Now()?
> 
> It seems that if we have a janky main thread you could end up missing a few
frames of the fling animation. Is that anticipated?

My rule of thumb is that in cases of jank during animations, it's best to
eventually recover to the same position as if the jank had never happened
(preserving animation rate).  That argues for using the event timestamp.  This
is consistent with what happens if the main thread is janky after the animation
has already started, and it's also consistent with what impl-thread FlingStart
does (it uses event timestamp, not Now).

[dtapuska, 2017-05-31 01:10:35]

On 2017/05/31 01:07:06, aelias wrote:
>
https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
> File third_party/WebKit/Source/web/WebViewImpl.cpp (left):
> 
>
https://codereview.chromium.org/2908073008/diff/1/third_party/WebKit/Source/w...
> third_party/WebKit/Source/web/WebViewImpl.cpp:702: gesture_animation_ =
> WebActiveGestureAnimation::CreateAtAnimationStart(
> On 2017/05/31 at 00:53:11, dtapuska wrote:
> > Do we want this to be the event creation time or Now()?
> > 
> > It seems that if we have a janky main thread you could end up missing a few
> frames of the fling animation. Is that anticipated?
> 
> My rule of thumb is that in cases of jank during animations, it's best to
> eventually recover to the same position as if the jank had never happened
> (preserving animation rate).  That argues for using the event timestamp.  This
> is consistent with what happens if the main thread is janky after the
animation
> has already started, and it's also consistent with what impl-thread FlingStart
> does (it uses event timestamp, not Now).

lgtm

[aelias_OOO_until_Jul13, 2017-05-31 01:18:09]

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

[aelias_OOO_until_Jul13, 2017-05-31 01:18:09]

+jbroman for OWNERS of dead code deletion in Source/platform

[commit-bot: I haz the power, 2017-05-31 02:40:15]

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

[commit-bot: I haz the power, 2017-05-31 02:40:16]

Dry run: This issue passed the CQ dry run.

[jbroman, 2017-05-31 15:27:23]

lgtm

[aelias_OOO_until_Jul13, 2017-05-31 18:19:34]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2017-05-31 18:20:07]

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-05-31 19:37:29]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496254774898620, "parent_rev":
"b8b79e31c9278183c9c1c1660573d44429e48673", "commit_rev":
"3562f1b7410b884e07bbd2fc75cfee7e96ed2a41"}

[commit-bot: I haz the power, 2017-05-31 19:37:42]

Description was changed from

==========
Remove a frame of delay on main-thread FlingStart.

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964
==========

to

==========
Remove a frame of delay on main-thread FlingStart.

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964

Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#475984}
Committed:
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...
==========

[commit-bot: I haz the power, 2017-05-31 19:37:43]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...

[Peter Kasting, 2017-06-01 03:54:50]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2917823003/ by pkasting@chromium.org.

The reason for reverting is: Possible cause of flakiness in
fast/events/touch/gesture/pad-gesture-fling.html ; see e.g.
https://luci-milo.appspot.com/buildbot/chromium.webkit/WebKit%20Mac10.11%20%2...
..

[aelias_OOO_until_Jul13, 2017-06-01 22:31:59]

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

[aelias_OOO_until_Jul13, 2017-06-01 22:32:20]

Description was changed from

==========
Remove a frame of delay on main-thread FlingStart.

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964

Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#475984}
Committed:
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...
==========

to

==========
Remove a frame of delay on main-thread FlingStart. [2nd land]

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964

Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#475984}
Committed:
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...
==========

[commit-bot: I haz the power, 2017-06-01 22:32:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aelias_OOO_until_Jul13, 2017-06-01 22:48:34]

The CQ bit was unchecked by aelias@chromium.org

[aelias_OOO_until_Jul13, 2017-06-01 22:48:36]

The CQ bit was checked by aelias@chromium.org

[aelias_OOO_until_Jul13, 2017-06-01 22:48:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2908073008/#ps20001
(title: "Fix flake in pad-gesture-fling")

[commit-bot: I haz the power, 2017-06-01 22:49:28]

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-06-02 01:23:18]

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

[commit-bot: I haz the power, 2017-06-02 01:23:20]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[aelias_OOO_until_Jul13, 2017-06-02 01:41:34]

The CQ bit was checked by aelias@chromium.org

[aelias_OOO_until_Jul13, 2017-06-02 01:41:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2908073008/#ps40001
(title: "Fix pad-gesture-fling properlyy")

[commit-bot: I haz the power, 2017-06-02 01:42:08]

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-06-02 04:06:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496367694884270,
"parent_rev": "95de5d0636180ae02584ae1ff55af061ae86b926", "commit_rev":
"8d929084e3659b06f6ba30f334541bdde21866a6"}

[commit-bot: I haz the power, 2017-06-02 04:06:34]

Description was changed from

==========
Remove a frame of delay on main-thread FlingStart. [2nd land]

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964

Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#475984}
Committed:
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...
==========

to

==========
Remove a frame of delay on main-thread FlingStart. [2nd land]

Main-thread flings were waiting for the first frame of animation to
populate the start timestamp, and only on the second frame would they
start animating.  But the start timestamp is included in the FlingStart
event, so there's no need for this (perhaps it was written that way for
historical reasons when the event timestamp wasn't populated properly).

This problem was extremely noticeable in prototype of middle-click autoscroll
that issues a FlingStart on every MouseMove to change velocity, but
the fix should also benefit touchscreen and touchpad flings slightly.

BUG=727964

Review-Url: https://codereview.chromium.org/2908073008
Cr-Original-Commit-Position: refs/heads/master@{#475984}
Committed:
https://chromium.googlesource.com/chromium/src/+/3562f1b7410b884e07bbd2fc75cf...
Review-Url: https://codereview.chromium.org/2908073008
Cr-Commit-Position: refs/heads/master@{#476558}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d929084e3659b06f6ba30f33454...
==========

[commit-bot: I haz the power, 2017-06-02 04:06:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8d929084e3659b06f6ba30f33454...