Process touch events during the tab-dragging window move

When the touch events move very quickly on detaching, some
touch events are already queued, those queued touch events
aren't processed properly.

- Detach() aren't releasing the capture on the source widget.
  This means those queued touch events are targeted to the
  source widget instead of the moving widget, but the touch
  history is already transferred to the moving widget, which
  makes troubles on gesture recognition. We can release the
  capture on the detach on this specific situation.
- the recognized gestures are sent to the root window of
  the moving widget and nobody can handle. The new class
  RemainingGestureEventHandler handles them.
- RemainingGestureEventHandler should not handle events
  originated from ET_TOUCH_CANCELLED -- some test cases
  actually fail if so. ET_TOUCH_CANCELLED can happen internally
  within the browser (like Window::CleanupGestureState()),
  but that's not the intention of this class.

Bug: 943316
Test: the new test case in interactive_ui_tests
Change-Id: I679562d7707342874960dd7ff4d70d749f1ad437
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1545113Reviewed-by: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#647789}

chrome/browser/ui/views/tabs/tab_drag_controller.cc

@@ -1333,7 +1333,16 @@ void TabDragController::DetachIntoNewBrowserAndRunMoveLoop(
       ui::TransferTouchesBehavior::kDontCancel);
 #endif
 
+#if defined(OS_CHROMEOS)
+  // On ChromeOS, Detach should release capture; |can_release_capture_| is
+  // false on ChromeOS because it can cancel touches, but for this cases
+  // the touches are already transferred, so releasing is fine. Without
+  // releasing, the capture remains and further touch events can be sent to a
+  // wrong target.
+  Detach(RELEASE_CAPTURE);
+#else
   Detach(can_release_capture_ ? RELEASE_CAPTURE : DONT_RELEASE_CAPTURE);
+#endif
 
   dragged_widget->SetCanAppearInExistingFullscreenSpaces(true);
   dragged_widget->SetVisibilityChangedAnimationsEnabled(false);
@@ -1370,6 +1379,16 @@ void TabDragController::RunMoveLoop(const gfx::Vector2d& drag_offset) {
     attached_tabstrip_->GetWidget()->ReleaseCapture();
     attached_tabstrip_->OwnDragController(this);
   }
+#if defined(OS_CHROMEOS)
+  // When the window service is used, there's some chance of having gesture
+  // events in the attached (moving) widget during the window move loop. Without
+  // setting the mouse handler, such gestures might arrive to a wrong view. See
+  // https://crbug.com/943316.
+  if (features::IsUsingWindowService()) {
+    attached_tabstrip_->GetWidget()->GetRootView()->SetMouseHandler(
+        attached_tabstrip_);
+  }
+#endif
   const views::Widget::MoveLoopSource move_loop_source =
       event_source_ == EVENT_SOURCE_MOUSE ?
       views::Widget::MOVE_LOOP_SOURCE_MOUSE :

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

@@ -3112,6 +3112,36 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTestTouch,
   ui::SetEventTickClockForTesting(nullptr);
 }
 
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTestTouch,
+                       FlingOnStartingDrag) {
+  SetMinFlingVelocity(1);
+  AddTabAndResetBrowser(browser());
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  const gfx::Point tab_0_center =
+      GetCenterInScreenCoordinates(tab_strip->tab_at(0));
+  const gfx::Vector2d detach(0, GetDetachY(tab_strip));
+
+  // Sends events to the server without waiting for its reply, which will cause
+  // extra touch events before PerformWindowMove starts handling events.
+  test::QuitDraggingObserver observer;
+  base::SimpleTestTickClock clock;
+  clock.SetNowTicks(base::TimeTicks::Now());
+  ui::SetEventTickClockForTesting(&clock);
+  ASSERT_TRUE(PressInput(tab_0_center));
+  clock.Advance(base::TimeDelta::FromMilliseconds(5));
+  ASSERT_TRUE(DragInputToAsync(tab_0_center + detach));
+  clock.Advance(base::TimeDelta::FromMilliseconds(5));
+  ASSERT_TRUE(DragInputToAsync(tab_0_center + detach + detach));
+  clock.Advance(base::TimeDelta::FromMilliseconds(2));
+  ASSERT_TRUE(ReleaseInput());
+  observer.Wait();
+
+  ASSERT_FALSE(tab_strip->IsDragSessionActive());
+  ASSERT_FALSE(TabDragController::IsActive());
+  EXPECT_EQ(2u, browser_list->size());
+  ui::SetEventTickClockForTesting(nullptr);
+}
+
 #endif  // OS_CHROMEOS
 
 #if defined(OS_CHROMEOS)

