Bundle white listed touch action into the compositor ack

With kCompositorTouchAction turned on, a touch event is acked from the
compositor thread instead of the main thread. Then the white listed
touch action is sent from the compositor to the browser from a separate
IPC. There is a potential race condition here, that the white listed
touch action would arrive before or after the ack. It could also happen
that the white listed touch action arrives after some gesture events
being dispatched, for example, the GSB might have reached the
TouchActionFilter while the white listed touch action is not arrived yet.

To solve this problem, this CL puts the white listed touch action as
a callback parameter, and send it through the ack, to the browser.

Bug: 905325, 876323
Change-Id: Ia9d0bb02b57291d2333f6cfe6959430ff41b5084
Reviewed-on: https://chromium-review.googlesource.com/c/1336071
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609429}

content/browser/renderer_host/input/input_router_impl.cc

@@ -276,6 +276,12 @@ void InputRouterImpl::SetTouchActionFromMain(cc::TouchAction touch_action) {
 void InputRouterImpl::SetWhiteListedTouchAction(cc::TouchAction touch_action,
                                                 uint32_t unique_touch_event_id,
                                                 InputEventAckState state) {
+  DCHECK(!compositor_touch_action_enabled_);
+  OnSetWhiteListedTouchAction(touch_action);
+}
+
+void InputRouterImpl::OnSetWhiteListedTouchAction(
+    cc::TouchAction touch_action) {
   touch_action_filter_.OnSetWhiteListedTouchAction(touch_action);
   client_->OnSetWhiteListedTouchAction(touch_action);
   if (compositor_touch_action_enabled_) {
@@ -574,8 +580,18 @@ void InputRouterImpl::TouchEventHandled(
   // The SetTouchAction IPC occurs on a different channel so always
   // send it in the input event ack to ensure it is available at the
   // time the ACK is handled.
-  if (touch_action.has_value())
-    OnSetTouchAction(touch_action.value());
+  if (touch_action.has_value()) {
+    if (!compositor_touch_action_enabled_) {
+      OnSetTouchAction(touch_action.value());
+    } else {
+      if (source == InputEventAckSource::COMPOSITOR_THREAD)
+        OnSetWhiteListedTouchAction(touch_action.value());
+      else if (source == InputEventAckSource::MAIN_THREAD)
+        OnSetTouchAction(touch_action.value());
+      else
+        NOTREACHED();
+    }
+  }
 
   bool should_stop_timeout_monitor =
       !compositor_touch_action_enabled_ ||

content/browser/renderer_host/input/input_router_impl.h

@@ -111,6 +111,7 @@ class CONTENT_EXPORT InputRouterImpl : public InputRouter,
   void ForceResetTouchActionForTest();
 
  private:
+  friend class InputRouterImplTest;
   friend class InputRouterImplTestBase;
   friend class MockRenderWidgetHost;
   friend class RenderWidgetHostSitePerProcessTest;
@@ -216,6 +217,7 @@ class CONTENT_EXPORT InputRouterImpl : public InputRouter,
       GestureEventWithLatencyInfo& gesture_event,
       const FilterGestureEventResult& existing_result);
   void ProcessDeferredGestureEventQueue();
+  void OnSetWhiteListedTouchAction(cc::TouchAction touch_action);
 
   InputRouterImplClient* client_;
   InputDispositionHandler* disposition_handler_;

content/browser/renderer_host/input/input_router_impl_unittest.cc

@@ -425,13 +425,14 @@ class InputRouterImplTestBase : public testing::Test {
     PressTouchPoint(1, 1);
     base::Optional<ui::DidOverscrollParams> overscroll;
     input_router_->SendTouchEvent(TouchEventWithLatencyInfo(touch_event_));
-    input_router_->TouchEventHandled(
-        TouchEventWithLatencyInfo(touch_event_), InputEventAckSource::BROWSER,
-        ui::LatencyInfo(), state, overscroll, touch_action);
+    input_router_->TouchEventHandled(TouchEventWithLatencyInfo(touch_event_),
+                                     InputEventAckSource::MAIN_THREAD,
+                                     ui::LatencyInfo(), state, overscroll,
+                                     touch_action);
     EXPECT_EQ(input_router_->touch_action_filter_.num_of_active_touches_, 1);
     ReleaseTouchPoint(0);
     input_router_->OnTouchEventAck(TouchEventWithLatencyInfo(touch_event_),
-                                   InputEventAckSource::BROWSER, state);
+                                   InputEventAckSource::MAIN_THREAD, state);
     EXPECT_EQ(input_router_->touch_action_filter_.num_of_active_touches_, 0);
   }
 
@@ -470,6 +471,14 @@ class InputRouterImplTest : public InputRouterImplTestBase,
     }
   }
 
