Don't logically coalesce touchpad pinch events with scroll updates

Unlike touchscreen pinch events, touchpad pinch events don't occur
within a gesture scroll sequence. If a touchpad pinch update couldn't
be directly coalesced with a previous pinch update, we now just enqueue
the pinch update rather than producing a scroll update.

We also now allow for pinch updates to be coalesced if their anchors are
approximately equal to accommodate OOPIF coordinate conversion
imprecision.

Bug: 897791
Change-Id: I3331fc5761b5ca11d4b05386e5ab4468ac2291d7
Reviewed-on: https://chromium-review.googlesource.com/c/1349851Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613245}

ui/events/blink/blink_event_util.cc

@@ -456,9 +456,16 @@ bool CanCoalesce(const WebGestureEvent& event_to_coalesce,
 
   // GesturePinchUpdate scales can be combined only if they share a focal point,
   // e.g., with double-tap drag zoom.
+  // Due to the imprecision of OOPIF coordinate conversions, the positions may
+  // not be exactly equal, so we only require approximate equality.
+  constexpr float kAnchorTolerance = 1.f;
   if (event.GetType() == WebInputEvent::kGesturePinchUpdate &&
-      event.PositionInWidget() == event_to_coalesce.PositionInWidget())
+      (std::abs(event.PositionInWidget().x -
+                event_to_coalesce.PositionInWidget().x) < kAnchorTolerance) &&
+      (std::abs(event.PositionInWidget().y -
+                event_to_coalesce.PositionInWidget().y) < kAnchorTolerance)) {
     return true;
+  }
 
   return false;
 }
@@ -575,9 +582,10 @@ void Coalesce(const blink::WebInputEvent& event_to_coalesce,
   }
 }
 
