Added multi-finger gestures to touch_exploration_controller

ui/chromeos/touch_exploration_controller.cc

Index: ui/chromeos/touch_exploration_controller.cc
diff --git a/ui/chromeos/touch_exploration_controller.cc b/ui/chromeos/touch_exploration_controller.cc
index 6b2584afc7f2afc4c3d6bec3c3491e1fb11ae054..23ad1571f31a400243c69096d94a92cfa5067925 100644
--- a/ui/chromeos/touch_exploration_controller.cc
+++ b/ui/chromeos/touch_exploration_controller.cc
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/time/default_tick_clock.h"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ -42,6 +43,7 @@ TouchExplorationController::TouchExplorationController(
       tick_clock_(NULL) {
   CHECK(root_window);
   root_window->GetHost()->GetEventSource()->AddEventRewriter(this);
+  InitializeSwipeGestureMaps();
 }
 
 TouchExplorationController::~TouchExplorationController() {
@@ -52,6 +54,13 @@ ui::EventRewriteStatus TouchExplorationController::RewriteEvent(
     const ui::Event& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   if (!event.IsTouchEvent()) {
+    if (event.IsKeyEvent()) {
+      const ui::KeyEvent& key_event = static_cast<const ui::KeyEvent&>(event);
+      VLOG(0) << "\nKeyboard event: " << key_event.name()
+              << "\n Key code: " << key_event.key_code()
+              << ", Flags: " << key_event.flags()
+              << ", Is char: " << key_event.is_char();
+    }
     return ui::EVENT_REWRITE_CONTINUE;
   }
   const ui::TouchEvent& touch_event = static_cast<const ui::TouchEvent&>(event);
@@ -73,14 +82,22 @@ ui::EventRewriteStatus TouchExplorationController::RewriteEvent(
   const gfx::PointF& location = touch_event.location_f();
   const int touch_id = touch_event.touch_id();
 
+  // Always process gesture events in the gesture provider. After a swipe

[mfomitchev, 2014/08/06 16:11:49]

Are you sure you want to do gesture recognition before updating
current_touch_ids_? Is there a good reason to? This breaks the pattern...

[evy, 2014/08/06 18:46:11]

We wanted to process every event, and checking the ids sometimes returns from
RewriteEvent.
This will probably be changed though, from your comments. See my question two
comments down.

[mfomitchev, 2014/08/07 18:34:58]

EVENT_REWRITE_CONTINUE should only be returned in that code if the controller
was activated while there were fingers down. You wouldn't want events from those
fingers go through gesture recognizer anyway, since it wasn't around to receive
the PRESS events for them, so it would just ignore move/release from them (in
the best case) or would end up i 
could end up in a bad state (in the worst case).

I am not so sure about FindEdgesWithinBounds() stuff - not quite sure what is
going on there...

[evy, 2014/08/07 20:40:26]

which EVENT_REWRITE_CONTINUE are you talking about?
The FindEdgesWithinBounds() stuff was written before we started, and I'm not
quite sure what the logic behind it is - I'll make a note to ask Dominic next
week and get back to you.

[aboxhall, 2014/08/07 20:53:21]

IIRC, lisayin@ wrote the FindEdgesWithinBounds logic. It's pretty
straightforward: it checks whether it's within |bounds| DIPs of one or more
edges, and returns a bitmap indicating which edges it's near.

I'm sure she'd be able to explain in more detail if you had any further
questions.

[lisayin, 2014/08/07 21:11:35]

There's some conversion to DIPS within the actual implementation details because
event locations aren't in DIPS but the window dimensions are, but other than
that it's pretty much exactly what Alice described it to be.
FindEdgesWithinBounds takes a location and determines if the location is within
the given bounds of an edge. It shouldn't affect Evy's code though. In this
iteration, bounds are to keep people from being able to make a single tap by
moving off the screen and back onto the screen without lifting their finger.

On 2014/08/07 20:53:21, aboxhall wrote:

[mfomitchev, 2014/08/08 20:23:13]

@evy: You said "checking the ids sometimes returns from
RewriteEvent". There are two returns, they return EVENT_REWRITE_CONTINUE

FindEdgesWithinBounds stuff:
Ok, so I think it would be cleaner to move FindEdgesWithinBounds logic out of
that code block. 

Logically that entire code block was supposed to be for updating
current_touch_ids_. It's easy to reason about it that way - whether it needs to
be before or after gesture detection, etc. It should probably even be moved to a
separate method. If we insert FindEdgesWithinBounds in there,  that block
becomes harder to reason about. 

+  // gesture is processed, gesture events will continue to be sent to the
+  // gesture provider but will not cause key events to be released again
+  // until after the user resets to no fingers down.
+  gesture_provider_.OnTouchEvent(touch_event);

[mfomitchev, 2014/08/06 16:11:49]

Glad to see you are using GestureProviderAura for this.

[evy, 2014/08/06 18:46:12]

Acknowledged.

+  gesture_provider_.OnTouchEventAck(false);
+  ProcessGestureEvents();

[mfomitchev, 2014/08/06 16:11:49]

Seems like you only want to call ProcessGestureEvents() in GESTURE_IN_PROGRESS
state? 

Also, this entire block of code with gesture evaluation can probably be skipped
in certain states like TOUCH_EXPLORATION. You can just create a new instance of
the gesture provider if you need it again or when all fingers are lifted. Less
state updating this way, so seems like it could be worth it.

[mfomitchev, 2014/08/06 16:11:49]

Would it be worth making this return a bool for consumed/not consumed, for state
update/validation or an early exit?

[evy, 2014/08/06 18:46:12]

I put it here because if I only updated it when in GESTURE_IN_PROGRESS state the
finger ids wouldn't reset, and if I did a 3 finger gesture then a 1 finger
gesture, the 1 finger was processed as a 3 finger gesture.
From what you've said here, it seems like we could just keep the same instance
of the gesture provider, and process gesture events in GESTURE_IN_PROGRESS and
then again in ResetToNoFingersDown - would this work?

[evy, 2014/08/06 21:31:56]

This is now right before the events are sent to be rewritten, and only happens
if the user is in a state that might lead to gestures.
I also reset the provider in ResetToNoFingersDown

+
   // Always update touch ids and touch locations, so we can use those
   // no matter what state we're in.
   if (type == ui::ET_TOUCH_PRESSED) {
     current_touch_ids_.push_back(touch_id);
     touch_locations_.insert(std::pair<int, gfx::PointF>(touch_id, location));
   } else if (type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED) {
-    // In order to avoid accidentally double tapping when moving off the edge of
-    // the screen, the state will be rewritten to NoFingersDown.
+    // In order to avoid accidentally double tapping when moving off the edge
+    // of the screen, the state will be rewritten to NoFingersDown.
     TouchEvent touch_event = static_cast<const TouchEvent&>(event);
     if (FindEdgesWithinBounds(touch_event.location(), kLeavingScreenEdge) !=
         NO_EDGE) {
@@ -111,7 +128,7 @@ ui::EventRewriteStatus TouchExplorationController::RewriteEvent(
   VLOG_STATE();
   VLOG_EVENT(touch_event);
   // The rest of the processing depends on what state we're in.
-  switch(state_) {
+  switch (state_) {
     case NO_FINGERS_DOWN:
       return InNoFingersDown(touch_event, rewritten_event);
     case SINGLE_TAP_PRESSED:
@@ -129,12 +146,12 @@ ui::EventRewriteStatus TouchExplorationController::RewriteEvent(
       return InGestureInProgress(touch_event, rewritten_event);
     case TOUCH_EXPLORE_SECOND_PRESS:
       return InTouchExploreSecondPress(touch_event, rewritten_event);
-    case SLIDE_GESTURE:
-      return InSlideGesture(touch_event, rewritten_event);
     case ONE_FINGER_PASSTHROUGH:
       return InOneFingerPassthrough(touch_event, rewritten_event);
-    case WAIT_FOR_ONE_FINGER:
-      return InWaitForOneFinger(touch_event, rewritten_event);
+    case WAIT_FOR_NO_FINGERS:
+      return InWaitForNoFingers(touch_event, rewritten_event);
+    case SLIDE_GESTURE:
+      return InSlideGesture(touch_event, rewritten_event);
   }
   NOTREACHED();
   return ui::EVENT_REWRITE_CONTINUE;
@@ -153,9 +170,6 @@ ui::EventRewriteStatus TouchExplorationController::InNoFingersDown(
     initial_press_.reset(new TouchEvent(event));
     last_unused_finger_event_.reset(new TouchEvent(event));
     StartTapTimer();
-    gesture_provider_.OnTouchEvent(event);
-    gesture_provider_.OnTouchEventAck(false);
-    ProcessGestureEvents();
     state_ = SINGLE_TAP_PRESSED;
     VLOG_STATE();
     return ui::EVENT_REWRITE_DISCARD;
@@ -169,10 +183,10 @@ ui::EventRewriteStatus TouchExplorationController::InSingleTapPressed(
   const ui::EventType type = event.type();
 
   if (type == ui::ET_TOUCH_PRESSED) {
-    // TODO (evy, lisayin) : add support for multifinger swipes.
-    // For now, we wait for there to be only one finger down again.
-    state_ = WAIT_FOR_ONE_FINGER;
-    return EVENT_REWRITE_DISCARD;
+    // This is the start of a multifinger gesture.
+    state_ = GESTURE_IN_PROGRESS;
+    VLOG_STATE();
+    return InGestureInProgress(event, rewritten_event);

[mfomitchev, 2014/08/06 16:11:49]

Should we consider making 2-finger interactions subject to the same
velocity/slop evaluation as one-finger interaction? It seems like currently the
user cannot  simply put 2 fingers down and start scrolling - they need to wait
for the timeout to enter TE mode first?

[evy, 2014/08/06 18:46:12]

Actually, two fingers are only used for gestures now.
From our survey feedback, users prefer multifinger gestures to passthrough mode.
(we have passthrough implemented with a double tap hold, and also a corner
passthrough mode Lisa will be committing soon)

When the tap timer times out, the user just leaves gesture mode and enters a
wait state for all the fingers to be lifted again.

On a slightly related note, we're going to be adding that two fingers held stops
the voice from speaking, so soon there will actually be a timer/slop check here
soon.

[mfomitchev, 2014/08/07 18:34:58]

Ah, okay, this clears things up, thanks.

   } else if (type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED) {
     if (current_touch_ids_.size() == 0 &&
         event.touch_id() == initial_press_->touch_id()) {
@@ -228,10 +242,11 @@ TouchExplorationController::InSingleTapOrTouchExploreReleased(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   const ui::EventType type = event.type();
-  // If there is more than one finger down, then discard to wait until only one
-  // finger is or no fingers are down.
+  // If there is more than one finger down, then discard to wait until no
+  // fingers are down.
   if (current_touch_ids_.size() > 1) {
-    state_ = WAIT_FOR_ONE_FINGER;
+    state_ = WAIT_FOR_NO_FINGERS;
+    VLOG_STATE();
     return ui::EVENT_REWRITE_DISCARD;
   }
   if (type == ui::ET_TOUCH_PRESSED) {
@@ -246,6 +261,9 @@ TouchExplorationController::InSingleTapOrTouchExploreReleased(
     // release at the location of the last touch exploration.
     state_ = DOUBLE_TAP_PENDING;
     VLOG_STATE();
+    // The old tap timer (from the initial click) is stopped if it is still
+    // going, and the new one is set.
+    tap_timer_.Stop();
     StartTapTimer();
     // This will update as the finger moves before a possible passthrough, and
     // will determine the offset.
@@ -334,8 +352,7 @@ ui::EventRewriteStatus TouchExplorationController::InTouchExploration(
   if (type == ui::ET_TOUCH_PRESSED) {
     // Handle split-tap.
     initial_press_.reset(new TouchEvent(event));
-    if (tap_timer_.IsRunning())
-      tap_timer_.Stop();
+    tap_timer_.Stop();
     rewritten_event->reset(
         new ui::TouchEvent(ui::ET_TOUCH_PRESSED,
                            last_touch_exploration_->location(),
@@ -365,32 +382,18 @@ ui::EventRewriteStatus TouchExplorationController::InGestureInProgress(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   ui::EventType type = event.type();
-  // If additional fingers are added before a swipe gesture has been
-  // registered, then the state will no longer be GESTURE_IN_PROGRESS.
-  if (type == ui::ET_TOUCH_PRESSED ||
-      event.touch_id() != initial_press_->touch_id()) {
-    if (tap_timer_.IsRunning())
-      tap_timer_.Stop();
-    // Discard any pending gestures.
-    delete gesture_provider_.GetAndResetPendingGestures();
-    state_ = WAIT_FOR_ONE_FINGER;
-    return EVENT_REWRITE_DISCARD;
-  }
-
-  // There should not be more than one finger down.
-  DCHECK(current_touch_ids_.size() <= 1);
-  if (type == ui::ET_TOUCH_MOVED) {
-    gesture_provider_.OnTouchEvent(event);
-    gesture_provider_.OnTouchEventAck(false);
+  if (type != ui::ET_TOUCH_RELEASED && type != ui::ET_TOUCH_CANCELLED &&

[mfomitchev, 2014/08/06 16:11:49]

This is done in a bunch of different places in slightly different ways. Some
In<State> methods are missing this check though, e.g. InTouchExploration. I'd
just ditch this check altogether or perhaps move to RewriteEvent. If you really
want to keep it for some reason - I'd make a method out of it and call it
everywhere consistently. It doesn't have to return anything - it can just do a
bunch DCHECK(), since we really don't expect it to ever evaluate to false.

[evy, 2014/08/06 18:46:12]

InTouchExploration checks each of them separately - a few functions check at the
top if it handles all events the same. If other touch events are ever added,
it'd be good to have the NOTREACHED() catch it. I think I'll move the check to
RewriteEvent - thanks for that idea.

+        type != ui::ET_TOUCH_MOVED && type != ui::ET_TOUCH_PRESSED) {
+    NOTREACHED() << "Unexpected event type received: " << event.name();
+    return ui::EVENT_REWRITE_CONTINUE;
   }
-  if (type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED) {
-    gesture_provider_.OnTouchEvent(event);
-    gesture_provider_.OnTouchEventAck(false);
-    if (current_touch_ids_.size() == 0) {
-      ResetToNoFingersDown();
-    }
+  // The events were sent to the gesture provider in RewriteEvent already.
+  // If no gesture is registered before the tap timer times out, the state
+  // will change to wait for no fingers down and this function will stop
+  // being called.
+  if (current_touch_ids_.size() == 0) {
+    ResetToNoFingersDown();
   }
-  ProcessGestureEvents();
   return ui::EVENT_REWRITE_DISCARD;
 }
 
@@ -398,9 +401,8 @@ ui::EventRewriteStatus TouchExplorationController::InOneFingerPassthrough(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   ui::EventType type = event.type();
-
-  if (!(type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED ||
-        type == ui::ET_TOUCH_MOVED || type == ui::ET_TOUCH_PRESSED)) {
+  if (type != ui::ET_TOUCH_RELEASED && type != ui::ET_TOUCH_CANCELLED &&
+      type != ui::ET_TOUCH_MOVED && type != ui::ET_TOUCH_PRESSED) {
     NOTREACHED() << "Unexpected event type received: " << event.name();
     return ui::EVENT_REWRITE_CONTINUE;
   }
@@ -466,7 +468,7 @@ ui::EventRewriteStatus TouchExplorationController::InTouchExploreSecondPress(
   return ui::EVENT_REWRITE_CONTINUE;
 }
 
-ui::EventRewriteStatus TouchExplorationController::InWaitForOneFinger(
+ui::EventRewriteStatus TouchExplorationController::InWaitForNoFingers(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   ui::EventType type = event.type();
@@ -475,14 +477,10 @@ ui::EventRewriteStatus TouchExplorationController::InWaitForOneFinger(
     NOTREACHED() << "Unexpected event type received: " << event.name();
     return ui::EVENT_REWRITE_CONTINUE;
   }
-  if (current_touch_ids_.size() == 1) {
-    EnterTouchToMouseMode();
-    state_ = TOUCH_EXPLORATION;
-    VLOG_STATE();
-    *rewritten_event = CreateMouseMoveEvent(event.location(), event.flags());
-    last_touch_exploration_.reset(new TouchEvent(event));
-    return ui::EVENT_REWRITE_REWRITTEN;
-  }
+
+  if (current_touch_ids_.size() == 0)
+    ResetToNoFingersDown();
+
   return EVENT_REWRITE_DISCARD;
 }
 
@@ -494,8 +492,7 @@ ui::EventRewriteStatus TouchExplorationController::InSlideGesture(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   // The timer should not fire when sliding.
-  if (tap_timer_.IsRunning())
-    tap_timer_.Stop();
+  tap_timer_.Stop();
 
   ui::EventType type = event.type();
   // If additional fingers are added before a swipe gesture has been registered,
@@ -506,10 +503,15 @@ ui::EventRewriteStatus TouchExplorationController::InSlideGesture(
       sound_timer_.Stop();
     // Discard any pending gestures.
     delete gesture_provider_.GetAndResetPendingGestures();
-    state_ = WAIT_FOR_ONE_FINGER;
+    state_ = WAIT_FOR_NO_FINGERS;
+    VLOG_STATE();
     return EVENT_REWRITE_DISCARD;
   }
 
+  // There should not be more than one finger down.
+  DCHECK(current_touch_ids_.size() <= 1);
+
+
   // Allows user to return to the edge to adjust the sound if they have left the
   // boundaries.
   int edge = FindEdgesWithinBounds(event.location(), kSlopDistanceFromEdge);
@@ -530,23 +532,10 @@ ui::EventRewriteStatus TouchExplorationController::InSlideGesture(
     delegate_->PlayVolumeAdjustSound();
   }
 
-  // There should not be more than one finger down.
-  DCHECK(current_touch_ids_.size() <= 1);
-  if (type == ui::ET_TOUCH_MOVED) {
-    gesture_provider_.OnTouchEvent(event);
-    gesture_provider_.OnTouchEventAck(false);
-  }
-  if (type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED) {
-    gesture_provider_.OnTouchEvent(event);
-    gesture_provider_.OnTouchEventAck(false);
-    delete gesture_provider_.GetAndResetPendingGestures();
-    if (current_touch_ids_.size() == 0) {
-      ResetToNoFingersDown();
-    }
-    return ui::EVENT_REWRITE_DISCARD;
+  if (current_touch_ids_.size() == 0) {
+    ResetToNoFingersDown();
   }
 
-  ProcessGestureEvents();
   return ui::EVENT_REWRITE_DISCARD;
 }
 
@@ -590,13 +579,23 @@ void TouchExplorationController::OnTapTimerFired() {
       return;
     }
     case SINGLE_TAP_PRESSED:
+      EnterTouchToMouseMode();
+      state_ = TOUCH_EXPLORATION;
+      break;
     case GESTURE_IN_PROGRESS:
       // Discard any pending gestures.
       delete gesture_provider_.GetAndResetPendingGestures();
-      EnterTouchToMouseMode();
-      state_ = TOUCH_EXPLORATION;
+      // If only one finger is down, go into touch exploration.
+      if (current_touch_ids_.size() == 1) {
+        EnterTouchToMouseMode();
+        state_ = TOUCH_EXPLORATION;
+        VLOG_STATE();
+        return;
+      }
+      // Otherwise wait for all fingers to be lifted.
+      state_ = WAIT_FOR_NO_FINGERS;
       VLOG_STATE();
-      break;
+      return;
     default:
       return;
   }
@@ -619,13 +618,14 @@ void TouchExplorationController::OnGestureEvent(
     ui::GestureEvent* gesture) {
   CHECK(gesture->IsGestureEvent());
   ui::EventType type = gesture->type();
-  if (VLOG_on_)
-    VLOG(0) << " \n Gesture Triggered: " << gesture->name();
-  if (type == ui::ET_GESTURE_SWIPE && state_ != SLIDE_GESTURE) {
-    if (VLOG_on_)
-      VLOG(0) << "Swipe!";
+  if (type == ui::ET_GESTURE_SWIPE && state_ == GESTURE_IN_PROGRESS) {
     delete gesture_provider_.GetAndResetPendingGestures();

[mfomitchev, 2014/08/06 16:11:49]

This seems wrong. You've already done GetAndResetPendingGestures() in
ProcessGestureEvents() and you are iterating over the gestures now. Calling
GetAndResetPendingGestures() probably will likely (almost?) always be a no-op.
Instead you probably want to return a bool in this method for consumed/not
consumed and cleanup appropriately in the calling method.

[evy, 2014/08/06 18:46:12]

I thought I had deleted these because we're always processing them.
I'll clean that up.

     OnSwipeEvent(gesture);
+    // The tap timer to leave gesture state is ended, and we now wait for all
+    // fingers to be released.
+    tap_timer_.Stop();
+    state_ = WAIT_FOR_NO_FINGERS;
+    VLOG_STATE();
     return;
   }
 }
@@ -637,6 +637,10 @@ void TouchExplorationController::ProcessGestureEvents() {
     for (ScopedVector<GestureEvent>::iterator i = gestures->begin();
          i != gestures->end();
          ++i) {
+      if (VLOG_on_ &&
+          (state_ == SLIDE_GESTURE || state_ == GESTURE_IN_PROGRESS) &&
+          (*i)->type() == ui::ET_GESTURE_SWIPE)
+        VLOG(0) << " \n Gesture Triggered: " << (*i)->name();
       if (state_ == SLIDE_GESTURE)
         SideSlideControl(*i);
       else
@@ -693,24 +697,27 @@ void TouchExplorationController::SideSlideControl(ui::GestureEvent* gesture) {
   delegate_->SetOutputLevel(int(volume));
 }
 
-
 void TouchExplorationController::OnSwipeEvent(ui::GestureEvent* swipe_gesture) {
   // A swipe gesture contains details for the direction in which the swipe
-  // occurred.
+  // occurred. TODO(evy) : Research which swipe results users most want and
+  // remap these swipes to the best events. Hopefully in the near future
+  // there will also be a menu for users to pick custom mappings.
   GestureEventDetails event_details = swipe_gesture->details();
-  if (event_details.swipe_left()) {
-    DispatchShiftSearchKeyEvent(ui::VKEY_LEFT);
-    return;
-  } else if (event_details.swipe_right()) {
-    DispatchShiftSearchKeyEvent(ui::VKEY_RIGHT);
-    return;
-  } else if (event_details.swipe_up()) {
-    DispatchShiftSearchKeyEvent(ui::VKEY_UP);
-    return;
-  } else if (event_details.swipe_down()) {
-    DispatchShiftSearchKeyEvent(ui::VKEY_DOWN);
+  int num_fingers = event_details.touch_points();
+  if(VLOG_on_)
+    VLOG(0) << "\nSwipe with " << num_fingers << " fingers.";
+
+  if (num_fingers > 4)
     return;
-  }
+
+  if (event_details.swipe_left())

[mfomitchev, 2014/08/06 16:11:49]

I'd check left_swipe_gestures_[num_fingers] to be safe. Same for other
directions. Or even better - call abstracted callbacks on the delegate - see
below.

[evy, 2014/08/08 22:39:13]

I did a check for is_null() - let me know if this isn't the right function to do
this check.

+    left_swipe_gestures_[num_fingers].Run();
+  else if (event_details.swipe_right())
+    right_swipe_gestures_[num_fingers].Run();
+  else if (event_details.swipe_up())
+    up_swipe_gestures_[num_fingers].Run();
+  else if (event_details.swipe_down())
+    down_swipe_gestures_[num_fingers].Run();
 }
 
 int TouchExplorationController::FindEdgesWithinBounds(gfx::Point point,
@@ -742,32 +749,45 @@ int TouchExplorationController::FindEdgesWithinBounds(gfx::Point point,
 }
 
 void TouchExplorationController::DispatchShiftSearchKeyEvent(

[mfomitchev, 2014/08/06 16:11:49]

Hmm.. I think it would be cleaner if TEC just called abstracted methods on its
delegate (e.g. Swipe(Direction direction, int num_fingers)), and the DispatchXYZ
methods for specific functionality lived in that delegate. This would let us
remove quite a bit of code from TEC which is getting pretty big.

[evy, 2014/08/08 22:39:13]

This would be awesome - our team has talked about this a bit already.
I'll submit this changelist first, add a TODO, and see if I can get it working
next week.

-    const ui::KeyboardCode direction) {
-  // In order to activate the shortcut shift+search+<arrow key>
+    const ui::KeyboardCode third_key) {
+  // In order to activate the shortcut shift+search+<key>
   // three KeyPressed events must be dispatched in succession along
   // with three KeyReleased events.
-  ui::KeyEvent shift_down = ui::KeyEvent(
+
+  ui::KeyEvent shift_down(
       ui::ET_KEY_PRESSED, ui::VKEY_SHIFT, ui::EF_SHIFT_DOWN);
-  ui::KeyEvent search_down = ui::KeyEvent(
+  ui::KeyEvent search_down(
       ui::ET_KEY_PRESSED, kChromeOSSearchKey, ui::EF_SHIFT_DOWN);
-  ui::KeyEvent direction_down =
-      ui::KeyEvent(ui::ET_KEY_PRESSED, direction, ui::EF_SHIFT_DOWN);
+  ui::KeyEvent third_key_down(ui::ET_KEY_PRESSED, third_key, ui::EF_SHIFT_DOWN);
 
-  ui::KeyEvent direction_up =
-      ui::KeyEvent(ui::ET_KEY_RELEASED, direction, ui::EF_SHIFT_DOWN);
-  ui::KeyEvent search_up = ui::KeyEvent(
+  ui::KeyEvent third_key_up(ui::ET_KEY_RELEASED, third_key, ui::EF_SHIFT_DOWN);
+  ui::KeyEvent search_up(
       ui::ET_KEY_RELEASED, kChromeOSSearchKey, ui::EF_SHIFT_DOWN);
-  ui::KeyEvent shift_up =
-      ui::KeyEvent(ui::ET_KEY_RELEASED, ui::VKEY_SHIFT, ui::EF_NONE);
+  ui ::KeyEvent shift_up(ui::ET_KEY_RELEASED, ui::VKEY_SHIFT, ui::EF_NONE);
 
   DispatchEvent(&shift_down);
   DispatchEvent(&search_down);
-  DispatchEvent(&direction_down);
-  DispatchEvent(&direction_up);
+  DispatchEvent(&third_key_down);
+  DispatchEvent(&third_key_up);
   DispatchEvent(&search_up);
   DispatchEvent(&shift_up);
 }
 
+void TouchExplorationController::DispatchKeyWithFlags(
+    const ui::KeyboardCode key,
+    int flags) {
+  ui::KeyEvent key_down(ui::ET_KEY_PRESSED, key, flags);
+  ui::KeyEvent key_up(ui::ET_KEY_RELEASED, key, flags);
+  DispatchEvent(&key_down);
+  DispatchEvent(&key_up);
+  if(VLOG_on_) {
+    VLOG(0) << "\nKey down: key code : " << key_down.key_code()
+            << ", flags: " << key_down.flags()
+            << "\nKey up: key code : " << key_up.key_code()
+            << ", flags: " << key_up.flags();
+  }
+}
+
 scoped_ptr<ui::Event> TouchExplorationController::CreateMouseMoveEvent(
     const gfx::PointF& location,
     int flags) {
@@ -809,8 +829,7 @@ void TouchExplorationController::ResetToNoFingersDown() {
     sound_timer_.Stop();
   state_ = NO_FINGERS_DOWN;
   VLOG_STATE();
-  if (tap_timer_.IsRunning())
-    tap_timer_.Stop();
+  tap_timer_.Stop();
 }
 
 void TouchExplorationController::VlogState(const char* function_name) {
@@ -843,6 +862,7 @@ void TouchExplorationController::VlogEvent(const ui::TouchEvent& touch_event,
       touch_event.type() == ET_TOUCH_MOVED){
     return;
   }
+
   const std::string& type = touch_event.name();
   const gfx::PointF& location = touch_event.location_f();
   const int touch_id = touch_event.touch_id();
@@ -874,14 +894,104 @@ const char* TouchExplorationController::EnumStateToString(State state) {
       return "GESTURE_IN_PROGRESS";
     case TOUCH_EXPLORE_SECOND_PRESS:
       return "TOUCH_EXPLORE_SECOND_PRESS";
-    case SLIDE_GESTURE:
-      return "SLIDE_GESTURE";
     case ONE_FINGER_PASSTHROUGH:
       return "ONE_FINGER_PASSTHROUGH";
-    case WAIT_FOR_ONE_FINGER:
-      return "WAIT_FOR_ONE_FINGER";
+    case WAIT_FOR_NO_FINGERS:
+      return "WAIT_FOR_NO_FINGERS";
+    case SLIDE_GESTURE:
+      return "SLIDE_GESTURE";
   }
   return "Not a state";
 }
 
+void TouchExplorationController::InitializeSwipeGestureMaps() {
+  // Gestures with one finger are used for navigation.
+  left_swipe_gestures_[1] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_LEFT);
+  right_swipe_gestures_[1] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_RIGHT);
+  up_swipe_gestures_[1] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_UP);
+  down_swipe_gestures_[1] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_DOWN);
+
+  // Gestures with two fingers.
+  left_swipe_gestures_[2] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_BROWSER_BACK,
+                 ui::EF_NONE);
+  right_swipe_gestures_[2] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_BROWSER_FORWARD,
+                 ui::EF_NONE);
+  // Jump to top.
+  up_swipe_gestures_[2] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_A);
+  // Read from here.
+  down_swipe_gestures_[2] =
+      base::Bind(&TouchExplorationController::DispatchShiftSearchKeyEvent,
+                 base::Unretained(this),
+                 ui::VKEY_R);
+
+  // Gestures with three fingers switch tabs left/right and scroll up/down.
+  left_swipe_gestures_[3] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_TAB,
+                 ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN);
+  right_swipe_gestures_[3] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_TAB,
+                 ui::EF_CONTROL_DOWN);
+  up_swipe_gestures_[3] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_NEXT,
+                 ui::EF_NONE);
+  down_swipe_gestures_[3] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_PRIOR,
+                 ui::EF_NONE);
+
+  // Gestures with four fingers should probably eventually be used for rare
+  // needs that are hard to access through menus.
+  // Note that brightness levels are here because they can be important for low
+  // vision users. However, none of these mappings are permanent.
+  left_swipe_gestures_[4] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 ui::VKEY_BRIGHTNESS_DOWN,
+                 ui::EF_NONE);
+  right_swipe_gestures_[4] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 VKEY_BRIGHTNESS_UP,
+                 ui::EF_NONE);
+  up_swipe_gestures_[4] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 VKEY_BROWSER_HOME,
+                 ui::EF_NONE);
+  down_swipe_gestures_[4] =
+      base::Bind(&TouchExplorationController::DispatchKeyWithFlags,
+                 base::Unretained(this),
+                 VKEY_BROWSER_REFRESH,
+                 ui::EF_NONE);
+
+}
+
 }  // namespace ui