+  base::Optional<cc::TouchAction> AllowedTouchAction() {
+    return input_router_->touch_action_filter_.allowed_touch_action_;
+  }
+
+  base::Optional<cc::TouchAction> WhiteListedTouchAction() {
+    return input_router_->touch_action_filter_.white_listed_touch_action_;
+  }
+
  protected:
   const bool compositor_touch_action_enabled_;
 
@@ -1272,7 +1281,7 @@ TEST_P(InputRouterImplTest,
   // kTouchActionNone should disable the timeout.
   CancelTouchTimeout();
   dispatched_messages[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
+      InputEventAckSource::MAIN_THREAD, ui::LatencyInfo(),
       INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
   EXPECT_EQ(1U, disposition_handler_->GetAndResetAckCount());
   EXPECT_FALSE(TouchEventTimeoutEnabled());
@@ -1356,7 +1365,7 @@ TEST_P(InputRouterImplTest, TouchActionResetBeforeEventReachesRenderer) {
   ASSERT_TRUE(touch_release_event2[0]->ToEvent());
 
   touch_press_event1[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
+      InputEventAckSource::MAIN_THREAD, ui::LatencyInfo(),
       INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
   touch_move_event1[0]->ToEvent()->CallCallback(INPUT_EVENT_ACK_STATE_CONSUMED);
 
@@ -1416,7 +1425,7 @@ TEST_P(InputRouterImplTest, TouchActionResetWhenTouchHasNoConsumer) {
   ASSERT_TRUE(touch_move_event1[0]->ToEvent());
   CancelTouchTimeout();
   touch_press_event1[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
+      InputEventAckSource::MAIN_THREAD, ui::LatencyInfo(),
       INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
   touch_move_event1[0]->ToEvent()->CallCallback(INPUT_EVENT_ACK_STATE_CONSUMED);
 
@@ -1490,7 +1499,7 @@ TEST_P(InputRouterImplTest, TouchActionResetWhenTouchHandlerRemoved) {
 
   // Ensure we have touch-action:none, suppressing scroll events.
   dispatched_messages[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
+      InputEventAckSource::MAIN_THREAD, ui::LatencyInfo(),
       INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
   EXPECT_EQ(0U, GetAndResetDispatchedMessages().size());
   dispatched_messages[1]->ToEvent()->CallCallback(
@@ -1577,7 +1586,7 @@ TEST_P(InputRouterImplTest, DoubleTapGestureDependsOnFirstTap) {
   ASSERT_EQ(1U, dispatched_messages.size());
   ASSERT_TRUE(dispatched_messages[0]->ToEvent());
   dispatched_messages[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
+      InputEventAckSource::MAIN_THREAD, ui::LatencyInfo(),
       INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
   ReleaseTouchPoint(0);
   SendTouchEvent();
@@ -1966,6 +1975,9 @@ TEST_P(InputRouterImplTest, OverscrollDispatch) {
 // Test proper routing of whitelisted touch action notifications received from
 // |SetWhiteListedTouchAction| IPC messages.
 TEST_P(InputRouterImplTest, OnSetWhiteListedTouchAction) {
+  // The white listed touch action is bundled in the ack.
+  if (compositor_touch_action_enabled_)
+    return;
   cc::TouchAction touch_action = cc::kTouchActionPanY;
   OnSetWhiteListedTouchAction(touch_action, 0,
                               INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
@@ -2021,14 +2033,24 @@ TEST_P(InputRouterImplTest, TouchActionInCallback) {
   DispatchedMessages dispatched_messages = GetAndResetDispatchedMessages();
   ASSERT_EQ(1U, dispatched_messages.size());
   ASSERT_TRUE(dispatched_messages[0]->ToEvent());
+  InputEventAckSource source = InputEventAckSource::MAIN_THREAD;
+  base::Optional<cc::TouchAction> expected_touch_action = cc::kTouchActionPan;
+  if (compositor_touch_action_enabled_)
+    source = InputEventAckSource::COMPOSITOR_THREAD;
   dispatched_messages[0]->ToEvent()->CallCallback(
-      InputEventAckSource::COMPOSITOR_THREAD, ui::LatencyInfo(),
-      INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt, cc::kTouchActionNone);
+      source, ui::LatencyInfo(), INPUT_EVENT_ACK_STATE_CONSUMED, base::nullopt,
+      expected_touch_action);
   ASSERT_EQ(1U, disposition_handler_->GetAndResetAckCount());
-  base::Optional<cc::TouchAction> allowed_touch_action =
-      input_router_->AllowedTouchAction();
-  DCHECK(allowed_touch_action.has_value());
-  EXPECT_EQ(cc::TouchAction::kTouchActionNone, allowed_touch_action.value());
+  base::Optional<cc::TouchAction> allowed_touch_action = AllowedTouchAction();
+  base::Optional<cc::TouchAction> white_listed_touch_action =
+      WhiteListedTouchAction();
+  if (compositor_touch_action_enabled_) {
+    EXPECT_FALSE(allowed_touch_action.has_value());
+    EXPECT_EQ(expected_touch_action, white_listed_touch_action);
+  } else {
+    EXPECT_FALSE(white_listed_touch_action.has_value());
+    EXPECT_EQ(expected_touch_action, allowed_touch_action);
+  }
 }
 
 namespace {

content/browser/renderer_host/input/touch_action_filter.h

@@ -76,6 +76,7 @@ class CONTENT_EXPORT TouchActionFilter {
   void AppendToGestureSequenceForDebugging(const char* str);
 
  private:
+  friend class InputRouterImplTest;
   friend class InputRouterImplTestBase;
   friend class MockRenderWidgetHost;
   friend class TouchActionFilterTest;

content/renderer/input/widget_input_handler_manager.cc

@@ -276,6 +276,10 @@ void WidgetInputHandlerManager::SetWhiteListedTouchAction(
     cc::TouchAction touch_action,
     uint32_t unique_touch_event_id,
     ui::InputHandlerProxy::EventDisposition event_disposition) {
+  if (base::FeatureList::IsEnabled(features::kCompositorTouchAction)) {
+    white_listed_touch_action_ = touch_action;
+    return;
+  }
   mojom::WidgetInputHandlerHost* host = GetWidgetInputHandlerHost();
   if (!host)
     return;
@@ -478,6 +482,10 @@ void WidgetInputHandlerManager::HandledInputEvent(
   if (!callback)
     return;
 
+  if (!touch_action.has_value()) {
+    touch_action = white_listed_touch_action_;
+    white_listed_touch_action_.reset();
+  }
   // This method is called from either the main thread or the compositor thread.
   bool is_compositor_thread = compositor_task_runner_ &&
                               compositor_task_runner_->BelongsToCurrentThread();

content/renderer/input/widget_input_handler_manager.h

@@ -146,6 +146,8 @@ class CONTENT_EXPORT WidgetInputHandlerManager
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;
 
+  base::Optional<cc::TouchAction> white_listed_touch_action_;
+
 #if defined(OS_ANDROID)
   std::unique_ptr<SynchronousCompositorProxyRegistry>
       synchronous_compositor_registry_;