Fixed scrolling for mixed-valuator mouse

Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
Committed: https://crrev.com/1517f16eda05f6cbec46cb9efdf806ce7ed2240c
Cr-Commit-Position: refs/heads/master@{#389059}

[Will Shackleton, 2016-04-15 17:10:44]

Description was changed from

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
==========

to

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
==========

[Will Shackleton, 2016-04-15 17:10:44]

w.shackleton@gmail.com changed reviewers:
+ bokan@chromium.org, dtapuska@chromium.org, sadrul@chromium.org

[Will Shackleton, 2016-04-15 17:12:10]

I've tested that scrolling still works with the devices I have, but I don't have
any devices that exhibit the behaviour shown in bug 596013.

[sadrul, 2016-04-19 14:28:24]

https://codereview.chromium.org/1891303002/diff/1/ui/events/x/events_x_utils.cc
File ui/events/x/events_x_utils.cc (right):

https://codereview.chromium.org/1891303002/diff/1/ui/events/x/events_x_utils....
ui/events/x/events_x_utils.cc:630: // block the legacy scroll events for the
necessary axes.
For mouse-press/releases on a normal mouse device, X11 can send both xinput1 and
xinput2 versions of events. But from what I have seen, if chrome listens for
XInput2 events (using XISelectEvents), then chrome does not get the xinput1
version of the events. Is that not the case for these scroll events?

[Will Shackleton, 2016-04-19 18:17:39]

On 2016/04/19 14:28:24, sadrul wrote:
>
https://codereview.chromium.org/1891303002/diff/1/ui/events/x/events_x_utils.cc
> File ui/events/x/events_x_utils.cc (right):
> 
>
https://codereview.chromium.org/1891303002/diff/1/ui/events/x/events_x_utils....
> ui/events/x/events_x_utils.cc:630: // block the legacy scroll events for the
> necessary axes.
> For mouse-press/releases on a normal mouse device, X11 can send both xinput1
and
> xinput2 versions of events. But from what I have seen, if chrome listens for
> XInput2 events (using XISelectEvents), then chrome does not get the xinput1
> version of the events. Is that not the case for these scroll events?

I think (and I'm not sure on this) that XISelectEvents is only for xinput2. Also
we currently don't use XISelectEvents anywhere in DeviceDataManagerX11, and the
only places we do use it we set an umbrella filter for all devices that
specifies all events Chromium may be interested in. While such a modification
could be made it would have to be a separate CL and I'm not sure I feel
confident making such modifications (as it would break code elsewhere that
assumes only the global filter has been set---this assumption is made in the
ChromeOS code for example).

[sadrul, 2016-04-22 02:42:28]

lgtm

[Will Shackleton, 2016-04-22 08:28:50]

The CQ bit was checked by w.shackleton@gmail.com

[commit-bot: I haz the power, 2016-04-22 08:29:02]

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

[commit-bot: I haz the power, 2016-04-22 09:11:40]

Description was changed from

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
==========

to

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
==========

[commit-bot: I haz the power, 2016-04-22 09:11:41]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-22 19:45:38]

Description was changed from

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013
==========

to

==========
Fixed scrolling for mixed-valuator mouse

I fixed scrolling for the case when a mouse only has a scroll valuator for one
of its axes; this is done by first making GetScrollOffsetsFromXEvent only
return true when an actual smooth scroll event is being processed, and secondly
by adding a second check that disables legacy scroll events if the device could
have outputted a valuator event.

BUG=596013

Committed: https://crrev.com/1517f16eda05f6cbec46cb9efdf806ce7ed2240c
Cr-Commit-Position: refs/heads/master@{#389059}
==========

[commit-bot: I haz the power, 2016-04-22 19:45:38]

Patchset 1 (id:??) landed as
https://crrev.com/1517f16eda05f6cbec46cb9efdf806ce7ed2240c
Cr-Commit-Position: refs/heads/master@{#389059}