Fixes to propagation of touch events in system gesture handling.

  - Switches from detecting fling gestures to detecting touch events,
    making it possible to prevent the initial ET_TOUCH_PRESSED event
    from propagating (b/78461207)
  - Uses new handler interface, which includes HandleSideSwipeContinue,
    to allow gradual exposure of the bottom settings bar (b/78462154) and
    to allow using X delta moved instead of the gesture end event for back
    gesture detection in the future (b/78397215)

Bug: b/78462154
Bug: b/78461207
Change-Id: Ic64e13825eaaa5cf8420e94df767a057563a8281
Tested: manual on device and unit test
Reviewed-on: https://chromium-review.googlesource.com/1030614
Commit-Queue: Ryan Daum <rdaum@chromium.org>
Reviewed-by: Alex Sakhartchouk <alexst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556433}

chromecast/graphics/cast_system_gesture_event_handler.cc

@@ -66,65 +66,79 @@ CastSideSwipeOrigin CastSystemGestureEventHandler::GetDragPosition(
   return CastSideSwipeOrigin::NONE;
 }
 
-void CastSystemGestureEventHandler::OnEvent(ui::Event* event) {
-  if (current_swipe_ != CastSideSwipeOrigin::NONE &&
-      (event->IsTouchEvent() || event->IsGestureEvent())) {
-    // If we're in the process of handling a swipe, prevent the underlying touch
-    // or gesture event from being propagated out to other users.
-    event->StopPropagation();
-  }
-
-  // From here-on in, we're only interested in gestures.
-  if (!event->IsGestureEvent()) {
+void CastSystemGestureEventHandler::OnTouchEvent(ui::TouchEvent* event) {
+  if (swipe_gesture_handlers_.empty()) {
     return;
   }
-  ui::GestureEvent* gesture_event = event->AsGestureEvent();
-  gfx::Point gesture_location(gesture_event->location());
-  aura::Window* target = static_cast<aura::Window*>(event->target());
 
+  gfx::Point touch_location(event->location());
+  aura::Window* target = static_cast<aura::Window*>(event->target());
   // Convert the event's point to the point on the physical screen.
   // For cast on root window this is likely going to be identical, but put it
   // through the ScreenPositionClient just to be sure.
-  ::wm::ConvertPointToScreen(target, &gesture_location);
+  ::wm::ConvertPointToScreen(target, &touch_location);
   gfx::Rect screen_bounds = display::Screen::GetScreen()
-                                ->GetDisplayNearestPoint(gesture_location)
+                                ->GetDisplayNearestPoint(touch_location)
                                 .bounds();
   CastSideSwipeOrigin side_swipe_origin =
-      GetDragPosition(gesture_location, screen_bounds);
-
-  // Detect the beginning of a system gesture swipe.
-  if (side_swipe_origin != CastSideSwipeOrigin::NONE &&
-      (event->type() == ui::ET_SCROLL_FLING_START ||
-       event->type() == ui::ET_GESTURE_BEGIN)) {
+      GetDragPosition(touch_location, screen_bounds);
+
+  // A located event has occurred inside the margin. It might be the start of
+  // our gesture, or a touch that we need to squash.
+  if (current_swipe_ == CastSideSwipeOrigin::NONE &&
+      side_swipe_origin != CastSideSwipeOrigin::NONE) {
+    // Check to see if we have any potential consumers of events on this side.
+    // If not, we can continue on without consuming it.
+    bool have_swipe_consumer = false;
     for (auto* side_swipe_handler : swipe_gesture_handlers_) {
-      // Let the subscriber know about the swipe. If it is actually consumed by
-      // them, it will be marked as handled.
-      side_swipe_handler->OnSideSwipeBegin(side_swipe_origin, gesture_event);
-
-      // If handled, remember the origin and then stop the further propagation
-      // of the event.
-      if (event->handled()) {
-        // Record the swipe origin to properly fire OnSideSwipeEnd when the
-        // swipe gesture finishes.
-        current_swipe_ = side_swipe_origin;
-        event->StopPropagation();
-
+      if (side_swipe_handler->CanHandleSwipe(side_swipe_origin)) {
+        have_swipe_consumer = true;
         break;
       }
     }
+    if (!have_swipe_consumer) {
+      return;
+    }
+
+    // All touch or gesture events inside the margins have to be consumed, or we
+    // risk a state issue later when the touch ends (b/78461207)
+    event->StopPropagation();
+
+    // Detect the beginning of a system gesture swipe.
+    if (event->type() == ui::ET_TOUCH_PRESSED) {
+      current_swipe_ = side_swipe_origin;
+      for (auto* side_swipe_handler : swipe_gesture_handlers_) {
+        // Let the subscriber know about the gesture begin.
+        side_swipe_handler->HandleSideSwipeBegin(side_swipe_origin,
+                                                 touch_location);
+      }
+    }
+
+    return;
+  }
+
+  if (current_swipe_ == CastSideSwipeOrigin::NONE) {
     return;
   }
 
-  // Detect the end of a system gesture swipe.
-  if (current_swipe_ != CastSideSwipeOrigin::NONE &&
-      event->type() == ui::ET_GESTURE_END) {
+  // A swipe is in progress, or has completed, so stop propagation of underlying
+  // gesture/touch events.
+  event->StopPropagation();
+
+  // The system gesture has ended.
+  if (event->type() == ui::ET_TOUCH_RELEASED) {
     for (auto* side_swipe_handler : swipe_gesture_handlers_) {
-      side_swipe_handler->OnSideSwipeEnd(current_swipe_, gesture_event);
+      side_swipe_handler->HandleSideSwipeEnd(current_swipe_, touch_location);
     }
     current_swipe_ = CastSideSwipeOrigin::NONE;
 
-    // Prevent the gesture from being used for other gesture uses.
-    target->CleanupGestureState();
+    return;
+  }
+
+  // The system gesture is ongoing...
+  for (auto* side_swipe_handler : swipe_gesture_handlers_) {
+    // Let the subscriber know about the gesture begin.
+    side_swipe_handler->HandleSideSwipeContinue(current_swipe_, touch_location);
   }
 }
 

chromecast/graphics/cast_system_gesture_event_handler.h

@@ -41,7 +41,7 @@ class CastSystemGestureEventHandler : public ui::EventHandler {
   CastSideSwipeOrigin GetDragPosition(const gfx::Point& point,
                                       const gfx::Rect& screen_bounds) const;
 
-  void OnEvent(ui::Event* event) override;
+  void OnTouchEvent(ui::TouchEvent* event) override;
 
  private:
   const int gesture_start_width_;

chromecast/graphics/cast_system_gesture_event_handler_test.cc

@@ -21,6 +21,7 @@ namespace {
 constexpr base::TimeDelta kTimeDelay = base::TimeDelta::FromMilliseconds(100);
 constexpr int kSwipeDistance = 50;
 constexpr int kNumSteps = 5;
+constexpr gfx::Point kZeroPoint{0, 0};
 
 }  // namespace
 
@@ -50,56 +51,61 @@ class TestEventGeneratorDelegate
 class TestSideSwipeGestureHandler
     : public CastSideSwipeGestureHandlerInterface {
  public:
+  TestSideSwipeGestureHandler()
+      : begin_swipe_point_(kZeroPoint), end_swipe_point_(kZeroPoint) {}
+
   ~TestSideSwipeGestureHandler() override = default;
 
-  void OnSideSwipeBegin(CastSideSwipeOrigin swipe_origin,
-                        ui::GestureEvent* gesture_event) override {
+  bool CanHandleSwipe(CastSideSwipeOrigin swipe_origin) override {
+    return handle_swipe_;
+  }
+
+  void HandleSideSwipeBegin(CastSideSwipeOrigin swipe_origin,
+                            const gfx::Point& touch_location) override {
     if (handle_swipe_) {
       begin_swipe_origin_ = swipe_origin;
-      begin_gesture_event_ = gesture_event;
-
-      gesture_event->SetHandled();
+      begin_swipe_point_ = touch_location;
     }
   }
 
-  void OnSideSwipeEnd(CastSideSwipeOrigin swipe_origin,
-                      ui::GestureEvent* gesture_event) override {
+  void HandleSideSwipeEnd(CastSideSwipeOrigin swipe_origin,
+                          const gfx::Point& gesture_event) override {
     end_swipe_origin_ = swipe_origin;
-    end_gesture_event_ = gesture_event;
+    end_swipe_point_ = gesture_event;
   }
 
   void SetHandleSwipe(bool handle_swipe) { handle_swipe_ = handle_swipe; }
 
   CastSideSwipeOrigin begin_swipe_origin() const { return begin_swipe_origin_; }
-  ui::GestureEvent* begin_gesture_event() const { return begin_gesture_event_; }
+  gfx::Point begin_swipe_point() const { return begin_swipe_point_; }
 
   CastSideSwipeOrigin end_swipe_origin() const { return end_swipe_origin_; }
-  ui::GestureEvent* end_gesture_event() const { return end_gesture_event_; }
+  gfx::Point end_swipe_point() const { return end_swipe_point_; }
 
  private:
   bool handle_swipe_ = true;
 
   CastSideSwipeOrigin begin_swipe_origin_ = CastSideSwipeOrigin::NONE;
-  ui::GestureEvent* begin_gesture_event_ = nullptr;
+  gfx::Point begin_swipe_point_;
 
   CastSideSwipeOrigin end_swipe_origin_ = CastSideSwipeOrigin::NONE;
-  ui::GestureEvent* end_gesture_event_ = nullptr;
+  gfx::Point end_swipe_point_;
 };
 
 // Event sink to check for events that get through (or don't get through) after
 // the system gesture handler handles them.
 class TestEventHandler : public ui::EventHandler {
  public:
-  TestEventHandler() : EventHandler(), num_gesture_events_received_(0) {}
+  TestEventHandler() : EventHandler(), num_touch_events_received_(0) {}
 
-  void OnGestureEvent(ui::GestureEvent* event) override {
-    num_gesture_events_received_++;
+  void OnTouchEvent(ui::TouchEvent* event) override {
+    num_touch_events_received_++;
   }
 
-  int NumGestureEventsReceived() const { return num_gesture_events_received_; }
+  int NumTouchEventsReceived() const { return num_touch_events_received_; }
 
  private:
-  int num_gesture_events_received_;
+  int num_touch_events_received_;
 };
 
 class CastSystemGestureEventHandlerTest : public aura::test::AuraTestBase {
@@ -157,11 +163,11 @@ class CastSystemGestureEventHandlerTest : public aura::test::AuraTestBase {
 TEST_F(CastSystemGestureEventHandlerTest, Initialization) {
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_EQ(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_EQ(0, test_event_handler().NumTouchEventsReceived());
 }
 
 // A swipe in the middle of the screen should produce no system gesture.
@@ -176,11 +182,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeWithNoSystemGesture) {
 
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_NE(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_NE(0, test_event_handler().NumTouchEventsReceived());
 }
 
 TEST_F(CastSystemGestureEventHandlerTest, SwipeFromLeft) {
@@ -193,11 +199,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeFromLeft) {
 
   EXPECT_EQ(CastSideSwipeOrigin::LEFT,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::LEFT,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_EQ(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_EQ(0, test_event_handler().NumTouchEventsReceived());
 }
 
 TEST_F(CastSystemGestureEventHandlerTest, SwipeFromRight) {
@@ -211,11 +217,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeFromRight) {
 
   EXPECT_EQ(CastSideSwipeOrigin::RIGHT,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::RIGHT,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_EQ(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_EQ(0, test_event_handler().NumTouchEventsReceived());
 }
 
 TEST_F(CastSystemGestureEventHandlerTest, SwipeFromTop) {
@@ -228,11 +234,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeFromTop) {
 
   EXPECT_EQ(CastSideSwipeOrigin::TOP,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::TOP,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_EQ(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_EQ(0, test_event_handler().NumTouchEventsReceived());
 }
 
 TEST_F(CastSystemGestureEventHandlerTest, SwipeFromBottom) {
@@ -246,11 +252,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeFromBottom) {
 
   EXPECT_EQ(CastSideSwipeOrigin::BOTTOM,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::BOTTOM,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_NE(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_EQ(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_NE(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_EQ(0, test_event_handler().NumTouchEventsReceived());
 }
 
 // Test that ignoring the gesture at its beginning will make it so the swipe
@@ -268,11 +274,11 @@ TEST_F(CastSystemGestureEventHandlerTest, SwipeUnhandledIgnored) {
 
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().begin_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().begin_gesture_event());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().begin_swipe_point());
   EXPECT_EQ(CastSideSwipeOrigin::NONE,
             test_gesture_handler().end_swipe_origin());
-  EXPECT_EQ(nullptr, test_gesture_handler().end_gesture_event());
-  EXPECT_NE(0, test_event_handler().NumGestureEventsReceived());
+  EXPECT_EQ(kZeroPoint, test_gesture_handler().end_swipe_point());
+  EXPECT_NE(0, test_event_handler().NumTouchEventsReceived());
 }
 
 }  // namespace test