chromeos: gets TabScrubber tests working with mash

Many of these tests previously worked by using a pattern such as:
. make TabScrubbers timer use a zero delay.
. generate a bunch of async scroll events (using EventGenerator)
. record when active tab changes.
. run message loop, waiting for active tab to change to certain index,
  then stop.

This is problematic in the mash case as EventGenerator, for mash, runs
a message loop. This meant the timer would fire at unexpected times.

The fix is to increase the timer delay, generate a single event and then
force the TabScrubber timer to complete. This way the test gets consistent
results, regardless of whether EventGenerator internally runs a message
loop.

BUG=889097,890050
TEST=test only changes

Change-Id: I5decb690c4ef7ff67af21cc9a588aaa5046fad89
Reviewed-on: https://chromium-review.googlesource.com/c/1297288Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602522}

ash/system/user/tray_user_unittest.cc

@@ -225,7 +225,6 @@ TEST_F(TrayUserTest, MultiUserModeDoesNotAllowToAddUser) {
 
   // Move the mouse over the status area and click to open the status menu.
   ui::test::EventGenerator* generator = GetEventGenerator();
-  generator->set_async(false);
 
   // Verify that nothing is shown.
   EXPECT_FALSE(tray()->IsSystemBubbleVisible());

chrome/browser/ui/ash/tab_scrubber.cc

@@ -257,8 +257,10 @@ void TabScrubber::FinishScrub(bool activate) {
 }
 
 void TabScrubber::ScheduleFinishScrubIfNeeded() {
+  // Tests use a really long delay to ensure RunLoops don't unnecessarily
+  // trigger the timer running.
   const base::TimeDelta delay = base::TimeDelta::FromMilliseconds(
-      use_default_activation_delay_ ? 200 : 0);
+      use_default_activation_delay_ ? 200 : 20000);
   activate_timer_.Start(FROM_HERE, delay,
                         base::BindRepeating(&TabScrubber::FinishScrub,
                                             base::Unretained(this), true));

chrome/browser/ui/ash/tab_scrubber.h

@@ -93,7 +93,7 @@ class TabScrubber : public ui::EventHandler,
   // Timer to control a delayed activation of the |highlighted_tab_|.
   base::RetainingOneShotTimer activate_timer_;
   // True if the default activation delay should be used with |activate_timer_|.
-  // A value of false means the |activate_timer_| gets a zero delay.
+  // A value of false means the |activate_timer_| gets a really long delay.
   bool use_default_activation_delay_ = true;
   // Forces the tabs to be revealed if we are in immersive fullscreen.
   std::unique_ptr<ImmersiveRevealedLock> immersive_reveal_lock_;

chrome/browser/ui/ash/tab_scrubber_browsertest.cc

@@ -26,6 +26,7 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/test/test_utils.h"
 #include "ui/aura/window.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/test/event_generator.h"
 
@@ -81,7 +82,7 @@ class ImmersiveRevealEndedWaiter : public ImmersiveModeController::Observer {
 class TabScrubberTest : public InProcessBrowserTest,
                         public TabStripModelObserver {
  public:
-  TabScrubberTest() : target_index_(-1) {}
+  TabScrubberTest() = default;
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(chromeos::switches::kNaturalScrollDefault);
@@ -89,7 +90,6 @@ class TabScrubberTest : public InProcessBrowserTest,
 
   void SetUpOnMainThread() override {
     TabScrubber::GetInstance()->use_default_activation_delay_ = false;
-
     // Disable external monitor scaling of coordinates.
     ash::Shell* shell = ash::Shell::Get();
     shell->event_transformation_handler()->set_transformation_mode(
@@ -109,19 +109,17 @@ class TabScrubberTest : public InProcessBrowserTest,
     return tab_strip;
   }
 
-  float GetStartX(Browser* browser,
-                  int index,
-                  TabScrubber::Direction direction) {
-    return static_cast<float>(
-        TabScrubber::GetStartPoint(GetTabStrip(browser), index, direction).x());
+  int GetStartX(Browser* browser, int index, TabScrubber::Direction direction) {
+    return TabScrubber::GetStartPoint(GetTabStrip(browser), index, direction)
+        .x();
   }
 
-  float GetTabCenter(Browser* browser, int index) {
-    return static_cast<float>(GetTabStrip(browser)
-                                  ->tab_at(index)
-                                  ->GetMirroredBounds()
-                                  .CenterPoint()
-                                  .x());
+  int GetTabCenter(Browser* browser, int index) {
+    return GetTabStrip(browser)
+        ->tab_at(index)
+        ->GetMirroredBounds()
+        .CenterPoint()
+        .x();
   }
 
   // The simulated scroll event's offsets are calculated in the tests rather
@@ -144,21 +142,19 @@ class TabScrubberTest : public InProcessBrowserTest,
   // Sends one scroll event synchronously without initial or final
   // fling events.
   void SendScrubEvent(Browser* browser, int index) {
-    aura::Window* window = browser->window()->GetNativeWindow();
-    aura::Window* root = window->GetRootWindow();
-    ui::test::EventGenerator event_generator(root, window);
+    auto event_generator = CreateEventGenerator(browser);
     int active_index = browser->tab_strip_model()->active_index();
     TabScrubber::Direction direction =
         index < active_index ? TabScrubber::LEFT : TabScrubber::RIGHT;
 
     direction = InvertDirectionIfNeeded(direction);
 
-    float offset = GetTabCenter(browser, index) -
-                   GetStartX(browser, active_index, direction);
+    int offset = GetTabCenter(browser, index) -
+                 GetStartX(browser, active_index, direction);
     ui::ScrollEvent scroll_event(ui::ET_SCROLL, gfx::Point(0, 0),
                                  ui::EventTimeForNow(), 0, offset, 0, offset, 0,
                                  3);
-    event_generator.Dispatch(&scroll_event);
+    event_generator->Dispatch(&scroll_event);
   }
 
   enum ScrubType {
@@ -170,10 +166,7 @@ class TabScrubberTest : public InProcessBrowserTest,
   // Sends asynchronous events and waits for tab at |index| to become
   // active.
   void Scrub(Browser* browser, int index, ScrubType scrub_type) {
-    aura::Window* window = browser->window()->GetNativeWindow();
-    aura::Window* root = window->GetRootWindow();
-    ui::test::EventGenerator event_generator(root, window);
-    event_generator.set_async(true);
+    auto event_generator = CreateEventGenerator(browser);
     activation_order_.clear();
     int active_index = browser->tab_strip_model()->active_index();
     ASSERT_NE(index, active_index);
@@ -192,21 +185,20 @@ class TabScrubberTest : public InProcessBrowserTest,
 
     if (scrub_type == SKIP_TABS)
       increment *= 2;
-    float last = GetStartX(browser, active_index, direction);
-    std::vector<gfx::PointF> offsets;
+    browser->tab_strip_model()->AddObserver(this);
+    ScrollGenerator scroll_generator(event_generator.get());
+    int last = GetStartX(browser, active_index, direction);
     for (int i = active_index + increment; i != (index + increment);
          i += increment) {
-      float tab_center = GetTabCenter(browser, i);
-      offsets.push_back(gfx::PointF(tab_center - last, 0));
+      int tab_center = GetTabCenter(browser, i);
+      scroll_generator.GenerateScroll(tab_center - last);
       last = GetStartX(browser, i, direction);
       if (scrub_type == REPEAT_TABS) {
-        offsets.push_back(gfx::PointF(static_cast<float>(increment), 0));
+        scroll_generator.GenerateScroll(increment);
         last += increment;
       }
     }
-    event_generator.ScrollSequence(
-        gfx::Point(0, 0), base::TimeDelta::FromMilliseconds(100), offsets, 3);
-    RunUntilTabActive(browser, index);
+    browser->tab_strip_model()->RemoveObserver(this);
   }
 
   // Sends events and waits for tab at |index| to become active
@@ -214,19 +206,11 @@ class TabScrubberTest : public InProcessBrowserTest,
   // If the active tab is expected to stay the same, send events
   // synchronously (as we don't have anything to wait for).
   void SendScrubSequence(Browser* browser, float x_offset, int index) {
-    aura::Window* window = browser->window()->GetNativeWindow();
-    aura::Window* root = window->GetRootWindow();
-    ui::test::EventGenerator event_generator(root, window);
-    bool wait_for_active = false;
-    if (index != browser->tab_strip_model()->active_index()) {
-      wait_for_active = true;
-      event_generator.set_async(true);
-    }
-    event_generator.ScrollSequence(gfx::Point(0, 0),
-                                   base::TimeDelta::FromMilliseconds(100),
-                                   x_offset, 0, 1, 3);
-    if (wait_for_active)
-      RunUntilTabActive(browser, index);
+    auto event_generator = CreateEventGenerator(browser);
+    browser->tab_strip_model()->AddObserver(this);
+    ScrollGenerator scroll_generator(event_generator.get());
+    scroll_generator.GenerateScroll(x_offset);
+    browser->tab_strip_model()->RemoveObserver(this);
   }
 
   void AddTabs(Browser* browser, int num_tabs) {
@@ -245,26 +229,65 @@ class TabScrubberTest : public InProcessBrowserTest,
                         int index,
                         int reason) override {
     activation_order_.push_back(index);
-    if (index == target_index_)
-      quit_closure_.Run();
   }
 
   // History of tab activation. Scrub() resets it.
   std::vector<int> activation_order_;
 
  private:
-  void RunUntilTabActive(Browser* browser, int target) {
-    base::RunLoop run_loop;
-    quit_closure_ = content::GetDeferredQuitTaskForRunLoop(&run_loop);
-    browser->tab_strip_model()->AddObserver(this);
-    target_index_ = target;
-    content::RunThisRunLoop(&run_loop);
-    browser->tab_strip_model()->RemoveObserver(this);
-    target_index_ = -1;
+  // Used to generate a sequence of scrolls. Starts with a cancel, is followed
+  // by any number of scrolls and finally a fling-start. After every event this
+  // forces the TabScrubber to complete any pending activation.
+  class ScrollGenerator {
+   public:
+    // TabScrubber reacts to three-finger scrolls.
+    static const int kNumFingers = 3;
+
+    explicit ScrollGenerator(ui::test::EventGenerator* event_generator)
+        : event_generator_(event_generator) {
+      ui::ScrollEvent fling_cancel(ui::ET_SCROLL_FLING_CANCEL, gfx::Point(),
+                                   time_for_next_event_, 0, 0, 0, 0, 0,
+                                   kNumFingers);
+      event_generator->Dispatch(&fling_cancel);
+      if (TabScrubber::GetInstance()->IsActivationPending())
+        TabScrubber::GetInstance()->FinishScrub(true);
+    }
+
+    ~ScrollGenerator() {
+      ui::ScrollEvent fling_start(ui::ET_SCROLL_FLING_START, gfx::Point(),
+                                  time_for_next_event_, 0, last_x_offset_, 0,
+                                  last_x_offset_, 0, kNumFingers);
+      event_generator_->Dispatch(&fling_start);
+      if (TabScrubber::GetInstance()->IsActivationPending())
+        TabScrubber::GetInstance()->FinishScrub(true);
+    }
+
+    void GenerateScroll(int x_offset) {
+      time_for_next_event_ += base::TimeDelta::FromMilliseconds(100);
+      ui::ScrollEvent scroll(ui::ET_SCROLL, gfx::Point(), time_for_next_event_,
+                             0, x_offset, 0, x_offset, 0, kNumFingers);
+      last_x_offset_ = x_offset;
+      event_generator_->Dispatch(&scroll);
+      if (TabScrubber::GetInstance()->IsActivationPending())
+        TabScrubber::GetInstance()->FinishScrub(true);
+    }
+
+   private:
+    ui::test::EventGenerator* event_generator_;
+    base::TimeTicks time_for_next_event_ = ui::EventTimeForNow();
+    int last_x_offset_ = 0;
+
+    DISALLOW_COPY_AND_ASSIGN(ScrollGenerator);
+  };
+
+  std::unique_ptr<ui::test::EventGenerator> CreateEventGenerator(
+      Browser* browser) {
+    aura::Window* window = browser->window()->GetNativeWindow();
+    aura::Window* root = window->GetRootWindow();
+    return std::make_unique<ui::test::EventGenerator>(
+        features::IsUsingWindowService() ? nullptr : root, window);
   }
 
-  base::Closure quit_closure_;
-  int target_index_;
 
   DISALLOW_COPY_AND_ASSIGN(TabScrubberTest);
 };

ui/events/test/event_generator.cc

@@ -121,7 +121,6 @@ EventGenerator::EventGenerator(std::unique_ptr<EventGeneratorDelegate> delegate)
 }
 
 EventGenerator::~EventGenerator() {
-  pending_events_.clear();
   ui::SetEventTickClockForTesting(nullptr);
 }
 
@@ -565,43 +564,6 @@ void EventGenerator::ScrollSequence(const gfx::Point& start,
   Dispatch(&fling_start);
 }
 
-void EventGenerator::ScrollSequence(const gfx::Point& start,
-                                    const base::TimeDelta& step_delay,
-                                    const std::vector<gfx::PointF>& offsets,
-                                    int num_fingers) {
-  size_t steps = offsets.size();
-  base::TimeTicks timestamp = ui::EventTimeForNow();
-  ui::ScrollEvent fling_cancel(ui::ET_SCROLL_FLING_CANCEL,
-                               start,
-                               timestamp,
-                               0,
-                               0, 0,
-                               0, 0,
-                               num_fingers);
-  Dispatch(&fling_cancel);
-
-  for (size_t i = 0; i < steps; ++i) {
-    timestamp += step_delay;
-    ui::ScrollEvent scroll(ui::ET_SCROLL,
-                           start,
-                           timestamp,
-                           0,
-                           offsets[i].x(), offsets[i].y(),
-                           offsets[i].x(), offsets[i].y(),
-                           num_fingers);
-    Dispatch(&scroll);
-  }
-
-  ui::ScrollEvent fling_start(ui::ET_SCROLL_FLING_START,
-                              start,
-                              timestamp,
-                              0,
-                              offsets[steps - 1].x(), offsets[steps - 1].y(),
-                              offsets[steps - 1].x(), offsets[steps - 1].y(),
-                              num_fingers);
-  Dispatch(&fling_start);
-}
-
 void EventGenerator::GenerateTrackpadRest() {
   int num_fingers = 2;
   ui::ScrollEvent scroll(ui::ET_SCROLL, current_location_,
@@ -627,7 +589,19 @@ void EventGenerator::ReleaseKey(ui::KeyboardCode key_code, int flags) {
 }
 
 void EventGenerator::Dispatch(ui::Event* event) {
-  DoDispatchEvent(event, async_);
+  if (event->IsTouchEvent()) {
+    ui::TouchEvent* touch_event = static_cast<ui::TouchEvent*>(event);
+    touch_pointer_details_.id = touch_event->pointer_details().id;
+    touch_event->SetPointerDetailsForTest(touch_pointer_details_);
+  }
+
+  if (!event->handled()) {
+    ui::EventSource* event_source = delegate()->GetEventSource(current_target_);
+    ui::EventSourceTestApi event_source_test(event_source);
+    ui::EventDispatchDetails details = event_source_test.SendEventToSink(event);
+    if (details.dispatcher_destroyed)
+      current_target_ = nullptr;
+  }
 }
 
 void EventGenerator::Init(gfx::NativeWindow root_window,
@@ -722,41 +696,5 @@ gfx::Point EventGenerator::CenterOfWindow(const EventTarget* window) const {
   return delegate()->CenterOfTarget(window);
 }
 
-void EventGenerator::DoDispatchEvent(ui::Event* event, bool async) {
-  if (event->IsTouchEvent()) {
-    ui::TouchEvent* touch_event = static_cast<ui::TouchEvent*>(event);
-    touch_pointer_details_.id = touch_event->pointer_details().id;
-    touch_event->SetPointerDetailsForTest(touch_pointer_details_);
-  }
-
-  if (async) {
-    std::unique_ptr<ui::Event> pending_event = ui::Event::Clone(*event);
-    if (pending_events_.empty()) {
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE, base::BindOnce(&EventGenerator::DispatchNextPendingEvent,
-                                    base::Unretained(this)));
-    }
-    pending_events_.push_back(std::move(pending_event));
-  } else if (!event->handled()) {
-    ui::EventSource* event_source = delegate()->GetEventSource(current_target_);
-    ui::EventSourceTestApi event_source_test(event_source);
-    ui::EventDispatchDetails details = event_source_test.SendEventToSink(event);
-    if (details.dispatcher_destroyed)
-      current_target_ = nullptr;
-  }
-}
-
-void EventGenerator::DispatchNextPendingEvent() {
-  DCHECK(!pending_events_.empty());
-  ui::Event* event = pending_events_.front().get();
-  DoDispatchEvent(event, false);
-  pending_events_.pop_front();
-  if (!pending_events_.empty()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(&EventGenerator::DispatchNextPendingEvent,
-                                  base::Unretained(this)));
-  }
-}
-
 }  // namespace test
 }  // namespace ui

ui/events/test/event_generator.h

@@ -5,7 +5,6 @@
 #ifndef UI_EVENTS_TEST_EVENT_GENERATOR_H_
 #define UI_EVENTS_TEST_EVENT_GENERATOR_H_
 
-#include <list>
 #include <memory>
 #include <vector>
 
@@ -18,10 +17,6 @@
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/native_widget_types.h"
 
-namespace gfx {
-class PointF;
-}
-
 namespace ui {
 class EventSource;
 class EventTarget;
@@ -138,9 +133,6 @@ class EventGenerator {
   }
   const gfx::Point& current_location() const { return current_location_; }
 
-  void set_async(bool async) { async_ = async; }
-  bool async() const { return async_; }
-
   // Events could be dispatched using different methods. The choice is a
   // tradeoff between test robustness and coverage of OS internals that affect
   // event dispatch.
@@ -411,13 +403,6 @@ class EventGenerator {
                       int steps,
                       int num_fingers);
 
-  // Generates scroll sequences of a FlingCancel, Scrolls, FlingStart, sending
-  // scrolls of each of the values in |offsets|.
-  void ScrollSequence(const gfx::Point& start,
-                      const base::TimeDelta& step_delay,
-                      const std::vector<gfx::PointF>& offsets,
-                      int num_fingers);
-
   // Generate a TrackPad "rest" event. That is, a user resting fingers on the
   // trackpad without moving. This may then be followed by a ScrollSequence(),
   // or a CancelTrackpadRest().
@@ -463,9 +448,6 @@ class EventGenerator {
   gfx::Point GetLocationInCurrentRoot() const;
   gfx::Point CenterOfWindow(const EventTarget* window) const;
 
-  void DispatchNextPendingEvent();
-  void DoDispatchEvent(Event* event, bool async);
-
   std::unique_ptr<EventGeneratorDelegate> delegate_;
   gfx::Point current_location_;
   EventTarget* current_target_ = nullptr;
@@ -474,11 +456,6 @@ class EventGenerator {
 
   ui::PointerDetails touch_pointer_details_;
 
-  std::list<std::unique_ptr<Event>> pending_events_;
-
-  // Set to true to cause events to be posted asynchronously.
-  bool async_ = false;
-
   // Whether to skip mapping of coordinates from the root window to a hit window
   // when dispatching events.
   bool assume_window_at_origin_ = true;