Correct missing radius in ui::TouchEvent.

Correct missing radius in ui::TouchEvent.

If a ui::TouchEvent is missing a radius on one axis, use the value from the other axis.

BUG=406004
Committed: https://crrev.com/15625947d71cacfefca735fe4d4959e1bbbf221f
Cr-Commit-Position: refs/heads/master@{#311352}

[tdresser, 2015-01-13 15:16:09]

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

[tdresser, 2015-01-13 15:16:09]

As I indicated in the bug, for some reason I was thinking this already happened
in web_input_event_util.cc, and that this issue would vanish when
https://codereview.chromium.org/510793003/ landed.

Clearly that wasn't the case. I think web_input_event_util.cc is a reasonable
place to put this logic, despite the fact that it will currently only be used on
Aura. It makes sense to me for it to live in the event conversion process.

I can move it to a more aura specific location if you think it makes more sense.

[jdduke (slow), 2015-01-13 15:50:26]

https://codereview.chromium.org/822803005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/822803005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/input/web_input_event_util.cc:216: else if
(major_radius == 0)
This shouldn't happen, should it? |major_radius| should always be the larger of
the two?

[tdresser, 2015-01-13 19:22:08]

https://codereview.chromium.org/822803005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/822803005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/input/web_input_event_util.cc:216: else if
(major_radius == 0)
On 2015/01/13 15:50:25, jdduke wrote:
> This shouldn't happen, should it? |major_radius| should always be the larger
of
> the two?

Done.

[jdduke (slow), 2015-01-13 19:31:26]

lgtm, though I'd almost prefer if this were fixed upstream where the underlying
ui::TouchEvents are created =/.

https://codereview.chromium.org/822803005/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/822803005/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/web_input_event_util.cc:214:
DCHECK(minor_radius <= major_radius);
Nit: DCHECK_LE

[tdresser, 2015-01-13 21:04:43]

On 2015/01/13 19:31:26, jdduke wrote:
> lgtm, though I'd almost prefer if this were fixed upstream where the
underlying
> ui::TouchEvents are created =/.
> 
>
https://codereview.chromium.org/822803005/diff/20001/content/browser/renderer...
> File content/browser/renderer_host/input/web_input_event_util.cc (right):
> 
>
https://codereview.chromium.org/822803005/diff/20001/content/browser/renderer...
> content/browser/renderer_host/input/web_input_event_util.cc:214:
> DCHECK(minor_radius <= major_radius);
> Nit: DCHECK_LE

Jared, how does this look to you?
Performing this in the constructor would be kinda ugly, as there are several
constructors, and there are cases where the radius is modified after the fact
(though they aren't common).

[jdduke (slow), 2015-01-13 21:08:28]

https://codereview.chromium.org/822803005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/web_input_event_util.cc (right):

https://codereview.chromium.org/822803005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/web_input_event_util.cc:215:
DCHECK_GT(major_radius, 0);
I'm not sure we can assume that major_radius is non-zero though.

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

https://codereview.chromium.org/822803005/diff/40001/ui/events/event.h#newcod...
ui/events/event.h:524: float radius_y() const { return radius_y_ > 0 ? radius_y_
: radius_x_; }
Hmm, or the cleanup could occur in MotionEventAura, when translating from an x/y
radius to a large/small radius. I dunno, up to you.

[tdresser, 2015-01-13 21:29:54]

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

[tdresser, 2015-01-13 21:29:54]

sadrul, can you take a look at ui/?

[sadrul, 2015-01-13 21:32:42]

lgtm

https://codereview.chromium.org/822803005/diff/60001/ui/events/event_unittest.cc
File ui/events/event_unittest.cc (right):

https://codereview.chromium.org/822803005/diff/60001/ui/events/event_unittest...
ui/events/event_unittest.cc:504: const float nonZeroLength2 = 46;
non_zero_length*

[tdresser, 2015-01-13 21:44:10]

https://codereview.chromium.org/822803005/diff/60001/ui/events/event_unittest.cc
File ui/events/event_unittest.cc (right):

https://codereview.chromium.org/822803005/diff/60001/ui/events/event_unittest...
ui/events/event_unittest.cc:504: const float nonZeroLength2 = 46;
On 2015/01/13 21:32:42, sadrul wrote:
> non_zero_length*

Done.

[tdresser, 2015-01-13 21:44:21]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-01-13 21:45:39]

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

[commit-bot: I haz the power, 2015-01-13 22:37:33]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-13 22:39:02]

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