[Android] Use DIP coordinates for GestureFlingStart events

[Android] Use DIP coordinates for GestureFlingStart events

The OverScroller used to generate fling updates on Android works in pixel space.
Consequently, GestureFlingStart velocities were forwarded also in pixel space.
This exceptional case is unnecessary and confusing, as all other gestures
components are provided as DIPs.  Adopt DIPs for GestureFlingStart, instead
scaling the velocity to pixels when generating the OverScroller object.

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

[jdduke (slow), 2014-02-18 19:53:54]

jamesr@: PTAL.  

I believe r.kasibhatla@samsung.com had a patch that also caches the DPI scale
when a fling is generated.  I've convinced myself that this is low enough risk
to be worth any potentially incorrect side-effects. Flings are of finite
duration, and this should occur *very infrequently.  Even if it does occur, I'm
not sure that Android is setup yet to properly handle the device scale changing
(we seem to be caching the device scale factor in a few places, probably places
we shouldn't, and certainly places more permanent than a transient fling).  If
you prefer I can remove the caching.

[aelias_OOO_until_Jul13, 2014-02-18 19:56:56]

This inconsistency is there for a reason, it's to address
http://crbug.com/176869.  We would need to modify the fling overscroller
implementation to avoid this issue.  Fortunately we're forking it soon so maybe
we can do it then?

[jdduke (slow), 2014-02-18 20:03:08]

On 2014/02/18 19:56:56, aelias wrote:
> This inconsistency is there for a reason, it's to address
> http://crbug.com/176869.  We would need to modify the fling overscroller
> implementation to avoid this issue.  Fortunately we're forking it soon so
maybe
> we can do it then?

Right that's why I rescale to pixels in fling_animation_impl_android.cc before
handing off to OverScroller.java.

[aelias_OOO_until_Jul13, 2014-02-18 20:19:53]

OK, should've looked at the details before commenting, lgtm.

In a future patch, I think we also want to push the deviceScaleFactor as part of
the WebInputEvent to avoid calling GetPrimaryDisplay from a renderer. 
Multiscreen support on Android isn't a thing yet but it probably will be sooner
or later, e.g. in the context of http://crbug.com/310763.  Also if this is
shared with other platforms then it will be even more important.

[jdduke (slow), 2014-02-18 21:17:07]

On 2014/02/18 20:19:53, aelias wrote:
> OK, should've looked at the details before commenting, lgtm.
> 
> In a future patch, I think we also want to push the deviceScaleFactor as part
of
> the WebInputEvent to avoid calling GetPrimaryDisplay from a renderer. 
> Multiscreen support on Android isn't a thing yet but it probably will be
sooner
> or later, e.g. in the context of http://crbug.com/310763.  Also if this is
> shared with other platforms then it will be even more important.

Agreed, filed crbug.com/344604 to track (also crbug.com/344370 to track progress
on porting OverScroller to native).

[jamesr, 2014-02-18 22:40:46]

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
File webkit/child/fling_animator_impl_android.cc (right):

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:55: dpi_scale_ =
gfx::Screen::GetNativeScreen()->GetPrimaryDisplay()
let's not do this. if you want to add a cache for the scale factor put it
somewhere where we have a chance of notifying it on updates

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:115: diff.y() / dpi_scale_);
bad alignment

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:135: current_velocity.y() /
dpi_scale_);
bad alignment

[jdduke (slow), 2014-02-18 22:51:28]

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
File webkit/child/fling_animator_impl_android.cc (right):

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:55: dpi_scale_ =
gfx::Screen::GetNativeScreen()->GetPrimaryDisplay()
On 2014/02/18 22:40:46, jamesr wrote:
> let's not do this. if you want to add a cache for the scale factor put it
> somewhere where we have a chance of notifying it on updates

Done.

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:115: diff.y() / dpi_scale_);
On 2014/02/18 22:40:46, jamesr wrote:
> bad alignment

Done.

https://codereview.chromium.org/170993003/diff/20001/webkit/child/fling_anima...
webkit/child/fling_animator_impl_android.cc:135: current_velocity.y() /
dpi_scale_);
On 2014/02/18 22:40:46, jamesr wrote:
> bad alignment

Done.

[jamesr, 2014-02-18 22:56:31]

lgtm

[jdduke (slow), 2014-02-19 00:22:38]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-19 00:25:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/170993003/180001

[commit-bot: I haz the power, 2014-02-19 02:55:58]

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

[commit-bot: I haz the power, 2014-02-19 02:55:58]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[jdduke (slow), 2014-02-19 03:22:50]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-19 03:23:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/170993003/180001

[commit-bot: I haz the power, 2014-02-19 06:18:47]

Change committed as 251963