Change GestureConfiguration types to be consistent with each other and with the

Change GestureConfiguration types to be consistent with each other and with the
types in GestureDetector::Config and other usage points.

Basically:
* Times should always be ints representing milliseconds
* Distances and velocities should always be floats

This eliminates a number of implicit type conversions, some of which trigger a
(currently disabled) MSVC warning due to possible value truncation.

BUG=81439
TEST=none

Committed: https://crrev.com/53aebab88d0b3304a5ba13ff08467fb2e39b674e
Cr-Commit-Position: refs/heads/master@{#299007}

[Peter Kasting, 2014-10-08 01:54:50]

pkasting@chromium.org changed reviewers:
+ rjkroege@chromium.org

[Peter Kasting, 2014-10-08 01:57:00]

Note that this will result in us resetting the stored value of the semi-long
press time, since the pref name has changed.  I assumed this was OK and I didn't
need to write a pref migration.

This also changes gesture_config.js slightly in a way I didn't test (how do you
test this?).

[rjkroege, 2014-10-08 17:44:42]

On 2014/10/08 01:57:00, Peter Kasting wrote:
> Note that this will result in us resetting the stored value of the semi-long
> press time, since the pref name has changed.  I assumed this was OK and I
didn't
> need to write a pref migration.

This seems reasonable to me. Please check with tdresser@ before landing.

> 
> This also changes gesture_config.js slightly in a way I didn't test (how do
you
> test this?).

chrome://gestures (might be missing the s, don't have a Chromebook handy to be
sure) needs to work properly. And the change to gesture_config.js is missing?

[rjkroege, 2014-10-08 17:45:22]

https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
File ui/aura/gestures/gesture_recognizer_unittest.cc (right):

https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:1094: const float kPositionX =
101;
I vaguely recall that there use to be at least one compiler on the complete
waterfall that would complain here without the .f suffix?

[Peter Kasting, 2014-10-08 18:05:53]

pkasting@chromium.org changed reviewers:
+ tdresser@chromium.org

[Peter Kasting, 2014-10-08 18:05:54]

On 2014/10/08 17:44:42, rjkroege wrote:
> On 2014/10/08 01:57:00, Peter Kasting wrote:
> > Note that this will result in us resetting the stored value of the semi-long
> > press time, since the pref name has changed.  I assumed this was OK and I
> didn't
> > need to write a pref migration.
> 
> This seems reasonable to me. Please check with tdresser@ before landing.

tdresser: Can you glance at this too?

> > This also changes gesture_config.js slightly in a way I didn't test (how do
> you
> > test this?).
> 
> chrome://gestures (might be missing the s, don't have a Chromebook handy to be
> sure) needs to work properly.

> And the change to gesture_config.js is missing?

No?:
https://codereview.chromium.org/630003003/diff/1/chrome/browser/resources/ges...

https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
File ui/aura/gestures/gesture_recognizer_unittest.cc (right):

https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:1094: const float kPositionX =
101;
On 2014/10/08 17:45:22, rjkroege wrote:
> I vaguely recall that there use to be at least one compiler on the complete
> waterfall that would complain here without the .f suffix?

You can definitely make compilers complain about something like:

float f = 1.0;

...since that technically initializes a float with a double.  Initializing with
an in-range int, however, should be fine -- we do that many places in our
codebase.

Obviously if some compiler starts whining when I check in, I can fix this :)

[rjkroege, 2014-10-08 18:17:44]

On 2014/10/08 18:05:54, Peter Kasting wrote:
> On 2014/10/08 17:44:42, rjkroege wrote:
> > On 2014/10/08 01:57:00, Peter Kasting wrote:
> > > Note that this will result in us resetting the stored value of the
semi-long
> > > press time, since the pref name has changed.  I assumed this was OK and I
> > didn't
> > > need to write a pref migration.
> > 
> > This seems reasonable to me. Please check with tdresser@ before landing.
> 
> tdresser: Can you glance at this too?
> 
> > > This also changes gesture_config.js slightly in a way I didn't test (how
do
> > you
> > > test this?).
> > 
> > chrome://gestures (might be missing the s, don't have a Chromebook handy to
be
> > sure) needs to work properly.
> 
> > And the change to gesture_config.js is missing?
> 
> No?:
>
https://codereview.chromium.org/630003003/diff/1/chrome/browser/resources/ges...

