Clear |held_move_event_| before processing it.

OnEventFromSource for the |held_move_event_| may not return if
one of the event handler creates a run loop -- this happens
on tab-dragging. As long as |held_move_event_| is valid, any
further touch moves are also held and not processed anymore.

Bug: b/119260190
Test: manually
Change-Id: Ibeacc167bbcf71d6d89a40ad2916ecf1f880998c
Reviewed-on: https://chromium-review.googlesource.com/c/1330061Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607023}

ui/aura/window_event_dispatcher.cc

@@ -837,15 +837,19 @@ ui::EventDispatchDetails WindowEventDispatcher::DispatchHeldEvents() {
   }
 
   if (held_move_event_) {
+    // |held_move_event_| should be cleared here. Some event handler can
+    // create its own run loop on an event (e.g. WindowMove loop for
+    // tab-dragging), which means the other move events need to be processed
+    // before this OnEventFromSource() finishes. See also b/119260190.
+    std::unique_ptr<ui::LocatedEvent> event = std::move(held_move_event_);
+
     // If a mouse move has been synthesized, the target location is suspect,
     // so drop the held mouse event.
-    if (held_move_event_->IsTouchEvent() ||
-        (held_move_event_->IsMouseEvent() && !synthesize_mouse_move_)) {
-      dispatching_held_event_ = held_move_event_.get();
-      dispatch_details = OnEventFromSource(held_move_event_.get());
+    if (event->IsTouchEvent() ||
+        (event->IsMouseEvent() && !synthesize_mouse_move_)) {
+      dispatching_held_event_ = event.get();
+      dispatch_details = OnEventFromSource(event.get());
     }
-    if (!dispatch_details.dispatcher_destroyed)
-      held_move_event_.reset();
   }
 
   if (!dispatch_details.dispatcher_destroyed) {

ui/aura/window_event_dispatcher_unittest.cc

@@ -2210,6 +2210,77 @@ TEST_P(WindowEventDispatcherTest, CaptureWindowDestroyed) {
   EXPECT_EQ(NULL, capture_window_tracker.capture_window());
 }
 
+namespace {
+
+class RunLoopHandler : public ui::EventHandler {
+ public:
+  explicit RunLoopHandler(aura::Window* target)
+      : run_loop_(base::RunLoop::Type::kNestableTasksAllowed), target_(target) {
+    target_->AddPreTargetHandler(this);
+  }
+  ~RunLoopHandler() override { target_->RemovePreTargetHandler(this); }
+  int num_scroll_updates() const { return num_scroll_updates_; }
+
+ private:
+  // ui::EventHandler:
+  void OnGestureEvent(ui::GestureEvent* event) override {
+    if (event->type() != ui::ET_GESTURE_SCROLL_UPDATE)
+      return;
+    num_scroll_updates_++;
+    if (running_) {
+      run_loop_.QuitWhenIdle();
+    } else {
+      running_ = true;
+      run_loop_.Run();
+    }
+  }
+
+  base::RunLoop run_loop_;
+  bool running_ = false;
+  int num_scroll_updates_ = 0;
+
+  aura::Window* target_;
+
+  DISALLOW_COPY_AND_ASSIGN(RunLoopHandler);
+};
+
+}  // namespace
+
+TEST_P(WindowEventDispatcherTest, HeldTouchMoveWithRunLoop) {
+  RunLoopHandler handler(root_window());
+
+  host()->dispatcher()->HoldPointerMoves();
+
+  gfx::Point point = root_window()->GetBoundsInScreen().CenterPoint();
+  ui::TouchEvent ev0(ui::ET_TOUCH_PRESSED, point, ui::EventTimeForNow(),
+                     ui::PointerDetails());
+  DispatchEventUsingWindowDispatcher(&ev0);
+
+  point.Offset(10, 10);
+  ui::TouchEvent ev1(ui::ET_TOUCH_MOVED, point, ui::EventTimeForNow(),
+                     ui::PointerDetails());
+  DispatchEventUsingWindowDispatcher(&ev1);
+  // The move event is held, so SCROLL_UPDATE does not happen yet.
+  EXPECT_EQ(0, handler.num_scroll_updates());
+
+  // ReleasePointerMoves() will post DispatchHeldEvent() asynchronously.
+  host()->dispatcher()->ReleasePointerMoves();
+  point.Offset(10, 10);
+  // Schedule another move event which should cause another SCROLL_UPDATE and
+  // quit the run_loop within the handler.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindLambdaForTesting([&]() {
+        ui::TouchEvent ev2(ui::ET_TOUCH_MOVED, point, base::TimeTicks::Now(),
+                           ui::PointerDetails());
+        DispatchEventUsingWindowDispatcher(&ev2);
+      }));
+  // Wait for both DispatchHeldEvent() and dispatch of |ev2|.
+  base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed).RunUntilIdle();
+
+  // Makes sure that the run_loop ran and then ended.
+  EXPECT_EQ(2, handler.num_scroll_updates());
+}
+
 class ExitMessageLoopOnMousePress : public ui::test::TestEventHandler {
  public:
   ExitMessageLoopOnMousePress() {}