Set correct whitelisted touch action in fling

A touch event can be blocking or non-blocking. They can be sent to the
main thread if the compositor determines that the main thread must know
about the event. If the event is sent as blocking, the browser will
avoid dispatching further events until the main thread responds by
ACK'ing the event. This allows author script to call preventDefault to
change how gestures are dispatched. A non-blocking event is sent to the
main thread but as purely informational; the main thread won't ACK the
event and the browser won't block before dispatching successive gesture
events

The compositor can turn a blocking touch event into a non-blocking one.
This is done while a fling is in progress to prevent janking fling
boosting (see https://crbug.com/595327). In this case, if a user touches
down during a fling we want the browser to dispatch scroll events
immediately without waiting on an ACK so that the scroll can continue or
boost the fling without interruption. The compositor does this by
setting the touch as DID_NOT_HANDLE_NON_BLOCKING_DUE_TO_FLING which will
cause it to be forwarded as non-blocking to the main thread.

Compositor touch action[1] lets the browser receive an ACK from the
compositor to let it know a set of "safe" whitelisted touch actions that
can be dispatched before the ACK from the main thread arrives. This
allows scrolls to be dispatched with less latency in some cases if the
compositor can determine that the main thread won't block certain touch
actions. The whitelisted touch action is sent back to the browser at the
same time as the event is forwarded to the main thread.

The bug here is caused by the whitelisted touch action not accounting
for the "non-blocking fling" behavior. When the touchstart hits a
blocking region (e.g. non-passive event listener) the whitelisted touch
action set to kNone (the compositor can't guarantee the main thread wont
block any gestures)[2,3]. However, if we're in a fling the event will be
made non-blocking so that the main thread *cannot* block scrolls. Worse,
because it's non-blocking the main thread won't ACK the event at all so
the browser will block dispatching any gestures until a new one is
started.

This CL fixes the issue by setting the whitelisted touch action to kAuto
(allow all) when we enter a "non-blocking due to fling" state.


[1] https://docs.google.com/document/d/1bSv_qNsLLnpXImuH7sHa6D3R_0GdDYkTiNjxso8Rgxo/edit#heading=h.jvxrm66og6z0
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_object.h?l=2229&rcl=b9275f87a13183e295a7a55bc6cef6eaea4ed86c
[3] https://cs.chromium.org/chromium/src/ui/events/blink/input_handler_proxy.cc?l=1016&rcl=25ed318ba5c24057ea2a402c62f25649fda5093f

Bug: 1048098
Change-Id: Ifa2fdd2fb7124f6f350f05e08f905229731297bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132843Reviewed-by: Xida Chen <xidachen@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756833}

content/browser/renderer_host/input/touch_action_filter.cc