doh.  sorry I missed that.

lgtm

> 
>
https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
> File ui/aura/gestures/gesture_recognizer_unittest.cc (right):
> 
>
https://codereview.chromium.org/630003003/diff/1/ui/aura/gestures/gesture_rec...
> ui/aura/gestures/gesture_recognizer_unittest.cc:1094: const float kPositionX =
> 101;
> On 2014/10/08 17:45:22, rjkroege wrote:
> > I vaguely recall that there use to be at least one compiler on the complete
> > waterfall that would complain here without the .f suffix?
> 
> You can definitely make compilers complain about something like:
> 
> float f = 1.0;
> 
> ...since that technically initializes a float with a double.  Initializing
with
> an in-range int, however, should be fine -- we do that many places in our
> codebase.
> 
> Obviously if some compiler starts whining when I check in, I can fix this :)

[Peter Kasting, 2014-10-08 18:58:06]

On 2014/10/08 17:44:42, rjkroege wrote:
> On 2014/10/08 01:57:00, Peter Kasting wrote:
> > Note that this will result in us resetting the stored value of the semi-long
> > press time, since the pref name has changed.  I assumed this was OK and I
> didn't
> > need to write a pref migration.
> 
> This seems reasonable to me. Please check with tdresser@ before landing.
> 
> > 
> > This also changes gesture_config.js slightly in a way I didn't test (how do
> you
> > test this?).
> 
> chrome://gestures (might be missing the s, don't have a Chromebook handy to be
> sure) needs to work properly.

I loaded chrome://gesture locally and it displayed the correct "400" value for
the semi-long press time, with the right units.  It let me toggle it up and down
and would reset correctly to 400.  So as far as I can tell that's all working.

[Peter Kasting, 2014-10-08 21:01:34]

pkasting@chromium.org changed reviewers:
+ sky@chromium.org

[Peter Kasting, 2014-10-08 21:01:35]

Patch set 2 just makes a few other minor type-correctness fixes discovered while
checking to make sure my changes didn't generate the sorts of MSVC warnings I
was trying to fix.

+sky for OWNERS for the following files:
    ash\frame\caption_buttons\frame_size_button_unittest.cc
    ash\wm\gestures\long_press_affordance_handler.cc
    chrome\browser\resources\gesture_config.js
    ui\aura\test\aura_test_base.cc
    ui\aura\window_unittest.cc

[sky, 2014-10-08 22:30:52]

LGTM

[tdresser, 2014-10-09 12:27:11]

tdresser@chromium.org changed reviewers:
+ lanwei@chromium.org

[tdresser, 2014-10-09 12:27:12]

You should talk to lanwei@ about the order you'll land this and
https://codereview.chromium.org/613373004/.

Otherwise this LGTM, thanks for doing this.

[Peter Kasting, 2014-10-09 18:12:33]

lanwei, is it OK if I go ahead and land this?  It's going to cause merge
conflicts with your change, so you'll need to update to take all these type
modifications into account, but it looks like your CL doesn't have signoff yet,
so I assume me landing first is better than waiting.

[lanwei, 2014-10-09 18:17:59]

On 2014/10/09 18:12:33, Peter Kasting wrote:
> lanwei, is it OK if I go ahead and land this?  It's going to cause merge
> conflicts with your change, so you'll need to update to take all these type
> modifications into account, but it looks like your CL doesn't have signoff
yet,
> so I assume me landing first is better than waiting.

Sure, please go ahead. I will pull out your code once it is landed and make my
change on top of it.

[Peter Kasting, 2014-10-09 22:03:49]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2014-10-09 22:05:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/630003003/210001

[commit-bot: I haz the power, 2014-10-09 23:27:19]

Committed patchset #5 (id:210001)

[commit-bot: I haz the power, 2014-10-09 23:28:01]

Patchset 5 (id:??) landed as
https://crrev.com/53aebab88d0b3304a5ba13ff08467fb2e39b674e
Cr-Commit-Position: refs/heads/master@{#299007}