[Android] Use DIP coordinates with MotionEventAndroid

[Android] Use DIP coordinates with MotionEventAndroid

With this change, all MotionEvent implementations will use DIP coordinates,
eliminating confusion and making the world a better place.  All assumptions of
integral coordinates in the gesture detection pipeline have been updated
appropriately. 

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

[jdduke (slow), 2014-04-09 01:40:53]

tdresser@: PTAL.  There are many bits here (more than I realized) that will be
very much necessary for Aura, and fractional coordinates in general.

[tdresser, 2014-04-09 14:00:01]

LGTM with nits.

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
File content/browser/renderer_host/input/motion_event_android.cc (right):

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
content/browser/renderer_host/input/motion_event_android.cc:249: GetX(0) /
px_to_dp_,
I guess caching the coordinates in pixels to avoid this double unit conversion
probably isn't worth it...

Maybe add a comment mentioning the double conversion here?

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
File content/browser/renderer_host/input/touch_event_queue.cc (right):

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
content/browser/renderer_host/input/touch_event_queue.cc:25: // slop
determination when using fractional DPI coordinates (assuming the slop
fractional DPI -> fractional DIP

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.cc (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:16: // slop determination when
using fractional DPI coordinates (assuming the slop
fractional DPI -> fractional DIP

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:277: const float distance =
delta_x * delta_x + delta_y * delta_y;
Maybe rename |distance| to |distance_squared| or |distance_square|?

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:288: } else if (scroll_x ||
scroll_y) {
Shouldn't there be some kind of epsilon check here?

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:35: // double tap (in dips).
Maximum distance between the first and second touch downs before it is...

considerd -> considered.

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_provider_unittest.cc:778: // Verify that
movement with the touch slop region does not generate a scroll,
with -> within

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/snap_scroll_controller.cc (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/snap_scroll_controller.cc:17: const float
kSnapChannelDipsPerScreenDip = kMinSnapChannelDistance / 480.f;
Looks good, thanks.

[jdduke (slow), 2014-04-10 22:04:52]

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
File content/browser/renderer_host/input/motion_event_android.cc (right):

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
content/browser/renderer_host/input/motion_event_android.cc:249: GetX(0) /
px_to_dp_,
On 2014/04/09 14:00:01, tdresser wrote:
> I guess caching the coordinates in pixels to avoid this double unit conversion
> probably isn't worth it...
> 
> Maybe add a comment mentioning the double conversion here?

|Cancel()| should be called relatively infrequently (relative to MotionEvent
creation), but it's worth a comment.

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
File content/browser/renderer_host/input/touch_event_queue.cc (right):

https://codereview.chromium.org/220063002/diff/100001/content/browser/rendere...
content/browser/renderer_host/input/touch_event_queue.cc:25: // slop
determination when using fractional DPI coordinates (assuming the slop
On 2014/04/09 14:00:01, tdresser wrote:
> fractional DPI -> fractional DIP

Done.

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.cc (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:16: // slop determination when
using fractional DPI coordinates (assuming the slop
On 2014/04/09 14:00:01, tdresser wrote:
> fractional DPI -> fractional DIP

Done.

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:277: const float distance =
delta_x * delta_x + delta_y * delta_y;
On 2014/04/09 14:00:01, tdresser wrote:
> Maybe rename |distance| to |distance_squared| or |distance_square|?

Done.

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.cc:288: } else if (scroll_x ||
scroll_y) {
On 2014/04/09 14:00:01, tdresser wrote:
> Shouldn't there be some kind of epsilon check here?

Ahh, of course.

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:35: // double tap (in dips).
On 2014/04/09 14:00:01, tdresser wrote:
> Maximum distance between the first and second touch downs before it is...
> 
> considerd -> considered.

Ah, I keep falling into the Texan spelling... ;)

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_provider_unittest.cc:778: // Verify that
movement with the touch slop region does not generate a scroll,
On 2014/04/09 14:00:01, tdresser wrote:
> with -> within

Done.

[tdresser, 2014-04-11 12:18:14]

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/220063002/diff/100001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:35: // double tap (in dips).
On 2014/04/10 22:04:53, jdduke wrote:
> On 2014/04/09 14:00:01, tdresser wrote:
> > Maximum distance between the first and second touch downs before it is...
> > 
> > considerd -> considered.
> 
> Ah, I keep falling into the Texan spelling... ;)

+1

[jdduke (slow), 2014-04-11 15:13:43]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-04-11 15:13:52]

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

[jdduke (slow), 2014-04-11 22:15:49]

The CQ bit was unchecked by jdduke@chromium.org

[jdduke (slow), 2014-04-11 22:22:23]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-04-11 22:24:36]

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

[commit-bot: I haz the power, 2014-04-11 23:28:12]

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

[commit-bot: I haz the power, 2014-04-11 23:28:13]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[jdduke (slow), 2014-04-12 00:09:32]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-04-12 00:10:19]

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

[commit-bot: I haz the power, 2014-04-12 05:51:38]

Change committed as 263493