ui/aura/mus/window_tree_client_unittest.cc

@@ -2825,6 +2825,86 @@ TEST_F(WindowTreeClientTest, SecondPerformWindowMoveIsNotAllowed) {
   EXPECT_TRUE(last_result);
 }
 
+TEST_F(WindowTreeClientTest, PerformWindowMoveHandlingQueuedTouches) {
+  auto top_level = CreateTopLevel();
+  window_tree()->AckAllChanges();
+  test::EventCountDelegate delegate1;
+  test::EventCountDelegate delegate2;
+  std::unique_ptr<aura::Window> window1(
+      CreateNormalWindow(10, host()->window(), &delegate1));
+  std::unique_ptr<aura::Window> window2(
+      CreateNormalWindow(20, top_level->host->window(), &delegate2));
+  window_tree()->AckAllChanges();
+
+  std::unique_ptr<ui::Event> touch = std::make_unique<ui::TouchEvent>(
+      ui::ET_TOUCH_PRESSED, window1->bounds().CenterPoint(),
+      ui::EventTimeForNow(), ui::PointerDetails(), 0);
+  window_tree_client()->OnWindowInputEvent(
+      1, server_id(window1.get()), host()->GetDisplayId(), std::move(touch), 0);
+  EXPECT_LT(0, delegate1.GetGestureCountAndReset());
+  EXPECT_EQ(0, delegate2.GetGestureCountAndReset());
+
+  window1->env()->gesture_recognizer()->TransferEventsTo(
+      window1.get(), window2.get(), ui::TransferTouchesBehavior::kDontCancel);
+  top_level->host->PerformWindowMove(
+      window2.get(), ws::mojom::MoveLoopSource::TOUCH, gfx::Point(), HTCAPTION,
+      base::BindOnce([](bool success) {}));
+
+  // Sending touch event to window1, but window2 should receive the gestures.
+  touch = std::make_unique<ui::TouchEvent>(
+      ui::ET_TOUCH_MOVED, window1->bounds().origin(), ui::EventTimeForNow(),
+      ui::PointerDetails(), 0);
+  window_tree_client()->OnWindowInputEvent(
+      2, server_id(window1.get()), host()->GetDisplayId(), std::move(touch), 0);
+  EXPECT_EQ(0, delegate1.GetGestureCountAndReset());
+  EXPECT_LT(0, delegate2.GetGestureCountAndReset());
+
+  // Finishes the window move.
+  window_tree()->AckAllChanges();
+  window1->env()->gesture_recognizer()->TransferEventsTo(
+      window2.get(), window1.get(), ui::TransferTouchesBehavior::kDontCancel);
+  touch = std::make_unique<ui::TouchEvent>(
+      ui::ET_TOUCH_RELEASED, window1->bounds().CenterPoint(),
+      ui::EventTimeForNow(), ui::PointerDetails(), 0);
+  window_tree_client()->OnWindowInputEvent(
+      3, server_id(window1.get()), host()->GetDisplayId(), std::move(touch), 0);
+  EXPECT_LT(0, delegate1.GetGestureCountAndReset());
+  EXPECT_EQ(0, delegate2.GetGestureCountAndReset());
+}
+
+TEST_F(WindowTreeClientTest, CleanupGestureEventsDuringWindowMove) {
+  auto top_level = CreateTopLevel();
+  window_tree()->AckAllChanges();
+  test::EventCountDelegate delegate1;
+  test::EventCountDelegate delegate2;
+  std::unique_ptr<aura::Window> window1(
+      CreateNormalWindow(10, host()->window(), &delegate1));
+  std::unique_ptr<aura::Window> window2(
+      CreateNormalWindow(20, top_level->host->window(), &delegate2));
+  window_tree()->AckAllChanges();
+
+  std::unique_ptr<ui::Event> touch = std::make_unique<ui::TouchEvent>(
+      ui::ET_TOUCH_PRESSED, window1->bounds().CenterPoint(),
+      ui::EventTimeForNow(), ui::PointerDetails(), 0);
+  window_tree_client()->OnWindowInputEvent(
+      1, server_id(window1.get()), host()->GetDisplayId(), std::move(touch), 0);
+  EXPECT_LT(0, delegate1.GetGestureCountAndReset());
+  EXPECT_EQ(0, delegate2.GetGestureCountAndReset());
+
+  window1->env()->gesture_recognizer()->TransferEventsTo(
+      window1.get(), window2.get(), ui::TransferTouchesBehavior::kDontCancel);
+  top_level->host->PerformWindowMove(
+      window2.get(), ws::mojom::MoveLoopSource::TOUCH, gfx::Point(), HTCAPTION,
+      base::BindOnce([](bool success) {}));
+
+  // CleanupGestureState will cancel the active touches but should not create
+  // additional gesture events.
+  window_tree_client()->CleanupGestureState(server_id(window2.get()));
+  EXPECT_EQ(0, delegate1.GetGestureCountAndReset());
+  EXPECT_EQ(0, delegate2.GetGestureCountAndReset());
+  window_tree()->AckAllChanges();
+}
+
 // Verifies occlusion state from server is applied to underlying window.
 TEST_F(WindowTreeClientTest, OcclusionStateFromServer) {
   struct {

ui/aura/mus/window_tree_host_mus.cc

@@ -16,6 +16,7 @@
 #include "ui/aura/mus/window_tree_host_mus_delegate.h"
 #include "ui/aura/mus/window_tree_host_mus_init_params.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_delegate.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tracker.h"
 #include "ui/aura/window_tree_host_observer.h"
@@ -45,6 +46,33 @@ DEFINE_UI_CLASS_PROPERTY_KEY(WindowTreeHostMus*, kWindowTreeHostMusKey, nullptr)
 // and increases).
 uint32_t next_accelerated_widget_id = std::numeric_limits<uint32_t>::max();
 
+// This class handles the gesture events occurring on the root window and sends
+// them to the content window during the window move. Typically gesture events
+// will stop arriving once PerformWindowMove is invoked, but sometimes events
+// are already queued and arrive to the root window. They should be handled
+// by the content window. See https://crbug.com/943316.
+class RemainingGestureEventHandler : public ui::EventHandler {
+ public:
+  RemainingGestureEventHandler(Window* content_window, Window* root)
+      : content_window_(content_window), root_(root) {
+    root_->AddPostTargetHandler(this);
+  }
+  ~RemainingGestureEventHandler() override {
+    root_->RemovePostTargetHandler(this);
+  }
+
+ private:
+  // ui::EventHandler:
+  void OnGestureEvent(ui::GestureEvent* event) override {
+    content_window_->delegate()->OnGestureEvent(event);
+  }
+
+  Window* content_window_;
+  Window* root_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemainingGestureEventHandler);
+};
+
 // ScopedTouchTransferController controls the transfer of touch events for
 // window move loop. It transfers touches before the window move starts, and
 // then transfers them back to the original window when the window move ends.
@@ -55,6 +83,7 @@ class ScopedTouchTransferController : public ui::GestureRecognizerObserver {
  public:
   ScopedTouchTransferController(Window* source, Window* dest)
       : tracker_({source, dest}),
+        remaining_gesture_event_handler_(source, dest),
         gesture_recognizer_(source->env()->gesture_recognizer()) {
     gesture_recognizer_->TransferEventsTo(
         source, dest, ui::TransferTouchesBehavior::kDontCancel);
@@ -87,7 +116,7 @@ class ScopedTouchTransferController : public ui::GestureRecognizerObserver {
   void OnActiveTouchesCanceled(ui::GestureConsumer* consumer) override {}
 
   WindowTracker tracker_;
-
+  RemainingGestureEventHandler remaining_gesture_event_handler_;
   ui::GestureRecognizer* gesture_recognizer_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedTouchTransferController);