Added accurate TouchToMouseMode testing to SplitTap

chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc

Index: chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc
diff --git a/chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc b/chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc
index 2eaf3c7105c762b390caecab60893a86efd665c8..0af9a2279ff10bb82d56099ebd4dda693111f0c4 100644
--- a/chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc
+++ b/chrome/browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc
@@ -14,6 +14,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/browser_test_utils.h"
+#include "ui/aura/client/cursor_client.h"
 #include "ui/aura/test/event_generator.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/compositor/compositor.h"
@@ -33,6 +34,27 @@ class TouchExplorationTest : public InProcessBrowserTest {
   virtual ~TouchExplorationTest() {}
 
  protected:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    // The RenderView for WebContents is created as a result of the
+    // navigation to the New Tab page which is done as part of the test
+    // SetUp. The creation involves sending a resize message to the renderer
+    // process. Here we wait for the resize ack to be received, because
+    // currently WindowEventDispatcher has code to hold touch and mouse
+    // move events until resize is complete (crbug.com/384342) which
+    // interferes with this test.
+    content::WebContents* web_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    content::WaitForResizeComplete(web_contents);
+    root_window_ = ash::Shell::GetInstance()->GetPrimaryRootWindow();
+    event_handler_.reset(new ui::test::TestEventHandler());
+    root_window_->AddPreTargetHandler(event_handler_.get());
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    SwitchTouchExplorationMode(false);
+    root_window_->RemovePreTargetHandler(event_handler_.get());
+  }
+
   void SwitchTouchExplorationMode(bool on) {
     ash::AccessibilityDelegate* ad =
         ash::Shell::GetInstance()->accessibility_delegate();
@@ -47,6 +69,8 @@ class TouchExplorationTest : public InProcessBrowserTest {
 
   ui::GestureDetector::Config gesture_detector_config_;
   base::SimpleTestTickClock* simulated_clock_;
+  aura::Window* root_window_;
+  scoped_ptr<ui::test::TestEventHandler> event_handler_;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(TouchExplorationTest);
@@ -56,73 +80,109 @@ private:
 // get rewritten when the touch exploration mode is on, and aren't affected
 // after the touch exploration mode is turned off.
 IN_PROC_BROWSER_TEST_F(TouchExplorationTest, ToggleOnOff) {
-  // The RenderView for WebContents is created as a result of the navigation
-  // to the New Tab page which is done as part of the test SetUp. The creation
-  // involves sending a resize message to the renderer process. Here we wait
-  // for the resize ack to be received, because currently WindowEventDispatcher
-  // has code to hold touch and mouse move events until resize is complete
-  // (crbug.com/384342) which interferes with this test.
-  content::WebContents* web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  content::WaitForResizeComplete(web_contents);
-  aura::Window* root_window = ash::Shell::GetInstance()->GetPrimaryRootWindow();
-  scoped_ptr<ui::test::TestEventHandler>
-      event_handler(new ui::test::TestEventHandler());
-  root_window->AddPreTargetHandler(event_handler.get());
   SwitchTouchExplorationMode(true);
-  aura::test::EventGenerator generator(root_window);
+  aura::test::EventGenerator generator(root_window_);
 
   base::TimeDelta initial_time = Now();
   ui::TouchEvent initial_press(
       ui::ET_TOUCH_PRESSED, gfx::Point(100, 200), 1, initial_time);
-  float delta_time =
-    (initial_press.location() - gfx::Point(109,209)).Length() /
-      gesture_detector_config_.minimum_swipe_velocity;
-  ui::TouchEvent touch_move(
-      ui::ET_TOUCH_MOVED,
-      gfx::Point(109, 209),
-      1,
-      initial_time + base::TimeDelta::FromSecondsD(delta_time));
-
   generator.Dispatch(&initial_press);
 
   // Since the touch exploration controller doesn't know if the user is
   // double-tapping or not, touch exploration is only initiated if the
-  // user moves more than 8 pixels away from the initial location (the "slop"),
-  // or after 300 ms has elapsed if the finger does not move fast enough.
+  // 300 ms has elapsed and the finger does not move fast enough to begin
+  // gestures.
+  ui::TouchEvent touch_move(ui::ET_TOUCH_MOVED,
+                            gfx::Point(100, 200),
+                            1,
+                            initial_time +
+                                gesture_detector_config_.double_tap_timeout +
+                                base::TimeDelta::FromMilliseconds(1));
   generator.Dispatch(&touch_move);
 
   // Number of mouse events may be greater than 1 because of ET_MOUSE_ENTERED.
-  EXPECT_GT(event_handler->num_mouse_events(), 0);
-  EXPECT_EQ(0, event_handler->num_touch_events());
-  event_handler->Reset();
+  EXPECT_GT(event_handler_->num_mouse_events(), 0);
+  EXPECT_EQ(0, event_handler_->num_touch_events());
+  event_handler_->Reset();
 
   SwitchTouchExplorationMode(false);
   generator.MoveTouchId(gfx::Point(11, 12), 1);
-  EXPECT_EQ(0, event_handler->num_mouse_events());
-  EXPECT_EQ(1, event_handler->num_touch_events());
-  event_handler->Reset();
+  EXPECT_EQ(0, event_handler_->num_mouse_events());
+  EXPECT_EQ(1, event_handler_->num_touch_events());
+  event_handler_->Reset();
 
   SwitchTouchExplorationMode(true);
   initial_time = Now();
   ui::TouchEvent second_initial_press(
       ui::ET_TOUCH_PRESSED, gfx::Point(500, 600), 2, initial_time);
-  delta_time =
-      (second_initial_press.location() - gfx::Point(509, 609)).Length() /
-      gesture_detector_config_.minimum_swipe_velocity;
-  ui::TouchEvent second_move(
-      ui::ET_TOUCH_MOVED,
-      gfx::Point(509, 609),
-      2,
-      initial_time + base::TimeDelta::FromSecondsD(delta_time));
+  ui::TouchEvent second_move(ui::ET_TOUCH_MOVED,
+                             gfx::Point(500, 600),
+                             2,
+                             initial_time +
+                                 gesture_detector_config_.double_tap_timeout +
+                                 base::TimeDelta::FromMilliseconds(1));
 
   generator.Dispatch(&second_initial_press);
   generator.Dispatch(&second_move);
-  EXPECT_GT(event_handler->num_mouse_events(), 0);
-  EXPECT_EQ(0, event_handler->num_touch_events());
+  EXPECT_GT(event_handler_->num_mouse_events(), 0);
+  EXPECT_EQ(0, event_handler_->num_touch_events());
+}
 
-  SwitchTouchExplorationMode(false);
-  root_window->RemovePreTargetHandler(event_handler.get());
+// This test makes sure that after the user clicks with split tap,
+// they continue to touch exploration mode if the original touch exploration
+// finger is still on the screen.
+IN_PROC_BROWSER_TEST_F(TouchExplorationTest, SplitTapExplore) {
+  SwitchTouchExplorationMode(true);
+  aura::test::EventGenerator generator(root_window_);
+  aura::client::CursorClient* cursor_client =
+      aura::client::GetCursorClient(root_window_);
+
+  // Mouse events should show the cursor.
+  generator.MoveMouseTo(gfx::Point(30, 31));
+  EXPECT_TRUE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_TRUE(cursor_client->IsCursorVisible());
+
+  // The cursor should be shown immediately after the  press, and hidden
+  // after the move.
+  base::TimeDelta initial_time = Now();
+  ui::TouchEvent initial_press(
+      ui::ET_TOUCH_PRESSED, gfx::Point(100, 200), 1, initial_time);
+  generator.Dispatch(&initial_press);
+  EXPECT_TRUE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_TRUE(cursor_client->IsCursorVisible());
+
+  // Initiate touch explore by moving out of the slop slowly.
+  ui::TouchEvent touch_move(ui::ET_TOUCH_MOVED,
+                            gfx::Point(109, 209),
+                            1,
+                            initial_time +
+                                gesture_detector_config_.double_tap_timeout +

[mfomitchev, 2014/07/09 19:20:37]

Using double_tap_timeout this way doesn't seem right. If you wait
double_tap_timeout - you are supposed enter touch exploration mode regardless of
whether you move or not. If you move far enough, you are supposed to enter TEM
regardless of the time (provided you haven't generated a swipe).

Also aboxhall@ made a comment about not using "magic numbers" for location..

[evy, 2014/07/09 19:36:00]

I need to change the comment to "Initiate touch explore by waiting for the tap
timer timeout". The touch_move doesn't actually move location - oh wait here it
does. See ToggleOnOff for how this should actually work, and I'll update this in
the next patch.

My update removes the use of magic numbers as well, since I don't use location
anymore to go into touch exploration.

[mfomitchev, 2014/07/09 20:47:21]

I think this is a bit artificial, but not terrible if that's the way you want to
do it. I think the previous way was a bit cleaner. That's because timeout should
technically occur without any moves - you are relying on the fallback mechanism
for the case when the timer doesn't fire to trigger the transition here. But if
that's how you want to do it, that's fine, just explain what you are doing in
comments (i.e. that it's not really important as a move, it's just a way for you
to fake the time advancing).

[evy, 2014/07/09 21:32:09]

I think the only cleaner thing (this might be what you were referring to) was
actually getting the touch exploration controller and advancing time - which is
what I was doing in the other branch.
I'll keep it this way for now, and might play around with the time advancement
later. I'll update the comments/variable names to make it more clear what I'm
doing.

[mfomitchev, 2014/07/09 21:53:02]

I was actually referring to performing a move like you did before (and ensuring
that swipe isn't generated by adjusting the time). But either way, it's fine.

[evy, 2014/07/10 02:44:20]

I'm more comfortable with what I have now, mostly because gesture vs. touch
exploration categorization is somewhat complicated right now and we're
considering simplifying it in the future.

+                                base::TimeDelta::FromMilliseconds(1));
+  generator.Dispatch(&touch_move);
+  EXPECT_TRUE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_FALSE(cursor_client->IsCursorVisible());
+  event_handler_->Reset();
+
+  // Press and release with a second finger for split tap. This should send
+  // touch press and release events which should send a click press and release.
+  // Once the press is passed through, mouse events should be disabled.
+  // Mouse events are reenabled after the release.
+  generator.set_current_location(gfx::Point(102, 202));
+  generator.PressTouchId(2);
+  EXPECT_FALSE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_FALSE(cursor_client->IsCursorVisible());
+  generator.ReleaseTouchId(2);
+  EXPECT_TRUE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_FALSE(cursor_client->IsCursorVisible());
+  EXPECT_EQ(2, event_handler_->num_touch_events());
+  event_handler_->Reset();
+
+  // Continuing to move the touch exploration finger should send more mouse
+  // events.
+  generator.MoveTouchId(gfx::Point(509, 609), 1);
+  EXPECT_EQ(0, event_handler_->num_touch_events());
+  EXPECT_TRUE(cursor_client->IsMouseEventsEnabled());
+  EXPECT_FALSE(cursor_client->IsCursorVisible());
 }
 
 }  // namespace ui