Clean up TouchDispositionGestureFilter unit tests.

Clean up TouchDispositionGestureFilter unit tests.

Previously, it was difficult to follow precisely where the location of
a gesture was coming from. This patch clearly separates the location of
synthesized gestures from non-synthesized gestures, and removes the
location from events for which location was unused, making the tests
easier to read.

Committed: https://crrev.com/56321972c69d6b2cfa468d5863d79a22b8369a77
Cr-Commit-Position: refs/heads/master@{#306399}

[tdresser, 2014-11-26 14:08:54]

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

[tdresser, 2014-11-26 14:08:55]

I started doing this as part of writing the eager GR tests, but split it out as
it got pretty big.

I find this makes it significantly easier to follow the tests.

The way that gestures were getting their location from the touch before the
touch which caused them to be dispatched was pretty confusing.

[jdduke (slow), 2014-11-26 15:31:33]

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
File ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc
(right):

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:306:
PressTouchPoint();
Yeah, this is definitely an improvement.

I wonder if it's worth going a little further, as I can't say I'm in love with
the "push some gestures then call this touch function that does something
magical".  What about something like:

SendPacket(PressTouchPoint(), Gestures(FOO, BAR, BAZ));

and

SendTimeoutPacket(Gestures(FOOBAR));

? Might need some fiddling for gestures that need a coordinate, but I believe
only a couple tests use that.

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:637:
EXPECT_EQ(LastSentGestureRawLocation(), gfx::PointF(1, 1) + raw_offset);
Hmm, does this test pass?

[tdresser, 2014-11-27 16:39:40]

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
File ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc
(right):

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:306:
PressTouchPoint();
On 2014/11/26 15:31:33, jdduke wrote:
> Yeah, this is definitely an improvement.
> 
> I wonder if it's worth going a little further, as I can't say I'm in love with
> the "push some gestures then call this touch function that does something
> magical".  What about something like:
> 
> SendPacket(PressTouchPoint(), Gestures(FOO, BAR, BAZ));
> 
> and
> 
> SendTimeoutPacket(Gestures(FOOBAR));
> 
> ? Might need some fiddling for gestures that need a coordinate, but I believe
> only a couple tests use that.

Done.

https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:637:
EXPECT_EQ(LastSentGestureRawLocation(), gfx::PointF(1, 1) + raw_offset);
On 2014/11/26 15:31:33, jdduke wrote:
> Hmm, does this test pass?

It does. The synthesized event gets its co-ordinates from the touch point, which
does have its raw offset set correctly.

[tdresser, 2014-12-02 15:41:28]

On 2014/11/27 16:39:40, tdresser wrote:
>
https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
> File ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
> ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:306:
> PressTouchPoint();
> On 2014/11/26 15:31:33, jdduke wrote:
> > Yeah, this is definitely an improvement.
> > 
> > I wonder if it's worth going a little further, as I can't say I'm in love
with
> > the "push some gestures then call this touch function that does something
> > magical".  What about something like:
> > 
> > SendPacket(PressTouchPoint(), Gestures(FOO, BAR, BAZ));
> > 
> > and
> > 
> > SendTimeoutPacket(Gestures(FOOBAR));
> > 
> > ? Might need some fiddling for gestures that need a coordinate, but I
believe
> > only a couple tests use that.
> 
> Done.
> 
>
https://codereview.chromium.org/753743006/diff/1/ui/events/gesture_detection/...
> ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:637:
> EXPECT_EQ(LastSentGestureRawLocation(), gfx::PointF(1, 1) + raw_offset);
> On 2014/11/26 15:31:33, jdduke wrote:
> > Hmm, does this test pass?
> 
> It does. The synthesized event gets its co-ordinates from the touch point,
which
> does have its raw offset set correctly.

Ping.

[jdduke (slow), 2014-12-02 16:00:05]

Sorry for the delay, lgtm.

https://codereview.chromium.org/753743006/diff/20001/ui/events/gesture_detect...
File ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc
(right):

https://codereview.chromium.org/753743006/diff/20001/ui/events/gesture_detect...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:74:
GestureList Gestures() { return GestureList(0); }
Hmm, what about calling this |NoGestures()|?

[tdresser, 2014-12-02 16:09:20]

https://codereview.chromium.org/753743006/diff/20001/ui/events/gesture_detect...
File ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc
(right):

https://codereview.chromium.org/753743006/diff/20001/ui/events/gesture_detect...
ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc:74:
GestureList Gestures() { return GestureList(0); }
On 2014/12/02 16:00:05, jdduke wrote:
> Hmm, what about calling this |NoGestures()|?

Done.

[tdresser, 2014-12-02 16:09:24]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-12-02 16:10:30]

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

[commit-bot: I haz the power, 2014-12-02 16:49:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-02 16:50:49]

Patchset 3 (id:??) landed as
https://crrev.com/56321972c69d6b2cfa468d5863d79a22b8369a77
Cr-Commit-Position: refs/heads/master@{#306399}