Fix slop region boundary handling.

Fix slop region boundary handling.

Ensure the screenPosition of touches is never truncated to an integer.

Check if the unified gesture detector is enabled before disabling
the inclusive slop region.

BUG=381174
TEST=RenderWidgetHostViewAuraTest.TouchEventPositionsArentRounded

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

[tdresser, 2014-06-10 15:37:59]

Jared, can you take a look?

[jdduke (slow), 2014-06-10 16:01:23]

lgtm with a nit, though I'm not an owner for most of this. In any case, thanks
for taking care of it!

https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
File ui/events/gestures/unified_gesture_detector_enabled.cc (right):

https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
ui/events/gestures/unified_gesture_detector_enabled.cc:24: } else if
(unified_gd_enabled_switch ==
Nit: No need for else if / else with preceding returns.  You could probably just
make the "== ...Disabled" check implicit, removing it and instead checking "!=
...Disabled" at the end for the LOG(ERROR) case, then have the final statement
be "return false".

[tdresser, 2014-06-10 16:41:14]

Sadrul, can you take a look?

https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
File ui/events/gestures/unified_gesture_detector_enabled.cc (right):

https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
ui/events/gestures/unified_gesture_detector_enabled.cc:24: } else if
(unified_gd_enabled_switch ==
On 2014/06/10 16:01:23, jdduke wrote:
> Nit: No need for else if / else with preceding returns.  You could probably
just
> make the "== ...Disabled" check implicit, removing it and instead checking "!=
> ...Disabled" at the end for the LOG(ERROR) case, then have the final statement
> be "return false".

Cleaned up the if / else stuff.
I think the explicit check is easier to follow - I'll change it if you're really
convinced it's superior.

[jdduke (slow), 2014-06-10 16:46:57]

On 2014/06/10 16:41:14, tdresser wrote:
> Sadrul, can you take a look?
> 
>
https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
> File ui/events/gestures/unified_gesture_detector_enabled.cc (right):
> 
>
https://codereview.chromium.org/328733003/diff/1/ui/events/gestures/unified_g...
> ui/events/gestures/unified_gesture_detector_enabled.cc:24: } else if
> (unified_gd_enabled_switch ==
> On 2014/06/10 16:01:23, jdduke wrote:
> > Nit: No need for else if / else with preceding returns.  You could probably
> just
> > make the "== ...Disabled" check implicit, removing it and instead checking
"!=
> > ...Disabled" at the end for the LOG(ERROR) case, then have the final
statement
> > be "return false".
> 
> Cleaned up the if / else stuff.
> I think the explicit check is easier to follow - I'll change it if you're
really
> convinced it's superior.

Eh, I'm not convinced enough to argue about it :) In any case the changes look
just fine (although I typically prefer line breaks after an "if (foo) return",
but there's definitely not anything in the style guide about that, the only
thing I could find is:

http://www.chromium.org/developers/coding-style
"Don't use else after return:
if (foo)
  return 1;
else
  return 2;

if (foo)
  return 1;
return 2;")

[tdresser, 2014-06-10 17:36:44]

Add line breaks after return statements.

[sadrul, 2014-06-11 20:58:13]

lgtm

https://codereview.chromium.org/328733003/diff/60001/content/browser/renderer...
File content/browser/renderer_host/ui_events_helper.cc (right):

https://codereview.chromium.org/328733003/diff/60001/content/browser/renderer...
content/browser/renderer_host/ui_events_helper.cc:317: const gfx::PointF
root_point = event.root_location_f();
&

https://codereview.chromium.org/328733003/diff/60001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/328733003/diff/60001/ui/events/event.h#newcod...
ui/events/event.h:258: gfx::PointF root_location_f() const {
Return const &

https://codereview.chromium.org/328733003/diff/60001/ui/events/gestures/unifi...
File ui/events/gestures/unified_gesture_detector_enabled.cc (right):

https://codereview.chromium.org/328733003/diff/60001/ui/events/gestures/unifi...
ui/events/gestures/unified_gesture_detector_enabled.cc:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
2014 (I wish we had presubmit scripts for this sort of thing)

[tdresser, 2014-06-12 12:45:24]

https://codereview.chromium.org/328733003/diff/60001/content/browser/renderer...
File content/browser/renderer_host/ui_events_helper.cc (right):

https://codereview.chromium.org/328733003/diff/60001/content/browser/renderer...
content/browser/renderer_host/ui_events_helper.cc:317: const gfx::PointF
root_point = event.root_location_f();
On 2014/06/11 20:58:12, sadrul wrote:
> &

Done.

https://codereview.chromium.org/328733003/diff/60001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/328733003/diff/60001/ui/events/event.h#newcod...
ui/events/event.h:258: gfx::PointF root_location_f() const {
On 2014/06/11 20:58:13, sadrul wrote:
> Return const &

Done.

https://codereview.chromium.org/328733003/diff/60001/ui/events/gestures/unifi...
File ui/events/gestures/unified_gesture_detector_enabled.cc (right):

https://codereview.chromium.org/328733003/diff/60001/ui/events/gestures/unifi...
ui/events/gestures/unified_gesture_detector_enabled.cc:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
On 2014/06/11 20:58:13, sadrul wrote:
> 2014 (I wish we had presubmit scripts for this sort of thing)

Thanks, done.

It might be a bit tricky to make the presubmit script handle files which are
moved correctly. We can't detect files moves completely reliably at this point,
can we?

[tdresser, 2014-06-12 12:45:28]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-12 12:55:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/328733003/80001

[commit-bot: I haz the power, 2014-06-12 15:01:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-12 15:03:56]

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

[commit-bot: I haz the power, 2014-06-12 15:03:57]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[tdresser, 2014-06-12 15:04:54]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-12 15:06:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/328733003/80001

[commit-bot: I haz the power, 2014-06-12 15:14:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-12 15:17:42]

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

[commit-bot: I haz the power, 2014-06-12 15:17:44]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[tdresser, 2014-06-12 15:19:55]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-12 15:21:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/328733003/80001

[commit-bot: I haz the power, 2014-06-12 15:28:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-12 15:30:24]

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

[commit-bot: I haz the power, 2014-06-12 15:30:26]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[tdresser, 2014-06-12 15:33:07]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-12 15:34:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/328733003/80001

[commit-bot: I haz the power, 2014-06-12 17:22:59]

Change committed as 276728