-// Whether |event_in_queue| is GesturePinchUpdate or GestureScrollUpdate and
-// has the same modifiers/source as the new scroll/pinch event. Compatible
-// scroll and pinch event pairs can be logically coalesced.
+// Whether |event_in_queue| is a touchscreen GesturePinchUpdate or
+// GestureScrollUpdate and has the same modifiers/source as the new
+// scroll/pinch event. Compatible touchscreen scroll and pinch event pairs
+// can be logically coalesced.
 bool IsCompatibleScrollorPinch(const WebGestureEvent& new_event,
                                const WebGestureEvent& event_in_queue) {
   DCHECK(new_event.GetType() == WebInputEvent::kGestureScrollUpdate ||
@@ -589,7 +597,8 @@ bool IsCompatibleScrollorPinch(const WebGestureEvent& new_event,
   return (event_in_queue.GetType() == WebInputEvent::kGestureScrollUpdate ||
           event_in_queue.GetType() == WebInputEvent::kGesturePinchUpdate) &&
          event_in_queue.GetModifiers() == new_event.GetModifiers() &&
-         event_in_queue.SourceDevice() == new_event.SourceDevice();
+         event_in_queue.SourceDevice() == blink::kWebGestureDeviceTouchscreen &&
+         new_event.SourceDevice() == blink::kWebGestureDeviceTouchscreen;
 }
 
 std::pair<WebGestureEvent, WebGestureEvent> CoalesceScrollAndPinch(

ui/events/blink/blink_event_util_unittest.cc

@@ -226,6 +226,56 @@ TEST(BlinkEventUtilTest, WebGestureEventCoalescing) {
   EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event));
 }
 
+TEST(BlinkEventUtilTest, GesturePinchUpdateCoalescing) {
+  gfx::PointF position(10.f, 10.f);
+  blink::WebGestureEvent coalesced_event(
+      blink::WebInputEvent::kGesturePinchUpdate,
+      blink::WebInputEvent::kNoModifiers,
+      blink::WebInputEvent::GetStaticTimeStampForTests(),
+      blink::kWebGestureDeviceTouchpad);
+  coalesced_event.data.pinch_update.scale = 1.1f;
+  coalesced_event.SetPositionInWidget(position);
+
+  blink::WebGestureEvent event_to_be_coalesced(coalesced_event);
+
+  ASSERT_TRUE(CanCoalesce(event_to_be_coalesced, coalesced_event));
+  Coalesce(event_to_be_coalesced, &coalesced_event);
+  EXPECT_FLOAT_EQ(1.21, coalesced_event.data.pinch_update.scale);
+
+  // Allow the updates to be coalesced if the anchors are nearly equal.
+  position.Offset(0.1f, 0.1f);
+  event_to_be_coalesced.SetPositionInWidget(position);
+  coalesced_event.data.pinch_update.scale = 1.1f;
+  ASSERT_TRUE(CanCoalesce(event_to_be_coalesced, coalesced_event));
+  Coalesce(event_to_be_coalesced, &coalesced_event);
+  EXPECT_FLOAT_EQ(1.21, coalesced_event.data.pinch_update.scale);
+
+  // The anchors are no longer considered equal, so don't coalesce.
+  position.Offset(1.f, 1.f);
+  event_to_be_coalesced.SetPositionInWidget(position);
+  EXPECT_FALSE(CanCoalesce(event_to_be_coalesced, coalesced_event));
+
+  // Don't logically coalesce touchpad pinch events as touchpad pinch events
+  // don't occur within a gesture scroll sequence.
+  EXPECT_FALSE(
+      IsCompatibleScrollorPinch(event_to_be_coalesced, coalesced_event));
+
+  // Touchscreen pinch events can be logically coalesced.
+  coalesced_event.SetSourceDevice(blink::kWebGestureDeviceTouchscreen);
+  event_to_be_coalesced.SetSourceDevice(blink::kWebGestureDeviceTouchscreen);
+  coalesced_event.data.pinch_update.scale = 1.1f;
+  ASSERT_TRUE(
+      IsCompatibleScrollorPinch(event_to_be_coalesced, coalesced_event));
+
+  blink::WebGestureEvent logical_scroll, logical_pinch;
+  std::tie(logical_scroll, logical_pinch) =
+      CoalesceScrollAndPinch(nullptr, coalesced_event, event_to_be_coalesced);
+  ASSERT_EQ(blink::WebInputEvent::kGestureScrollUpdate,
+            logical_scroll.GetType());
+  ASSERT_EQ(blink::WebInputEvent::kGesturePinchUpdate, logical_pinch.GetType());
+  EXPECT_FLOAT_EQ(1.21, logical_pinch.data.pinch_update.scale);
+}
+
 TEST(BlinkEventUtilTest, MouseEventCoalescing) {
   blink::WebMouseEvent coalesced_event;
   coalesced_event.SetType(blink::WebInputEvent::kMouseMove);

ui/events/blink/compositor_thread_event_queue.cc

@@ -19,8 +19,9 @@ void CompositorThreadEventQueue::Queue(
     base::TimeTicks timestamp_now) {
   if (queue_.empty() ||
       !IsContinuousGestureEvent(new_event->event().GetType()) ||
-      !IsCompatibleScrollorPinch(ToWebGestureEvent(new_event->event()),
-                                 ToWebGestureEvent(queue_.back()->event()))) {
+      !(queue_.back()->CanCoalesceWith(*new_event) ||
+        IsCompatibleScrollorPinch(ToWebGestureEvent(new_event->event()),
+                                  ToWebGestureEvent(queue_.back()->event())))) {
     if (new_event->first_original_event()) {
       // Trace could be nested as there might be multiple events in queue.
       // e.g. |ScrollUpdate|, |ScrollEnd|, and another scroll sequence.

ui/events/blink/input_handler_proxy_unittest.cc

@@ -1645,6 +1645,41 @@ TEST_F(InputHandlerProxyEventQueueTest, VSyncAlignedCoalesceScrollAndPinch) {
   testing::Mock::VerifyAndClearExpectations(&mock_input_handler_);
 }
 
+TEST_F(InputHandlerProxyEventQueueTest, VSyncAlignedCoalesceTouchpadPinch) {
+  EXPECT_CALL(mock_input_handler_, PinchGestureBegin());
+  EXPECT_CALL(mock_input_handler_, SetNeedsAnimateInput());
+
+  HandleGestureEventWithSourceDevice(WebInputEvent::kGesturePinchBegin,
+                                     blink::kWebGestureDeviceTouchpad);
+  HandleGestureEventWithSourceDevice(WebInputEvent::kGesturePinchUpdate,
+                                     blink::kWebGestureDeviceTouchpad, 1.1f, 10,
+                                     20);
+  // The second update should coalesce with the first.
+  HandleGestureEventWithSourceDevice(WebInputEvent::kGesturePinchUpdate,
+                                     blink::kWebGestureDeviceTouchpad, 1.1f, 10,
+                                     20);
+  // The third update has a different anchor so it should not be coalesced.
+  HandleGestureEventWithSourceDevice(WebInputEvent::kGesturePinchUpdate,
+                                     blink::kWebGestureDeviceTouchpad, 1.1f, 11,
+                                     21);
+  HandleGestureEventWithSourceDevice(WebInputEvent::kGesturePinchEnd,
+                                     blink::kWebGestureDeviceTouchpad);
+
+  // Only the PinchBegin was dispatched.
+  EXPECT_EQ(3ul, event_queue().size());
+  EXPECT_EQ(1ul, event_disposition_recorder_.size());
+
+  ASSERT_EQ(WebInputEvent::kGesturePinchUpdate,
+            event_queue()[0]->event().GetType());
+  EXPECT_FLOAT_EQ(
+      1.21f,
+      ToWebGestureEvent(event_queue()[0]->event()).data.pinch_update.scale);
+  EXPECT_EQ(WebInputEvent::kGesturePinchUpdate,
+            event_queue()[1]->event().GetType());
+  EXPECT_EQ(WebInputEvent::kGesturePinchEnd,
+            event_queue()[2]->event().GetType());
+}
+
 TEST_F(InputHandlerProxyEventQueueTest, OriginalEventsTracing) {
   // Handle scroll on compositor.
   cc::InputHandlerScrollResult scroll_result_did_scroll_;