New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 727964 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
inactive
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Move middle-click autoscroll to flings

Project Member Reported by aelias@chromium.org, May 30 2017

Issue description

As of https://chromium-review.googlesource.com/502728, we now have fixed-velocity fling curves that are used for gamepad autoscroll.  We should also move all the types of autoscrolls currently handled in AutoscrollController.cpp to the same system.  This will allow them to scroll on the impl-thread, and unify the latching and animation logic.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41

commit 3562f1b7410b884e07bbd2fc75cfee7e96ed2a41
Author: aelias <aelias@chromium.org>
Date: Wed May 31 19:37:27 2017

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}

[modify] https://crrev.com/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41/third_party/WebKit/LayoutTests/fast/scroll-behavior/scroll-customization/touch-scroll-customization.html
[modify] https://crrev.com/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.cpp
[modify] https://crrev.com/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.h
[modify] https://crrev.com/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41/third_party/WebKit/Source/web/WebViewImpl.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c284454162c7b36d587707af42828f41336904f

commit 0c284454162c7b36d587707af42828f41336904f
Author: pkasting <pkasting@chromium.org>
Date: Thu Jun 01 03:55:26 2017

Revert of Remove a frame of delay on main-thread FlingStart. (patchset #1 id:1 of https://codereview.chromium.org/2908073008/ )

Reason for revert:
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%28dbg%29/8992 .

Original issue's description:
> 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/+/3562f1b7410b884e07bbd2fc75cfee7e96ed2a41

TBR=jbroman@chromium.org,dtapuska@chromium.org,aelias@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 727964 

Review-Url: https://codereview.chromium.org/2917823003
Cr-Commit-Position: refs/heads/master@{#476181}

[modify] https://crrev.com/0c284454162c7b36d587707af42828f41336904f/third_party/WebKit/LayoutTests/fast/scroll-behavior/scroll-customization/touch-scroll-customization.html
[modify] https://crrev.com/0c284454162c7b36d587707af42828f41336904f/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.cpp
[modify] https://crrev.com/0c284454162c7b36d587707af42828f41336904f/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.h
[modify] https://crrev.com/0c284454162c7b36d587707af42828f41336904f/third_party/WebKit/Source/web/WebViewImpl.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d929084e3659b06f6ba30f334541bdde21866a6

commit 8d929084e3659b06f6ba30f334541bdde21866a6
Author: aelias <aelias@chromium.org>
Date: Fri Jun 02 04:06:19 2017

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}

[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/LayoutTests/fast/events/touch/gesture/pad-gesture-fling-expected.txt
[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/LayoutTests/fast/events/touch/gesture/pad-gesture-fling.js
[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/LayoutTests/fast/scroll-behavior/scroll-customization/touch-scroll-customization.html
[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.cpp
[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/Source/platform/exported/WebActiveGestureAnimation.h
[modify] https://crrev.com/8d929084e3659b06f6ba30f334541bdde21866a6/third_party/WebKit/Source/web/WebViewImpl.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5971e47dc08dbb773f58fd6fb13b1df7917fd295

commit 5971e47dc08dbb773f58fd6fb13b1df7917fd295
Author: aelias <aelias@chromium.org>
Date: Fri Jun 16 02:39:52 2017

Move middle-click autoscroll to synthetic fling.

Middle-click autoscroll is the Windows feature where after clicking
middle mouse button, the page then scrolls at a velocity depending
on the distance of the mouse cursor from the start point.  It's
one of the few remaining scroll modalities still doing scroller latching
and animation entirely on the Blink main thread.  This patch moves
it to the standard fling animation codepaths, allowing these scrolls
to run on the compositor thread.

Notes:
- Middle-click autoscroll comes in "holding" and "toggle" modes.  This
was implicit in the previous "CanStop" logic: I refactored it into a
more explicit state machine.
- Device-scale-factor correctness seems to have regressed with the
"page zoom for DSF" refactor.  Middle-click autoscroll velocity became
faster and deadzone smaller on higher-DPI.  I brought back DPI normalization.
- I didn't match the exact previous velocity curve as the formula is
quite strange as well as framerate-dependent.  I tweaked values on a
simple exponentiation formula until I found something that felt
similar on DPI=1 and 60fps at high speed, and with more range than before
at low speeds.
- This type of fling is frequently zero-velocity without actually ending
(i.e. preserving the scroller latching), when the mouse cursor is within
the deadzone.  Thus FlingStart([0,0]) events are legal and expected.
- As a next step, I plan to move selection autoscroll and
drag-and-drop autoscroll to the same mechanism.  Then stuff like
autoscroll_layout_object_ and ScheduleMainThreadAnimation will be deleted
entirely.

TEST=fast/events/middleClickAutoscroll-*
BUG= 727964 

Review-Url: https://codereview.chromium.org/2918053002
Cr-Commit-Position: refs/heads/master@{#479931}

[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/common/view_messages.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/renderer/render_widget.cc
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/content/renderer/render_widget.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-click-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-drag-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-event-fired-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-in-iframe-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-nested-divs-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-nested-divs-forbidden-expected.txt
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/middleClickAutoscroll-use-count.html
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/LayoutTests/fast/events/resources/middleClickAutoscroll.js
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/MouseEventManager.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/ScrollManager.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/input/ScrollManager.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/page/AutoscrollController.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/page/AutoscrollController.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/third_party/WebKit/public/web/WebWidgetClient.h
[modify] https://crrev.com/5971e47dc08dbb773f58fd6fb13b1df7917fd295/ui/events/blink/input_handler_proxy.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/955a7967ee52ade9fd54724a9f798a2fb7477ab3

commit 955a7967ee52ade9fd54724a9f798a2fb7477ab3
Author: Alexandre Elias <aelias@chromium.org>
Date: Thu Aug 10 01:34:08 2017

Remove zero-fling-velocity DCHECKs.

As of https://codereview.chromium.org/2918053002, zero-velocity flings
are used and expected to work (because using FlingCancel instead would
lose the scroller latching information from the first fling which must
be preserved for middle-click autoscroll).  But I had forgotten to
verify against a DCHECK-enabled build.  This deletes all three DCHECKs.

BUG= 750774 , 727964 

Change-Id: If60f5f7699cf88d87adc642f8bffb95da2db8a3a
Reviewed-on: https://chromium-review.googlesource.com/607213
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Alexandre Elias <aelias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493215}
[modify] https://crrev.com/955a7967ee52ade9fd54724a9f798a2fb7477ab3/content/browser/renderer_host/input/legacy_input_router_impl.cc
[modify] https://crrev.com/955a7967ee52ade9fd54724a9f798a2fb7477ab3/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/955a7967ee52ade9fd54724a9f798a2fb7477ab3/ui/events/gestures/blink/web_gesture_curve_impl.cc

Comment 6 by aelias@chromium.org, Oct 26 2017

Status: Fixed (was: Assigned)
Summary: Move middle-click autoscroll to flings (was: Move middle-click, selection and drag-and-drop autoscroll to flings)
Impl-thread middle-click autoscroll is shipped but there's more a lot work needed for selection and drag-and-drop autoscroll.  Let's just rename this issue to middle-click and mark it fixed (other cases should be tracked in separate bugs).

Sign in to add a comment