Added multi-finger gestures to touch_exploration_controller

ui/chromeos/touch_exploration_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/chromeos/touch_exploration_controller.h"
 
 #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"
 #include "ui/aura/window_tree_host.h"
 #include "ui/events/event.h"
 #include "ui/events/event_processor.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/geometry/rect.h"
 
 #define VLOG_STATE() if (VLOG_IS_ON(0)) VlogState(__func__)
@@ skipping 16 matching lines @@
     : root_window_(root_window),
       delegate_(delegate),
       state_(NO_FINGERS_DOWN),
       event_handler_for_testing_(NULL),
       gesture_provider_(this),
       prev_state_(NO_FINGERS_DOWN),
       VLOG_on_(true),
       tick_clock_(NULL) {
   CHECK(root_window);
   root_window->GetHost()->GetEventSource()->AddEventRewriter(this);
+  InitializeSwipeGestureMaps();
 }
 
 TouchExplorationController::~TouchExplorationController() {
   root_window_->GetHost()->GetEventSource()->RemoveEventRewriter(this);
 }
 
 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);
 
   // If the tap timer should have fired by now but hasn't, run it now and
   // stop the timer. This is important so that behavior is consistent with
   // the timestamps of the events, and not dependent on the granularity of
   // the timer.
   if (tap_timer_.IsRunning() &&
       touch_event.time_stamp() - initial_press_->time_stamp() >
           gesture_detector_config_.double_tap_timeout) {
     tap_timer_.Stop();
     OnTapTimerFired();
     // Note: this may change the state. We should now continue and process
     // this event under this new state.
   }
 
   const ui::EventType type = touch_event.type();
   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) {
       if (current_touch_ids_.size() == 0) {
         ResetToNoFingersDown();
       }
     }
 
     std::vector<int>::iterator it = std::find(
         current_touch_ids_.begin(), current_touch_ids_.end(), touch_id);
@@ skipping 10 matching lines @@
 
     // Can happen if touch exploration is enabled while fingers were down.
     if (it == current_touch_ids_.end())
       return ui::EVENT_REWRITE_CONTINUE;
 
     touch_locations_[*it] = location;
   }
   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:
       return InSingleTapPressed(touch_event, rewritten_event);
     case SINGLE_TAP_RELEASED:
     case TOUCH_EXPLORE_RELEASED:
       return InSingleTapOrTouchExploreReleased(touch_event, rewritten_event);
     case DOUBLE_TAP_PENDING:
       return InDoubleTapPending(touch_event, rewritten_event);
     case TOUCH_RELEASE_PENDING:
       return InTouchReleasePending(touch_event, rewritten_event);
     case TOUCH_EXPLORATION:
       return InTouchExploration(touch_event, rewritten_event);
     case GESTURE_IN_PROGRESS:
       return InGestureInProgress(touch_event, rewritten_event);
     case TOUCH_EXPLORE_SECOND_PRESS:
       return InTouchExploreSecondPress(touch_event, rewritten_event);
+    case ONE_FINGER_PASSTHROUGH:
+      return InOneFingerPassthrough(touch_event, rewritten_event);
+    case WAIT_FOR_NO_FINGERS:
+      return InWaitForNoFingers(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);
   }
   NOTREACHED();
   return ui::EVENT_REWRITE_CONTINUE;
 }
 
 ui::EventRewriteStatus TouchExplorationController::NextDispatchEvent(
     const ui::Event& last_event, scoped_ptr<ui::Event>* new_event) {
   NOTREACHED();
   return ui::EVENT_REWRITE_CONTINUE;
 }
 
 ui::EventRewriteStatus TouchExplorationController::InNoFingersDown(
     const ui::TouchEvent& event, scoped_ptr<ui::Event>* rewritten_event) {
   const ui::EventType type = event.type();
   if (type == ui::ET_TOUCH_PRESSED) {
     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;
   }
   NOTREACHED() << "Unexpected event type received: " << event.name();
   return ui::EVENT_REWRITE_CONTINUE;
 }
 
 ui::EventRewriteStatus TouchExplorationController::InSingleTapPressed(
     const ui::TouchEvent& event, scoped_ptr<ui::Event>* rewritten_event) {
   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()) {
       state_ = SINGLE_TAP_RELEASED;
       VLOG_STATE();
     } else if (current_touch_ids_.size() == 0) {
       ResetToNoFingersDown();
     }
     return EVENT_REWRITE_DISCARD;
   } else if (type == ui::ET_TOUCH_MOVED) {
@@ skipping 26 matching lines @@
     if (velocity > gesture_detector_config_.minimum_swipe_velocity) {
       state_ = GESTURE_IN_PROGRESS;
       VLOG_STATE();
       return InGestureInProgress(event, rewritten_event);
     }
     EnterTouchToMouseMode();
     state_ = TOUCH_EXPLORATION;
     VLOG_STATE();
     return InTouchExploration(event, rewritten_event);
   }
   NOTREACHED() << "Unexpected event type received: " << event.name();

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

What happened to these useful logs?

[evy, 2014/08/07 21:20:33]

They're all in RewriteEvent now, before event sending an event to any of these
functions. 
I kept the NOTREACHED() just in case this function is somehow called through
another means where the touch event could be wild - should I keep the logs here
too?

   return ui::EVENT_REWRITE_CONTINUE;
 }
 
 ui::EventRewriteStatus
 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) {
     // If there is no touch exploration yet, we can't send a click, so discard.
     if (!last_touch_exploration_) {
       tap_timer_.Stop();
       return ui::EVENT_REWRITE_DISCARD;
     }
     // This is the second tap in a double-tap (or double tap-hold).
     // We set the tap timer. If it fires before the user lifts their finger,
     // one-finger passthrough begins. Otherwise, there is a touch press and
     // 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.
     last_unused_finger_event_.reset(new ui::TouchEvent(event));
     return ui::EVENT_REWRITE_DISCARD;
   } else if (type == ui::ET_TOUCH_RELEASED && !last_touch_exploration_) {
     // If the previous press was discarded, we need to also handle its
     // release.
     if (current_touch_ids_.size() == 0) {
       ResetToNoFingersDown();
@@ skipping 68 matching lines @@
   return ui::EVENT_REWRITE_CONTINUE;
 }
 
 ui::EventRewriteStatus TouchExplorationController::InTouchExploration(
     const ui::TouchEvent& event,
     scoped_ptr<ui::Event>* rewritten_event) {
   const ui::EventType type = event.type();
   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(),
                            event.touch_id(),
                            event.time_stamp()));
     (*rewritten_event)->set_flags(event.flags());
     state_ = TOUCH_EXPLORE_SECOND_PRESS;
     VLOG_STATE();
     return ui::EVENT_REWRITE_REWRITTEN;
   } else if (type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED) {
     initial_press_.reset(new TouchEvent(event));
     StartTapTimer();
     state_ = TOUCH_EXPLORE_RELEASED;
     VLOG_STATE();
   } else if (type != ui::ET_TOUCH_MOVED) {
     NOTREACHED() << "Unexpected event type received: " << event.name();
     return ui::EVENT_REWRITE_CONTINUE;
   }
 
   // Rewrite as a mouse-move event.
   *rewritten_event = CreateMouseMoveEvent(event.location(), event.flags());
   last_touch_exploration_.reset(new TouchEvent(event));
   return ui::EVENT_REWRITE_REWRITTEN;
 }
 
 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;
+  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;
   }
-
-  // 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);
+  // 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();
   }
-  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();
-    }
-  }
-  ProcessGestureEvents();
   return ui::EVENT_REWRITE_DISCARD;
 }
 
 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;
   }
   if (event.touch_id() != initial_press_->touch_id()) {
     if (current_touch_ids_.size() == 0) {
       ResetToNoFingersDown();
     }
     return ui::EVENT_REWRITE_DISCARD;
   }
   rewritten_event->reset(
@@ skipping 45 matching lines @@
     (*rewritten_event)->set_flags(event.flags());
     state_ = TOUCH_EXPLORATION;
     EnterTouchToMouseMode();
     VLOG_STATE();
     return ui::EVENT_REWRITE_REWRITTEN;
   }
   NOTREACHED() << "Unexpected event type received: " << event.name();
   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();
   if (!(type == ui::ET_TOUCH_PRESSED || type == ui::ET_TOUCH_MOVED ||
         type == ui::ET_TOUCH_RELEASED || type == ui::ET_TOUCH_CANCELLED)) {
     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;
 }
 
 void TouchExplorationController::PlaySoundForTimer() {
   delegate_->PlayVolumeAdjustSound();
 }
 
 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,
   // then wait until all fingers have been lifted.
   if (type == ui::ET_TOUCH_PRESSED ||
       event.touch_id() != initial_press_->touch_id()) {
     if (sound_timer_.IsRunning())
       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);
   if (!(edge & RIGHT_EDGE) && (type != ui::ET_TOUCH_RELEASED)) {
     if (sound_timer_.IsRunning()) {
       sound_timer_.Stop();
     }
     return EVENT_REWRITE_DISCARD;
   }
 
   // This can occur if the user leaves the screen edge and then returns to it to
   // continue adjusting the sound.
   if (!sound_timer_.IsRunning()) {
     sound_timer_.Start(FROM_HERE,
                        kSoundDelay,
                        this,
                        &ui::TouchExplorationController::PlaySoundForTimer);
     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;
 }
 
 base::TimeDelta TouchExplorationController::Now() {
   if (tick_clock_) {
     // This is the same as what EventTimeForNow() does, but here we do it
     // with a clock that can be replaced with a simulated clock for tests.
     return base::TimeDelta::FromInternalValue(
         tick_clock_->NowTicks().ToInternalValue());
   }
@@ skipping 23 matching lines @@
                             last_touch_exploration_->location();
       scoped_ptr<ui::TouchEvent> passthrough_press(
           new ui::TouchEvent(ui::ET_TOUCH_PRESSED,
                              last_touch_exploration_->location(),
                              last_unused_finger_event_->touch_id(),
                              Now()));
       DispatchEvent(passthrough_press.get());
       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;
   }
   scoped_ptr<ui::Event> mouse_move =
       CreateMouseMoveEvent(initial_press_->location(), initial_press_->flags());
   DispatchEvent(mouse_move.get());
   last_touch_exploration_.reset(new TouchEvent(*initial_press_));
 }
 
 void TouchExplorationController::DispatchEvent(ui::Event* event) {
   if (event_handler_for_testing_) {
     event_handler_for_testing_->OnEvent(event);
     return;
   }
   ui::EventDispatchDetails result ALLOW_UNUSED =
       root_window_->GetHost()->dispatcher()->OnEventFromSource(event);
 }
 
 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;
   }
 }
 
 void TouchExplorationController::ProcessGestureEvents() {

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

return a bool?

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

I'm doing it a bit differently - tell me what you think :)

   scoped_ptr<ScopedVector<ui::GestureEvent> > gestures(
       gesture_provider_.GetAndResetPendingGestures());
   if (gestures) {
     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
         OnGestureEvent(*i);

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

break the loop if this returns true

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

breaks the loop on a return if a swipe or slide gesture goes through

     }
   }
 }
 
 void TouchExplorationController::SideSlideControl(ui::GestureEvent* gesture) {
   ui::EventType type = gesture->type();
   if (!gesture->IsScrollGestureEvent())
     return;
 
   if (type == ET_GESTURE_SCROLL_BEGIN) {
@@ skipping 32 matching lines @@
   float ratio = (location.y() - kMaxDistanceFromEdge) / volume_adjust_height;
   float volume = 100 - 100 * ratio;
   if (VLOG_on_) {
     VLOG(0) << "\n Volume = " << volume
             << "\n Location = " << location.ToString()
             << "\n Bounds = " << root_window_->bounds().right();
   }
   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);
+  int num_fingers = event_details.touch_points();
+  if(VLOG_on_)
+    VLOG(0) << "\nSwipe with " << num_fingers << " fingers.";
+
+  if (num_fingers > 4)
     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);
-    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,
                                                       float bounds) {
   // Since GetBoundsInScreen is in DIPs but point is not, then point needs to be
   // converted.
   root_window_->GetHost()->ConvertPointFromNativeScreen(&point);
   gfx::Rect window = root_window_->GetBoundsInScreen();
 
   float left_edge_limit = window.x() + bounds;
@@ skipping 10 matching lines @@
     result |= LEFT_EDGE;
   if (point.x() > right_edge_limit)
     result |= RIGHT_EDGE;
   if (point.y() < top_edge_limit)
     result |= TOP_EDGE;
   if (point.y() > bottom_edge_limit)
     result |= BOTTOM_EDGE;
   return result;
 }
 
 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) {
   // The "synthesized" flag should be set on all events that don't have a
   // backing native event.
   flags |= ui::EF_IS_SYNTHESIZED;
 
   // This flag is used to identify mouse move events that were generated from
   // touch exploration in Chrome code.
   flags |= ui::EF_TOUCH_ACCESSIBILITY;
@@ skipping 21 matching lines @@
   if (cursor_client && cursor_client->IsCursorVisible())
     cursor_client->HideCursor();
 }
 
 void TouchExplorationController::ResetToNoFingersDown() {
   ProcessGestureEvents();
   if (sound_timer_.IsRunning())
     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) {
   if (!VLOG_on_)
     return;
   if (prev_state_ == state_)
     return;
   prev_state_ = state_;
   const char* state_string = EnumStateToString(state_);
   VLOG(0) << "\n Function name: " << function_name
@@ skipping 12 matching lines @@
     return;
   }
   // The above statement prevents events of the same type and id from being
   // printed in a row. However, if two fingers are down, they would both be
   // moving and alternating printing move events unless we check for this.
   if (prev_event_ != NULL &&
       prev_event_->type() == ET_TOUCH_MOVED &&
       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();
 
   VLOG(0) << "\n Function name: " << function_name
           << "\n Event Type: " << type
           << "\n Location: " << location.ToString()
           << "\n Touch ID: " << touch_id;
   prev_event_.reset(new TouchEvent(touch_event));
 }
@@ skipping 11 matching lines @@
     case DOUBLE_TAP_PENDING:
       return "DOUBLE_TAP_PENDING";
     case TOUCH_RELEASE_PENDING:
       return "TOUCH_RELEASE_PENDING";
     case TOUCH_EXPLORATION:
       return "TOUCH_EXPLORATION";
     case GESTURE_IN_PROGRESS:
       return "GESTURE_IN_PROGRESS";
     case TOUCH_EXPLORE_SECOND_PRESS:
       return "TOUCH_EXPLORE_SECOND_PRESS";
+    case ONE_FINGER_PASSTHROUGH:
+      return "ONE_FINGER_PASSTHROUGH";
+    case WAIT_FOR_NO_FINGERS:
+      return "WAIT_FOR_NO_FINGERS";
     case SLIDE_GESTURE:
       return "SLIDE_GESTURE";
-    case ONE_FINGER_PASSTHROUGH:
-      return "ONE_FINGER_PASSTHROUGH";
-    case WAIT_FOR_ONE_FINGER:
-      return "WAIT_FOR_ONE_FINGER";
   }
   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