Remove ui::TouchEvent -> blink::WebTouchEvent conversion methods.

Remove ui::TouchEvent -> blink::WebTouchEvent conversion methods.

Instead, we can just use ui::TouchEvent -> ui::MotionEvent ->
blink::WebInputEvent.

We need ui::TouchEvent -> ui::MotionEvent for gesture detection,
and ui::MotionEvent -> blink::WebInputEvent will be used in
the same way across all platforms supporting touch.

BUG=407645
TEST=RenderWidgetHostViewAuraTest.*

Committed: https://crrev.com/62bb8d3ff2322ed11a11913c3414c3a2f4d4e60f
Cr-Commit-Position: refs/heads/master@{#310514}

[tdresser, 2014-08-28 14:36:33]

https://codereview.chromium.org/510793003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/510793003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:853:
EXPECT_EQ(1U, view_->touch_event_.touchesLength);
These changes are only necessary because the true state is stored in the
MotionEventAura, and the WebInputEvent is only updated as necessary.

[tdresser, 2014-12-11 14:28:32]

On 2014/08/28 14:36:33, tdresser wrote:
>
https://codereview.chromium.org/510793003/diff/1/content/browser/renderer_hos...
> File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/510793003/diff/1/content/browser/renderer_hos...
> content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:853:
> EXPECT_EQ(1U, view_->touch_event_.touchesLength);
> These changes are only necessary because the true state is stored in the
> MotionEventAura, and the WebInputEvent is only updated as necessary.

sadrul@, how does this look?

[tdresser, 2014-12-11 15:03:47]

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

[sadrul, 2014-12-11 15:55:32]

https://codereview.chromium.org/510793003/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/510793003/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.h:523:
blink::WebTouchEvent touch_event_;
We shouldn't need |touch_event_| anymore.

[tdresser, 2014-12-11 21:10:38]

https://codereview.chromium.org/510793003/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/510793003/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.h:523:
blink::WebTouchEvent touch_event_;
On 2014/12/11 15:55:31, sadrul wrote:
> We shouldn't need |touch_event_| anymore.

Done.

[sadrul, 2014-12-11 23:23:31]

LGTM

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
File content/browser/renderer_host/input/web_input_event_util.cc (left):

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
content/browser/renderer_host/input/web_input_event_util.cc:275:
DCHECK_GT(result.touchesLength, 0U);
I suppose you are removing this because the tests code triggers this?

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.h:364: const
ui::MotionEventAura& pointer_state() { return pointer_state_; }
const method

[tdresser, 2015-01-05 21:51:10]

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
File content/browser/renderer_host/input/web_input_event_util.cc (left):

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
content/browser/renderer_host/input/web_input_event_util.cc:275:
DCHECK_GT(result.touchesLength, 0U);
On 2014/12/11 23:23:31, sadrul (OOO) wrote:
> I suppose you are removing this because the tests code triggers this?

Correct.

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.h (right):

https://codereview.chromium.org/510793003/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.h:364: const
ui::MotionEventAura& pointer_state() { return pointer_state_; }
On 2014/12/11 23:23:31, sadrul (OOO) wrote:
> const method

Done.

[tdresser, 2015-01-05 21:55:16]

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

[tdresser, 2015-01-05 21:55:17]

jdduke@, can you take a look at ui/events/gestures/motion_event_aura.cc?

This is a little ugly, and it's a fix for a relatively uncommon issue. It was
actually this DCHECK
(https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...)
that made me realize something terrible was going on. We were throwing out the
first touch sequence after a drag & drop operation.

[jdduke (slow), 2015-01-05 22:29:50]

On 2015/01/05 21:55:17, tdresser wrote:
> jdduke@, can you take a look at ui/events/gestures/motion_event_aura.cc?
> 
> This is a little ugly, and it's a fix for a relatively uncommon issue. It was
> actually this DCHECK
>
(https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...)
> that made me realize something terrible was going on. We were throwing out the
> first touch sequence after a drag & drop operation.

Is there an associated bug for the drag/drop issue?

[jdduke (slow), 2015-01-05 22:35:14]

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/gest...
File ui/events/gestures/gesture_provider_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/gest...
ui/events/gestures/gesture_provider_aura.cc:30: last_unique_touch_event_id_ =
event->unique_event_id();
Should we only update these if |pointer_state_.OnTouch| succeeds?

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:79: // FIXME(tdresser): this should
return false (or NOTREACHED());
Nit: TODO is more common in chromium (vs blink).

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:81: // touch press from a currently used
touch id. See
Will the drag/drop case only occur when the touch id is already active?

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:83: 
No need to include the commented out code.

[tdresser, 2015-01-06 14:20:17]

The associated bug is here: crbug.com/373125.
If we correctly sent a touch end in this case, then the event stream including
the new touch press would be valid.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/gest...
File ui/events/gestures/gesture_provider_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/gest...
ui/events/gestures/gesture_provider_aura.cc:30: last_unique_touch_event_id_ =
event->unique_event_id();
On 2015/01/05 22:35:14, jdduke wrote:
> Should we only update these if |pointer_state_.OnTouch| succeeds?

The latency info should definitely only be set if OnTouch succeeds.

We get a slightly stricter condition on synchronous acks if we always set the
unique_touch_id, but it's probably clearer to only update both fields if OnTouch
succeeds. Done.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:79: // FIXME(tdresser): this should
return false (or NOTREACHED());
On 2015/01/05 22:35:14, jdduke wrote:
> Nit: TODO is more common in chromium (vs blink).

Done.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:81: // touch press from a currently used
touch id. See
On 2015/01/05 22:35:14, jdduke wrote:
> Will the drag/drop case only occur when the touch id is already active?

In the drag and drop case, there are two windows involved, the source window
(from which the drag is started), and the target window (which pops up and is
dragged around).

We get:
- touch press on source
- target pops up, and we transfer the current GestureProvider to the target
window
- touch release on target
- touch press on source

The second touch press on source will fail, because the RWHVAura's
pointer_state_ received the first touch press, but not the touch release.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:83: 
On 2015/01/05 22:35:14, jdduke wrote:
> No need to include the commented out code.

Done.

[jdduke (slow), 2015-01-06 16:49:54]

https://codereview.chromium.org/510793003/diff/140001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_util.cc (left):

https://codereview.chromium.org/510793003/diff/140001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_util.cc:277:
DCHECK_GT(result.touchesLength, 0U);
I'm a little nervous about removing this DCHECK.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:81: // touch press from a currently used
touch id. See
On 2015/01/06 14:20:16, tdresser wrote:
> On 2015/01/05 22:35:14, jdduke wrote:
> > Will the drag/drop case only occur when the touch id is already active?
> 
> In the drag and drop case, there are two windows involved, the source window
> (from which the drag is started), and the target window (which pops up and is
> dragged around).
> 
> We get:
> - touch press on source
> - target pops up, and we transfer the current GestureProvider to the target
> window
> - touch release on target
> - touch press on source
> 
> The second touch press on source will fail, because the RWHVAura's
> pointer_state_ received the first touch press, but not the touch release.

OK, I just want to make sure we don't get into a situation where the new touch
press has a *different* id than the current touch press (the latter never having
received a touch end), resulting in two logical pointers when we really only
have one.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:92: // If this is a touchmove event, and
it isn't different from the last
Nit: This comment is somewhat redundant. Or maybe say that we can safely ignore
stationary events?

https://codereview.chromium.org/510793003/diff/160001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura_unittest.cc (right):

https://codereview.chromium.org/510793003/diff/160001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura_unittest.cc:302:
move1.set_location(gfx::Point(20, 21));
Can we get an additional test or 2 (or 3) covering the additional logic in
MotionEventAura?

[tdresser, 2015-01-07 17:01:15]

https://codereview.chromium.org/510793003/diff/140001/content/browser/rendere...
File content/browser/renderer_host/input/web_input_event_util.cc (left):

https://codereview.chromium.org/510793003/diff/140001/content/browser/rendere...
content/browser/renderer_host/input/web_input_event_util.cc:277:
DCHECK_GT(result.touchesLength, 0U);
On 2015/01/06 16:49:54, jdduke wrote:
> I'm a little nervous about removing this DCHECK.

I've re-added it, at the expense of making the RenderWidgetHostViewAuraTests a
bit uglier.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura.cc (right):

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:81: // touch press from a currently used
touch id. See
On 2015/01/06 16:49:54, jdduke wrote:
> On 2015/01/06 14:20:16, tdresser wrote:
> > On 2015/01/05 22:35:14, jdduke wrote:
> > > Will the drag/drop case only occur when the touch id is already active?
> > 
> > In the drag and drop case, there are two windows involved, the source window
> > (from which the drag is started), and the target window (which pops up and
is
> > dragged around).
> > 
> > We get:
> > - touch press on source
> > - target pops up, and we transfer the current GestureProvider to the target
> > window
> > - touch release on target
> > - touch press on source
> > 
> > The second touch press on source will fail, because the RWHVAura's
> > pointer_state_ received the first touch press, but not the touch release.
> 
> OK, I just want to make sure we don't get into a situation where the new touch
> press has a *different* id than the current touch press (the latter never
having
> received a touch end), resulting in two logical pointers when we really only
> have one.

That can happen, both with this patch, and on master. It's another symptom of
crbug.com/373125.

https://codereview.chromium.org/510793003/diff/140001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura.cc:92: // If this is a touchmove event, and
it isn't different from the last
On 2015/01/06 16:49:54, jdduke wrote:
> Nit: This comment is somewhat redundant. Or maybe say that we can safely
ignore
> stationary events?

Removed comment.

https://codereview.chromium.org/510793003/diff/160001/ui/events/gestures/moti...
File ui/events/gestures/motion_event_aura_unittest.cc (right):

https://codereview.chromium.org/510793003/diff/160001/ui/events/gestures/moti...
ui/events/gestures/motion_event_aura_unittest.cc:302:
move1.set_location(gfx::Point(20, 21));
On 2015/01/06 16:49:54, jdduke wrote:
> Can we get an additional test or 2 (or 3) covering the additional logic in
> MotionEventAura?

Done.

[jdduke (slow), 2015-01-07 17:21:12]

Thanks, motion_event_aura* changes lgtm.

[tdresser, 2015-01-07 17:25:15]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-01-07 17:25:38]

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

[commit-bot: I haz the power, 2015-01-07 18:49:53]

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

[commit-bot: I haz the power, 2015-01-07 18:49:55]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tdresser, 2015-01-08 15:39:01]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-01-08 15:39:27]

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

[commit-bot: I haz the power, 2015-01-08 16:26:59]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-01-08 16:28:15]

Patchset 11 (id:??) landed as
https://crrev.com/62bb8d3ff2322ed11a11913c3414c3a2f4d4e60f
Cr-Commit-Position: refs/heads/master@{#310514}