@@ -130,6 +130,13 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
     case WebInputEvent::kGestureScrollBegin: {
       // In VR or virtual keyboard (https://crbug.com/880701),
       // GestureScrollBegin could come without GestureTapDown.
+      // TODO(bokan): This can also happen due to the fling controller
+      // filtering out the GestureTapDown due to tap suppression (i.e. tapping
+      // during a fling should stop the fling, not be sent to the page). We
+      // should not reset the touch action in this case! We currently work
+      // around this by resetting the whitelisted touch action from the
+      // compositor in this case as well but we should investigate not
+      // filtering the TapDown.
       if (!gesture_sequence_in_progress_) {
         gesture_sequence_in_progress_ = true;
         if (allowed_touch_action_.has_value()) {

content/renderer/input/main_thread_event_queue.cc

@@ -301,7 +301,7 @@ void MainThreadEventQueue::HandleEvent(
 
   HandledEventCallback event_callback;
   if (!non_blocking) {
-    TRACE_EVENT_INSTANT0("input", "NonBlocking", TRACE_EVENT_SCOPE_THREAD);
+    TRACE_EVENT_INSTANT0("input", "Blocking", TRACE_EVENT_SCOPE_THREAD);
     event_callback = std::move(callback);
   }
 

ui/events/blink/input_handler_proxy.cc

@@ -1120,8 +1120,17 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleTouchStart(
 
   bool is_in_inertial_scrolling_on_impl =
       in_inertial_scrolling_ && handling_gesture_on_impl_thread_;
-  if (is_in_inertial_scrolling_on_impl && is_touching_scrolling_layer)
+  if (is_in_inertial_scrolling_on_impl && is_touching_scrolling_layer) {
+    // If the touchstart occurs during a fling, it will be ACK'd immediately
+    // and it and its following touch moves will be dispatched as non-blocking.
+    // Due to tap suppression on the browser side, this will reset the
+    // browser-side touch action (see comment in
+    // TouchActionFilter::FilterGestureEvent for GestureScrollBegin). Ensure we
+    // send back a white_listed_touch_action that matches this non-blocking
+    // behavior rather than treating it as if it'll block.
+    white_listed_touch_action = cc::TouchAction::kAuto;
     result = DID_NOT_HANDLE_NON_BLOCKING_DUE_TO_FLING;
+  }
 
   client_->SetWhiteListedTouchAction(white_listed_touch_action,
                                      touch_event.unique_touch_event_id, result);

ui/events/blink/input_handler_proxy_unittest.cc

@@ -49,9 +49,15 @@ using blink::WebMouseEvent;
 using blink::WebMouseWheelEvent;
 using blink::WebTouchEvent;
 using blink::WebTouchPoint;
+using cc::InputHandler;
+using cc::ScrollBeginThreadState;
+using cc::TouchAction;
 using testing::_;
 using testing::DoAll;
 using testing::Field;
+using testing::Mock;
+using testing::Return;
+using testing::SetArgPointee;
 
 namespace ui {
 namespace test {
@@ -494,12 +500,15 @@ class InputHandlerProxyEventQueueTest : public testing::Test {
                                           float delta_y_or_scale = 0,
                                           int x = 0,
                                           int y = 0) {
+    InjectInputEvent(CreateGestureScrollPinch(
+        type, source_device, input_handler_proxy_.tick_clock_->NowTicks(),
+        delta_y_or_scale, x, y));
+  }
+
+  void InjectInputEvent(WebScopedInputEvent event) {
     LatencyInfo latency;
     input_handler_proxy_.HandleInputEventWithLatencyInfo(
-        CreateGestureScrollPinch(type, source_device,
-                                 input_handler_proxy_.tick_clock_->NowTicks(),
-                                 delta_y_or_scale, x, y),
-        latency,
+        std::move(event), latency,
         base::BindOnce(
             &InputHandlerProxyEventQueueTest::DidHandleInputEventAndOverscroll,
             weak_ptr_factory_.GetWeakPtr()));
@@ -559,6 +568,10 @@ class InputHandlerProxyEventQueueTest : public testing::Test {
         ->GeneratePrediction(WebInputEvent::GetStaticTimeStampForTests());
   }
 
+  base::TimeTicks NowTimestampForEvents() {
+    return input_handler_proxy_.tick_clock_->NowTicks();
+  }
+
  protected:
   testing::StrictMock<MockInputHandler> mock_input_handler_;
   testing::StrictMock<MockInputHandlerProxyClient> mock_client_;
@@ -1322,6 +1335,103 @@ TEST_P(InputHandlerProxyTest, HitTestTouchEventNonNullTouchAction) {
   VERIFY_AND_RESET_MOCKS();
 }
 
+// Tests that the whitelisted touch action is correctly set when a touch is
+// made non blocking due to an ongoing fling. https://crbug.com/1048098.
+TEST_F(InputHandlerProxyEventQueueTest, AckTouchActionNonBlockingForFling) {
+  // Simulate starting a compositor scroll and then flinging. This is setup for
+  // the real checks below.
+  {
+    float delta = 10;
+
+    // ScrollBegin
+    {
+      EXPECT_CALL(mock_input_handler_, ScrollBegin(_, _))
+          .WillOnce(Return(kImplThreadScrollState));
+      EXPECT_CALL(
+          mock_input_handler_,
+          RecordScrollBegin(_, ScrollBeginThreadState::kScrollingOnCompositor))
+          .Times(1);
+
+      HandleGestureEvent(WebInputEvent::kGestureScrollBegin, delta);
+      Mock::VerifyAndClearExpectations(&mock_input_handler_);
+    }
+
+    // ScrollUpdate
+    {
+      EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput()).Times(1);
+      EXPECT_CALL(mock_input_handler_, ScrollingShouldSwitchtoMainThread())
+          .WillOnce(Return(false));
+      EXPECT_CALL(mock_input_handler_, ScrollUpdate(_, _)).Times(1);
+
+      HandleGestureEvent(WebInputEvent::kGestureScrollUpdate, delta);
+      DeliverInputForBeginFrame();
+      Mock::VerifyAndClearExpectations(&mock_input_handler_);
+    }
+
+    // Start a fling - ScrollUpdate with momentum
+    {
+      cc::InputHandlerScrollResult scroll_result_did_scroll;
+      scroll_result_did_scroll.did_scroll = true;
+      EXPECT_CALL(mock_input_handler_, ScrollUpdate(_, _))
+          .WillOnce(Return(scroll_result_did_scroll));
+      EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput()).Times(1);
+      EXPECT_CALL(mock_input_handler_, ScrollingShouldSwitchtoMainThread())
+          .WillOnce(Return(false));
+      EXPECT_CALL(mock_input_handler_,
+                  GetSnapFlingInfoAndSetAnimatingSnapTarget(_, _, _))
+          .WillOnce(Return(false));
+
+      auto gsu_fling = CreateGestureScrollPinch(
+          WebInputEvent::kGestureScrollUpdate, WebGestureDevice::kTouchscreen,
+          NowTimestampForEvents(), delta, /*x=*/0, /*y=*/0);
+      static_cast<blink::WebGestureEvent*>(gsu_fling.get())
+          ->data.scroll_update.inertial_phase =
+          WebGestureEvent::InertialPhaseState::kMomentum;
+      InjectInputEvent(std::move(gsu_fling));
+      DeliverInputForBeginFrame();
+    }
+  }
+
+  // We're now in an active gesture fling. Simulate the user touching down on
+  // the screen. If this touch hits a blocking region (e.g. touch-action or a
+  // non-passive touchstart listener), we won't actually treat it as blocking;
+  // because of the ongoing fling it will be treated as non blocking. However,
+  // we also have to ensure that the whitelisted_touch_action reported is also
+  // kAuto so that the browser knows that it shouldn't wait for an ACK with an
+  // allowed touch-action before dispatching more scrolls.
+  {
+    // Simulate hitting a blocking region on the scrolling layer, as if there
+    // was a non-passive touchstart handler.
+    EXPECT_CALL(mock_input_handler_,
+                EventListenerTypeForTouchStartOrMoveAt(_, _))
+        .WillOnce(DoAll(SetArgPointee<1>(TouchAction::kNone),
+                        Return(InputHandler::TouchStartOrMoveEventListenerType::
+                                   HANDLER_ON_SCROLLING_LAYER)));
+
+    std::unique_ptr<WebTouchEvent> touch_start =
+        std::make_unique<WebTouchEvent>(
+            WebInputEvent::kTouchStart, WebInputEvent::kNoModifiers,
+            WebInputEvent::GetStaticTimeStampForTests());
+    touch_start->touches_length = 1;
+    touch_start->touch_start_or_first_touch_move = true;
+    touch_start->touches[0] =
+        CreateWebTouchPoint(WebTouchPoint::kStatePressed, 10, 10);
+
+    // This is the call this test is checking: we expect that the client will
+    // report the touch as non-blocking and also that the whitelisted touch
+    // action matches the non blocking expectatithe whitelisted touch action
+    // matches the non blocking expectation (i.e. all touches are allowed).
+    EXPECT_CALL(
+        mock_client_,
+        SetWhiteListedTouchAction(
+            TouchAction::kAuto, touch_start->unique_touch_event_id,
+            InputHandlerProxy::DID_NOT_HANDLE_NON_BLOCKING_DUE_TO_FLING))
+        .WillOnce(Return());
+
+    InjectInputEvent(std::move(touch_start));
+  }
+}
+
 TEST_P(InputHandlerProxyTest, HitTestTouchEventNullTouchAction) {
   // One of the touch points is on a touch-region. So the event should be sent
   // to the main thread.