Do not scale ScrollEvent by device scale factor

Do not scale ScrollEvent by device scale factor

We used to scale scroll events in event_transformation_handler
because we want to mimic the behaviour of the CrOS X patch which
scales pointer motions w.r.t. the screen resolution. See the
patch here https://chromium-review.googlesource.com/#/c/45522/.
The idea is actually wrong because Aura Views use DIPs instead of
pixels as what X server does. The scaling would make scroll events
to have 2x higher speed on Pixel as compared to normal resolution
devices. This CL fixes the problem so that scroll events will
have identical sensitivity on all devices regardless of the screen
resolution.

Note that the sensitivity would still be slightly different due to
the sllightly different DIP resolution on each device.

Contributed by sheckylin@chromium.org

BUG=288211
TEST=Tested on Pixel and non-Pixel devices. Make sure:
1. Scrolling on webpages with touchpads goes through roughly the same
amount of page if the scrolling distance and speed is the same.
2. 2f back/forward gesture should require roughly equal distance
of scrolling on the touchpad at the same speed to trigger.
3. 3f tab-switching should go through roughly the same ratio of tab
counts for the equal distance of scrolling on the touchpad at the same
speed.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224341

[Shecky Lin, 2013-09-18 08:01:59]

Sadrul/Dave:

This is to make sure scroll events to have identical sensitivity on all devices.
Please let me know how you think about it. Thanks.

oshima:
I would need your OWNER review once Sadrul/Dave are happy with it. Thanks.

[sadrul, 2013-09-18 15:36:40]

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
File ash/display/event_transformation_handler.cc (right):

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
ash/display/event_transformation_handler.cc:61: // gesture and 3f tab switching
would have equal sensitivity).
Does 'It is not necessary to scale the event for the device scale factor since
the event location etc. are already in DIP.' effectively summarize this long
comment?

[DaveMoore, 2013-09-18 15:45:09]

On 2013/09/18 15:36:40, sadrul wrote:
>
https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
> File ash/display/event_transformation_handler.cc (right):
> 
>
https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
> ash/display/event_transformation_handler.cc:61: // gesture and 3f tab
switching
> would have equal sensitivity).
> Does 'It is not necessary to scale the event for the device scale factor since
> the event location etc. are already in DIP.' effectively summarize this long
> comment?

Agreed. Comments shouldn't be about what's not there (Originally...)

[oshima, 2013-09-18 16:26:10]

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
File ash/display/event_transformation_handler.cc (right):

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
ash/display/event_transformation_handler.cc:61: // gesture and 3f tab switching
would have equal sensitivity).
On 2013/09/18 15:36:41, sadrul wrote:
> Does 'It is not necessary to scale the event for the device scale factor since
> the event location etc. are already in DIP.' effectively summarize this long
> comment?

Agreed.

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
ash/display/event_transformation_handler.cc:71: scale *= kBoostForNonIntegrated;
if (display.Internal())
   event->Scale(kBoostForNonIntegrated);

and nuke scale variable.

[Shecky Lin, 2013-09-20 04:44:10]

Thanks guys! I have addressed your comments. Let's get this in.

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
File ash/display/event_transformation_handler.cc (right):

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
ash/display/event_transformation_handler.cc:61: // gesture and 3f tab switching
would have equal sensitivity).
On 2013/09/18 16:26:10, oshima wrote:
> On 2013/09/18 15:36:41, sadrul wrote:
> > Does 'It is not necessary to scale the event for the device scale factor
since
> > the event location etc. are already in DIP.' effectively summarize this long
> > comment?
> 
> Agreed.

Done.

https://codereview.chromium.org/23452037/diff/1/ash/display/event_transformat...
ash/display/event_transformation_handler.cc:71: scale *= kBoostForNonIntegrated;
On 2013/09/18 16:26:10, oshima wrote:
> if (display.Internal())
>    event->Scale(kBoostForNonIntegrated);
> 
> and nuke scale variable.

Done.

[sadrul, 2013-09-20 04:48:52]

LGTM with the comment updated

https://codereview.chromium.org/23452037/diff/8001/ash/display/event_transfor...
File ash/display/event_transformation_handler.cc (right):

https://codereview.chromium.org/23452037/diff/8001/ash/display/event_transfor...
ash/display/event_transformation_handler.cc:45: // in DIP.
Remove 'as in the CrOS X server patch'. i.e.

'It is unnecessary to scale the event for the device scale factor
since the event locations etc. are already in DIP.'

is sufficient.

[oshima, 2013-09-20 04:49:55]

lgtm once you addressed sadrul comment.

[Shecky Lin, 2013-09-20 05:02:00]

Great thanks for light-speed response!

https://codereview.chromium.org/23452037/diff/8001/ash/display/event_transfor...
File ash/display/event_transformation_handler.cc (right):

https://codereview.chromium.org/23452037/diff/8001/ash/display/event_transfor...
ash/display/event_transformation_handler.cc:45: // in DIP.
On 2013/09/20 04:48:53, sadrul wrote:
> Remove 'as in the CrOS X server patch'. i.e.
> 
> 'It is unnecessary to scale the event for the device scale factor
> since the event locations etc. are already in DIP.'
> 
> is sufficient.

Done.

[commit-bot: I haz the power, 2013-09-20 05:03:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sheckylin@chromium.org/23452037/9002

[commit-bot: I haz the power, 2013-09-20 08:58:09]

Change committed as 224341