Added split tap to TouchExplorationController

ui/chromeos/touch_exploration_controller_unittest.cc

Index: ui/chromeos/touch_exploration_controller_unittest.cc
diff --git a/ui/chromeos/touch_exploration_controller_unittest.cc b/ui/chromeos/touch_exploration_controller_unittest.cc
index d0e43a535aba0b1c94a47752d23605396540e973..b1d498d136017f6269132de8c3627803ffb802ba 100644
--- a/ui/chromeos/touch_exploration_controller_unittest.cc
+++ b/ui/chromeos/touch_exploration_controller_unittest.cc
@@ -685,7 +685,7 @@ TEST_F(TouchExplorationTest, DoubleTapLongPress) {
   gfx::Point second_tap_location(23, 24);
   generator_->set_current_location(second_tap_location);
   generator_->PressTouch();
-  simulated_clock_->Advance(base::TimeDelta::FromMilliseconds(8000));
+  simulated_clock_->Advance(gesture_detector_config_.longpress_timeout);
   generator_->ReleaseTouch();
 
   const ScopedVector<ui::LocatedEvent>& captured_events = GetCapturedEvents();
@@ -696,9 +696,206 @@ TEST_F(TouchExplorationTest, DoubleTapLongPress) {
   EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
   EXPECT_EQ(tap_location, captured_events[1]->location());
   base::TimeDelta released_time = captured_events[1]->time_stamp();
-  EXPECT_EQ(
-      base::TimeDelta::FromMilliseconds(8000),
-      released_time - pressed_time);
+  EXPECT_EQ(gesture_detector_config_.longpress_timeout,
+            released_time - pressed_time);
 }
 
+// Tapping and releasing with a second finger when in touch exploration mode
+// should send a touch press and released to the location of the last
+// successful touch exploration and return to touch explore.
+TEST_F(TouchExplorationTest, SplitTap) {
+  SwitchTouchExplorationMode(true);
+  gfx::Point tap_location(11, 12);

[aboxhall, 2014/06/13 18:20:01]

Suggestion: initial_touch_location

[evy, 2014/06/13 20:27:39]

Done.

+
+ // Confirm events from other fingers go through as is.

[aboxhall, 2014/06/13 18:20:01]

This comment is a bit confusing as you're not actually asserting anything yet
(and it's at a weird indent).

[evy, 2014/06/13 20:27:39]

Whoops that was a comment from the passthrough code we copied. Gone now.

+  ui::TouchEvent touch0_press(ui::ET_TOUCH_PRESSED, tap_location, 0, Now());
+  ui::TouchEvent touch1_press(

[aboxhall, 2014/06/13 18:20:01]

Consider constructing these just before they're used rather than here.

[evy, 2014/06/13 20:27:39]

Done. This is what Dominic later suggested below, right?

[aboxhall, 2014/06/13 20:52:49]

Basically, yeah.

+      ui::ET_TOUCH_PRESSED, gfx::Point(33, 34), 1, Now());

[aboxhall, 2014/06/13 18:20:00]

Suggest using a variable for this location too, e.g. "second_touch_location"

[evy, 2014/06/13 20:27:39]

For the tapping finger, or the touch explore finger?
We could also say something like "touch_explore_press" and "split_tap_press".
How do you feel about that?

[aboxhall, 2014/06/13 20:52:49]

I meant for the actual location (i.e. the gfx::Point). But giving the
TouchEvents meaningful names would probably also be useful.
For the events, yeah, names like that would be good: touch_explore_press,
split_tap_press and split_tap_release make sense.

[evy, 2014/06/13 22:03:47]

Done.

+  ui::TouchEvent touch1_release(
+      ui::ET_TOUCH_RELEASED, gfx::Point(33, 34), 1, Now());
+
+  // Tap and hold at one location, and get a mouse move event in touch explore.
+  generator_->Dispatch(&touch0_press);
+  AdvanceSimulatedTimePastTapDelay();
+  EXPECT_TRUE(IsInTouchToMouseMode());
+
+  std::vector<ui::LocatedEvent*> events =
+      GetCapturedEventsOfType(ui::ET_MOUSE_MOVED);
+  ASSERT_EQ(1U, events.size());
+
+  EXPECT_EQ(tap_location, events[0]->location());
+  EXPECT_TRUE(events[0]->flags() & ui::EF_IS_SYNTHESIZED);
+  EXPECT_TRUE(events[0]->flags() & ui::EF_TOUCH_ACCESSIBILITY);
+  ClearCapturedEvents();
+
+  // Now tap and release at a different location. This should result in a
+  // single touch and release at the location of the first (held) tap,
+  // not at the location of the second tap and release.
+  // After the release, there is still a finger in touch explore mode.
+  generator_->Dispatch(&touch1_press);
+  generator_->Dispatch(&touch1_release);
+  EXPECT_FALSE(IsInNoFingersDownState());
+
+  const ScopedVector<ui::LocatedEvent>& captured_events = GetCapturedEvents();
+  ASSERT_EQ(2U, captured_events.size());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(tap_location, captured_events[0]->location());
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
+  EXPECT_EQ(tap_location, captured_events[1]->location());
+}
+
+// If split tap is started but the touch explore finger is released first,
+// there should still be a touch press and release sent to the location of
+// the last successful touch exploration.
+// All fingers should be released after the click goes through.
+TEST_F(TouchExplorationTest, SplitTapRelease) {
+  SwitchTouchExplorationMode(true);
+
+  gfx::Point tap_location(11, 12);
+
+ // Confirm events from other fingers go through as is.
+  ui::TouchEvent touch0_press(ui::ET_TOUCH_PRESSED, tap_location, 0, Now());
+  ui::TouchEvent touch1_press(
+      ui::ET_TOUCH_PRESSED, gfx::Point(33, 34), 1, Now());
+  ui::TouchEvent touch0_release(ui::ET_TOUCH_RELEASED, tap_location, 0, Now());
+  ui::TouchEvent touch1_release(
+      ui::ET_TOUCH_RELEASED, gfx::Point(33, 34), 1, Now());
+
+  // Tap and hold at one location, and get a mouse move event in touch explore.
+  generator_->Dispatch(&touch0_press);

[aboxhall, 2014/06/13 18:20:00]

It might be worthwhile pulling the assertions for the first tap into a method on
the test fixture, since it's reused in a few places and isn't the main point of
the test.

[evy, 2014/06/13 20:27:39]

Done. Great idea!

+  AdvanceSimulatedTimePastTapDelay();
+  EXPECT_TRUE(IsInTouchToMouseMode());
+
+  std::vector<ui::LocatedEvent*> events =
+      GetCapturedEventsOfType(ui::ET_MOUSE_MOVED);
+  ASSERT_EQ(1U, events.size());
+
+  EXPECT_EQ(tap_location, events[0]->location());
+  EXPECT_TRUE(events[0]->flags() & ui::EF_IS_SYNTHESIZED);
+  EXPECT_TRUE(events[0]->flags() & ui::EF_TOUCH_ACCESSIBILITY);
+  ClearCapturedEvents();
+
+  // Now tap at a different location. Release at the first location,
+  // then release at the second. This should result in a
+  // single touch and release at the location of the first (held) tap,
+  // not at the location of the second tap and release.
+  generator_->Dispatch(&touch1_press);
+  generator_->Dispatch(&touch0_release);
+  generator_->Dispatch(&touch1_release);
+  EXPECT_TRUE(IsInNoFingersDownState());
+
+  const ScopedVector<ui::LocatedEvent>& captured_events = GetCapturedEvents();
+  ASSERT_EQ(2U, captured_events.size());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(tap_location, captured_events[0]->location());
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
+  EXPECT_EQ(tap_location, captured_events[1]->location());
+}
+
+// When in touch exploration mode, making a long press with a second finger
+// should send a touch press and released to the location of the last
+// successful touch exploration. There should be a delay between the
+// touch and release events (right click).
+TEST_F(TouchExplorationTest, SplitTapLongPress) {
+  SwitchTouchExplorationMode(true);
+  gfx::Point tap_location(11, 12);
+
+ // Confirm events from other fingers go through as is.
+  ui::TouchEvent touch0_press(ui::ET_TOUCH_PRESSED, tap_location, 0, Now());
+  ui::TouchEvent touch1_press(
+      ui::ET_TOUCH_PRESSED, gfx::Point(33, 34), 1, Now());
+  simulated_clock_->Advance(gesture_detector_config_.longpress_timeout);

[aboxhall, 2014/06/13 18:20:01]

This confuses me. Could you also just do Now() +
gesture_detector_config_.longpress_timeout in the constructor call below? 

[dmazzoni, 2014/06/13 18:36:13]

I think that'd be a lot more bookkeeping - you'd have to keep adding that to
every event from this point on.

What's confusing about advancing the time?

[aboxhall, 2014/06/13 18:40:29]

No more events are created after this one, so the bookkeeping issue is moot.

The confusing thing is that later on we call AdvanceSimulatedTimePastTapDelay(),
so the original Now() is really not Now(), it's really Now() - 1000ms -
longpress_timeout. It seems like the simulated time is doing double duty here,
as an initialization value and as a value during the test.

[dmazzoni, 2014/06/13 18:46:06]

Oh! I see what you mean.

I think it'd be more clear if events were dispatched as they're created.

Create the touch0_press, dispatch it.
Then advance the simulated time, to enter touch exploration mode.
Then create the touch1_press, and dispatch it.
Then advance the simulated time again - so we're past the long-press timeout.

...and so on.

[aboxhall, 2014/06/13 18:47:22]

Yeah, that would work well.

[evy, 2014/06/13 20:27:39]

Done. Events dispatched as they're created

[aboxhall, 2014/06/13 20:52:49]

Thanks, this is much easier to follow now.

+  ui::TouchEvent touch1_release(
+      ui::ET_TOUCH_RELEASED, gfx::Point(33, 34), 1, Now());
+
+  // Tap and hold at one location, and get a mouse move event in touch explore.
+  generator_->Dispatch(&touch0_press);
+  AdvanceSimulatedTimePastTapDelay();
+  EXPECT_TRUE(IsInTouchToMouseMode());
+
+  std::vector<ui::LocatedEvent*> events =
+      GetCapturedEventsOfType(ui::ET_MOUSE_MOVED);
+  ASSERT_EQ(1U, events.size());
+
+  EXPECT_EQ(tap_location, events[0]->location());
+  EXPECT_TRUE(events[0]->flags() & ui::EF_IS_SYNTHESIZED);
+  EXPECT_TRUE(events[0]->flags() & ui::EF_TOUCH_ACCESSIBILITY);
+  ClearCapturedEvents();
+
+  // Now tap and release at a different location. This should result in a
+  // single touch and release at the location of the first (held) tap,
+  // not at the location of the second tap and release.
+  // After the release, there is still a finger in touch explore mode.
+  generator_->Dispatch(&touch1_press);
+  generator_->Dispatch(&touch1_release);
+  EXPECT_FALSE(IsInNoFingersDownState());
+
+  const ScopedVector<ui::LocatedEvent>& captured_events = GetCapturedEvents();
+  ASSERT_EQ(2U, captured_events.size());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(tap_location, captured_events[0]->location());
+  base::TimeDelta pressed_time = captured_events[0]->time_stamp();
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
+  EXPECT_EQ(tap_location, captured_events[1]->location());
+  base::TimeDelta released_time = captured_events[1]->time_stamp();
+  EXPECT_EQ(gesture_detector_config_.longpress_timeout,
+            released_time - pressed_time);
+}
+
+// If split tap is started but the touch explore finger is released first,
+// there should still be a touch press and release sent to the location of
+// the last successful touch exploration. If the remaining finger is held
+// as a longpress, there should be a delay between the sent touch and release
+// events (right click).
+// All fingers should be released after the click goes through.
+TEST_F(TouchExplorationTest, SplitTapReleaseLongPress) {
+  SwitchTouchExplorationMode(true);
+  gfx::Point tap_location(11, 12);
+
+ // Confirm events from other fingers go through as is.
+  ui::TouchEvent touch0_press(ui::ET_TOUCH_PRESSED, tap_location, 0, Now());
+  ui::TouchEvent touch1_press(
+      ui::ET_TOUCH_PRESSED, gfx::Point(33, 34), 1, Now());
+  ui::TouchEvent touch0_release(ui::ET_TOUCH_RELEASED, tap_location, 0, Now());
+  simulated_clock_->Advance(gesture_detector_config_.longpress_timeout);
+  ui::TouchEvent touch1_release(
+      ui::ET_TOUCH_RELEASED, gfx::Point(33, 34), 1, Now());
+
+  // Tap and hold at one location, and get a mouse move event in touch explore.
+  generator_->Dispatch(&touch0_press);
+  AdvanceSimulatedTimePastTapDelay();
+  EXPECT_TRUE(IsInTouchToMouseMode());
+
+  std::vector<ui::LocatedEvent*> events =
+      GetCapturedEventsOfType(ui::ET_MOUSE_MOVED);
+  ASSERT_EQ(1U, events.size());
+
+  EXPECT_EQ(tap_location, events[0]->location());
+  EXPECT_TRUE(events[0]->flags() & ui::EF_IS_SYNTHESIZED);
+  EXPECT_TRUE(events[0]->flags() & ui::EF_TOUCH_ACCESSIBILITY);
+  ClearCapturedEvents();
+
+  // Now tap at a different location. Release at the first location,
+  // then release at the second. This should result in a
+  // single touch and release at the location of the first (held) tap,
+  // not at the location of the second tap and release.
+  // After the release, TouchToMouseMode should still be on.
+  generator_->Dispatch(&touch1_press);
+  generator_->Dispatch(&touch0_release);
+  generator_->Dispatch(&touch1_release);
+  EXPECT_TRUE(IsInTouchToMouseMode());
+
+  const ScopedVector<ui::LocatedEvent>& captured_events = GetCapturedEvents();
+  ASSERT_EQ(2U, captured_events.size());
+  EXPECT_EQ(ui::ET_TOUCH_PRESSED, captured_events[0]->type());
+  EXPECT_EQ(tap_location, captured_events[0]->location());
+  base::TimeDelta pressed_time = captured_events[0]->time_stamp();
+  EXPECT_EQ(ui::ET_TOUCH_RELEASED, captured_events[1]->type());
+  EXPECT_EQ(tap_location, captured_events[1]->location());
+  base::TimeDelta released_time = captured_events[1]->time_stamp();
+  EXPECT_EQ(gesture_detector_config_.longpress_timeout,
+            released_time - pressed_time);
+ }
+
+
 }  // namespace ui