Disable scroll anchoring if the user engages in a drag motion

Disable scroll anchoring if the user engages in a drag motion

This CL adds a heuristic for disabling scroll anchoring  when a user is 
interacting with a page, specifically clicking and dragging for panning.

With this CL:

Scroll anchoring is disabled if we see a pointermove while a pointer is down.
Scroll anchoring is re-enabled when the scroll offset is changed (via userscroll, 
programmaticscroll, etc)

This logic breaks scroll anchoring in the following cases:
1) Because scroll anchoring is disabled from when we see the first pointermove 
until the next scroll, we don't see its advantages on some pages. For example,
if the the user is panning a map in the viewport and content shifts above them.

2) If the user clicks, drags, and releases the mouse pointer on desktop. Unlike in the 
case of touch, clicking and dragging doesn't cause a scroll. As a result, scroll 
anchoring is not re-enabled and we are left without scroll anchoring until the
next scroll. This could happen for cases involving text selection? Perhaps this is
also a problem on touch.

3) Edge case: Lets say there is page with a map on it with a janky MT. If the 
user scrolls the page (on CC) and starts to interact with the map, we will re-enable
scroll anchoring after the CC scroll offset syncs with MT scroll offset and before the
user stops panning the page, resulting on the original bug.  

BUG=628074

[ymalik, 2016-08-03 21:06:41]

Description was changed from

==========
presubmit


Disable scroll anchoring if the user engages in a drag motion

BUG=
==========

to

==========
Disable scroll anchoring if the user engages in a drag motion

This CL adds a heuristic for disabling scroll anchoring  when a user is 
interacting with a page, specifically clicking and dragging for panning.

With this CL:

Scroll anchoring is disabled if we see a pointermove while a pointer is down.
Scroll anchoring is re-enabled when the scroll offset is changed (via
userscroll, 
programmaticscroll, etc)

This logic breaks scroll anchoring in the following cases:
1) Because scroll anchoring is disabled from when we see the first pointermove 
until the next scroll, we don't see its advantages on some pages. For example,
if the the user is panning a map in the viewport and content shifts above them.

2) If the user clicks, drags, and releases the mouse pointer on desktop. Unlike
in the 
case of touch, clicking and dragging doesn't cause a scroll. As a result, scroll

anchoring is not re-enabled and we are left without scroll anchoring until the
next scroll. This could happen for cases involving text selection? Perhaps this
is
also a problem on touch.

3) Edge case: Lets say there is page with a map on it with a janky MT. If the 
user scrolls the page (on CC) and starts to interact with the map, we will
re-enable
scroll anchoring after the CC scroll offset syncs with MT scroll offset and
before the
user stops panning the page, resulting on the original bug.  

BUG=628074
==========

[ymalik, 2016-08-03 21:11:34]

ymalik@chromium.org changed reviewers:
+ dtapuska@chromium.org, nzolghadr@chromium.org, skobes@chromium.org

[ymalik, 2016-08-03 21:11:34]

Hey guys, 

consider this WIP, subpar solution for the problem in crbug.com/628074. 

Please see the description for more details.

I am starting to think that for touch, we should hook into the gesture path in
GestureManager to enable and disable scroll anchoring.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/input/Ges...


In that case we will need another solution for non-touch.

[skobes, 2016-08-03 23:52:36]

Thanks for the detailed writeup.

I think (1) is an acceptable tradeoff.

For (2) can we check if the drag fired an event listener?  Suppress anchoring if
script handled the drag, but allow it if the drag produced the native browser
behavior (e.g. text selection).

I don't understand (3).  The map interaction requires the main thread; wouldn't
any delta from cc already be applied by the time we dispatch pointermove?

Can you clarify your comment about the gesture path?  How does that compare to
this patch?

[skobes, 2016-08-03 23:59:41]

+ojan fyi

[Navid Zolghadr, 2016-08-04 18:11:00]

https://codereview.chromium.org/2212543002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/2212543002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:162:
m_frame->view()->setUserMayHaveDragged(true);
Maybe for the events that cannot possibly cause scroll you can do this on
pointerdown?
This field (which we haven't added yet and it is unrelated to the change for the
slop region) could be useful in that case. I don't think we have a crbug for
that thought. Just something to keep in mind.