Reland "Process touch events during the tab-dragging window move"

This is a reland of 402ea29b

Original change was reverted due to msan failures. Indeed it
didn't take care of the case of window destruction during
move. This CL includes its fix.

Original change's description:
> 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/+/1545113
> Reviewed-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}

Bug: 943316
Change-Id: Ice077b4e53309a2a59b254581da7e6dd0f3b409c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553171
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648357}

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,44 @@ 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, WindowObserver {
+ public:
+  RemainingGestureEventHandler(Window* content_window, Window* root)
+      : content_window_({content_window}), root_(root) {
+    root_->AddPostTargetHandler(this);
+    root_->AddObserver(this);
+  }
+  ~RemainingGestureEventHandler() override {
+    if (root_)
+      StopObserving();
+  }
+
+ private:
+  void StopObserving() {
+    root_->RemoveObserver(this);
+    root_->RemovePostTargetHandler(this);
+    root_ = nullptr;
+  }
+
+  // ui::EventHandler:
+  void OnGestureEvent(ui::GestureEvent* event) override {
+    if (!content_window_.windows().empty())
+      (*content_window_.windows().begin())->delegate()->OnGestureEvent(event);
+  }
+  // WindowObserver:
+  void OnWindowDestroying(Window* window) override { StopObserving(); }
+
+  WindowTracker 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 +94,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 +127,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);