Added touch event permutations test to touch_exploration_controller.

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 baf3a765f9253da85d91be1f6cd5a370945ee65f..ef6c37918ca21dcfc033542019578045aa4a48ec 100644
--- a/ui/chromeos/touch_exploration_controller_unittest.cc
+++ b/ui/chromeos/touch_exploration_controller_unittest.cc
@@ -13,6 +13,7 @@
 #include "ui/aura/window.h"
 #include "ui/events/event.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/test/events_test_utils.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
@@ -948,4 +949,93 @@ TEST_F(TouchExplorationTest, Passthrough) {
   EXPECT_TRUE(IsInNoFingersDownState());
 }
 
+int Factorial(int n) {

[dmazzoni, 2014/06/26 18:16:43]

Put this inside the "namespace {" at the top, so that we don't get a link error
if someone else tries to add a function called Factorial.

I guess we should do that for ConfirmEventsAreTouchAndEqual too, but that can be
a separate change.

[evy, 2014/06/26 22:31:05]

Done.

+  if (n <= 0)
+    return 0;
+  if (n == 1)
+    return 1;
+  if (n == 2)
+      return 2;
+  return n * Factorial(n - 1);
+}
+
+TEST_F(TouchExplorationTest, AllFingerPermutations) {
+  SwitchTouchExplorationMode(true);
+
+  // We will test all permutations of events from three different fingers
+  // to ensure that we return to NO_FINGERS_DOWN when fingers have been
+  // released.
+  ScopedVector<ui::TouchEvent> all_events;
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_PRESSED, gfx::Point(11, 12), 0, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_MOVED, gfx::Point(13, 14), 0, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_RELEASED, gfx::Point(15, 16), 0, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_PRESSED, gfx::Point(21, 22), 1, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_MOVED, gfx::Point(23, 24), 1, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_RELEASED, gfx::Point(25, 26), 1, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_PRESSED, gfx::Point(31, 32), 2, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_MOVED, gfx::Point(33, 34), 2, Now()));
+  all_events.push_back(
+      new TouchEvent(ui::ET_TOUCH_RELEASED, gfx::Point(35, 36), 2, Now()));
+
+  const int num_events = all_events.size();
+  const int num_permutations = Factorial(num_events);
+  std::vector<ui::TouchEvent*> queued_events;
+  ui::TouchEvent* next_dispatch;
+  int index;

[dmazzoni, 2014/06/26 18:16:43]

Declare all of these variables inside the block scope where they're used - so in
the case of "index", inside the second "for" loop.

Otherwise it appears as if you might care about the value of these variables
after the end of the loop.

[evy, 2014/06/26 22:31:05]

Done.

+  std::vector<bool> fingers_pressed(3, false);
+
+  for (int i = 0; i < num_permutations; i++) {
+    queued_events = all_events.get();
+    for (int j = num_events - 1; j >= 0; j--) {
+      if (j == num_events - 1) {
+        index = (i / Factorial(num_events - 1));

[dmazzoni, 2014/06/26 18:16:43]

This logic looks correct but inefficient. Calling Factorial inside this inner
loop is doing a lot of extra computation, it's computing the same Factorial over
and over again.

Try to do it this way: make a copy of "i" into a new variable, like
"permutation_index" or something like that. Then in each inner loop, the index
is (permutation_index % j) and then divide permutation_index by j. Essentially
you keep modifying permutation_index as you go.

[evy, 2014/06/26 22:31:05]

I'm not quite sure what you meant, but I did something that modifies the
factorial value as I go - tell me what you think.

+      } else if (j == 0){
+        index = 0;
+      } else {
+        index = ((i % Factorial(j + 1)) / Factorial(j));
+      }
+      next_dispatch = queued_events[index];
+      ASSERT_TRUE(next_dispatch != NULL);
+      EventTestApi test_dispatch(next_dispatch);
+      test_dispatch.set_time_stamp(Now());
+      generator_->Dispatch(next_dispatch);
+      queued_events.erase(queued_events.begin() + index);
+      // Note: it is possible to send a touched move event in the state
+      // SINGLE_TAP_RELEASED or TOUCH_EXPLORE_RELEASED
+      // because the events are not always generated in a sensical
+      // order. If there is a NOTREACHED() for touch moved hit in

[dmazzoni, 2014/06/26 18:16:43]

We should probably change the NOTREACHED to a LOG(WARNING), then.

NOTREACHED means "this should never happen".

[evy, 2014/06/26 22:31:05]

The thing is, this should never happen when the user is actually interacting. It
just happens in this test. Do you want me to stop releases from being deployed
if that finger isn't currently pressed? Or should we just change it to a
LOG(WARNING)

[dmazzoni, 2014/06/27 07:25:16]

What about if the user already has fingers down when they enable ChromeVox?
Couldn't it happen then?

Basically if it could happen even in really obscure cases like that, it's not
appropriate for NOTREACHED.

[evy, 2014/06/27 16:33:37]

Okay, a touch moved is now properly handled. 

+      // InSingleTapOrTouchExploreReleased, that is fine here.
+
+      // Keep track of what fingers have been pressed, to release
+      // only those fingers at the end, so the check for being in
+      // no fingers down can be accurate.
+      if (next_dispatch->type() == ET_TOUCH_PRESSED) {
+        fingers_pressed[next_dispatch->touch_id()] = true;
+      } else if (next_dispatch->type() == ET_TOUCH_RELEASED) {
+        fingers_pressed[next_dispatch->touch_id()] = false;
+      }
+    }
+    ASSERT_EQ(queued_events.size(), 0u);
+
+    // Release fingers recorded as pressed.
+    for(int j = 0; j < int(fingers_pressed.size()); j++){
+      if (fingers_pressed[j] == true) {
+        generator_->ReleaseTouchId(j);
+        fingers_pressed[j] = false;
+      }
+    }
+    AdvanceSimulatedTimePastTapDelay();
+    EXPECT_TRUE(IsInNoFingersDownState());
+    ClearCapturedEvents();
+  }
+}
+
 }  // namespace ui