Suppressed MEs for gestures from cancelled PEs

third_party/WebKit/Source/core/input/EventHandler.cpp

Index: third_party/WebKit/Source/core/input/EventHandler.cpp
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp
index bad5b0dc36e3628c3abf728963fe111b80484d2d..e008c516e360f405ce9b8be6f35c8d42a7b08071 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -215,6 +215,7 @@ EventHandler::EventHandler(LocalFrame* frame)
     , m_longTapShouldInvokeContextMenu(false)
     , m_activeIntervalTimer(this, &EventHandler::activeIntervalTimerFired)
     , m_lastShowPressTimestamp(0)
+    , m_suppressMouseEventsFromGestures(false)
 {
 }
 
@@ -281,6 +282,7 @@ void EventHandler::clear()
     m_longTapShouldInvokeContextMenu = false;
     m_dragStartPos = LayoutPoint();
     m_mouseDown = PlatformMouseEvent();
+    m_suppressMouseEventsFromGestures = false;
 }
 
 WebInputEventResult EventHandler::mergeEventResult(
@@ -1809,6 +1811,8 @@ WebInputEventResult EventHandler::handleGestureEventInFrame(const GestureEventWi
     }
 
     switch (gestureEvent.type()) {
+    case PlatformEvent::GestureTapDown:
+        return handleGestureTapDown(targetedEvent);
     case PlatformEvent::GestureTap:
         return handleGestureTap(targetedEvent);
     case PlatformEvent::GestureShowPress:
@@ -1819,7 +1823,6 @@ WebInputEventResult EventHandler::handleGestureEventInFrame(const GestureEventWi
         return handleGestureLongTap(targetedEvent);
     case PlatformEvent::GestureTwoFingerTap:
         return sendContextMenuEventForGesture(targetedEvent);
-    case PlatformEvent::GestureTapDown:
     case PlatformEvent::GesturePinchBegin:
     case PlatformEvent::GesturePinchEnd:
     case PlatformEvent::GesturePinchUpdate:
@@ -1840,6 +1843,13 @@ WebInputEventResult EventHandler::handleGestureScrollEvent(const PlatformGesture
     return m_scrollManager.handleGestureScrollEvent(gestureEvent);
 }
 
+WebInputEventResult EventHandler::handleGestureTapDown(const GestureEventWithHitTestResults& targetedEvent)
+{
+    m_suppressMouseEventsFromGestures =
+        m_pointerEventManager.primaryPointerdownCanceled(targetedEvent.event().uniqueTouchEventId());
+    return WebInputEventResult::NotHandled;
+}
+
 WebInputEventResult EventHandler::handleGestureTap(const GestureEventWithHitTestResults& targetedEvent)
 {
     FrameView* frameView(m_frame->view());
@@ -1856,12 +1866,15 @@ WebInputEventResult EventHandler::handleGestureTap(const GestureEventWithHitTest
     // co-ordinates outside the target's bounds.
     IntPoint adjustedPoint = frameView->rootFrameToContents(gestureEvent.position());
 
-    unsigned modifiers = gestureEvent.getModifiers();
-    PlatformMouseEvent fakeMouseMove(gestureEvent.position(), gestureEvent.globalPosition(),
-        NoButton, PlatformEvent::MouseMoved, /* clickCount */ 0,
-        static_cast<PlatformEvent::Modifiers>(modifiers),
-        PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
-    dispatchMouseEvent(EventTypeNames::mousemove, currentHitTest.innerNode(), 0, fakeMouseMove);
+    const unsigned modifiers = gestureEvent.getModifiers();
+
+    if (!m_suppressMouseEventsFromGestures) {
+        PlatformMouseEvent fakeMouseMove(gestureEvent.position(), gestureEvent.globalPosition(),
+            NoButton, PlatformEvent::MouseMoved, /* clickCount */ 0,
+            static_cast<PlatformEvent::Modifiers>(modifiers),
+            PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
+        dispatchMouseEvent(EventTypeNames::mousemove, currentHitTest.innerNode(), 0, fakeMouseMove);
+    }
 
     // Do a new hit-test in case the mousemove event changed the DOM.
     // Note that if the original hit test wasn't over an element (eg. was over a scrollbar) we
@@ -1889,13 +1902,19 @@ WebInputEventResult EventHandler::handleGestureTap(const GestureEventWithHitTest
     PlatformMouseEvent fakeMouseDown(gestureEvent.position(), gestureEvent.globalPosition(),
         LeftButton, PlatformEvent::MousePressed, gestureEvent.tapCount(),
         static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::LeftButtonDown),
-        PlatformMouseEvent::FromTouch,  gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
-    WebInputEventResult mouseDownEventResult = dispatchMouseEvent(EventTypeNames::mousedown, currentHitTest.innerNode(), gestureEvent.tapCount(), fakeMouseDown);
-    selectionController().initializeSelectionState();
-    if (mouseDownEventResult == WebInputEventResult::NotHandled)
-        mouseDownEventResult = handleMouseFocus(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest), InputDeviceCapabilities::firesTouchEventsSourceCapabilities());
-    if (mouseDownEventResult == WebInputEventResult::NotHandled)
-        mouseDownEventResult = handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest));
+        PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
+
+    // TODO(mustaq): We suppress MEs plus all it's side effects. What would that
+    // mean for for TEs?  What's the right balance here? crbug.com/617255
+    WebInputEventResult mouseDownEventResult = WebInputEventResult::NotHandled;

[Rick Byers, 2016/06/09 16:07:33]

nit: this local isn't used outside the below scope - move the declaration into
the scope of the if.

[mustaq, 2016/06/09 16:41:03]

The mergeEventResult calls at the end combine all the individual results,
including this one, to get the "highest" result. If you think we can skip the
mousedown there, I will add a TODO to avoid changing any behavior in this CL).

[Rick Byers, 2016/06/09 17:01:33]

Oh sorry, my search missed it (due to wrapping in rietveld) - I was surprised
not to find that.  It's probably only contextual search that relies on the
return value from this function, and in that case cancelling the pointerDown
should definitely prevent CS UI from appearing (so you really don't want to
return NotHandled in that case).

[mustaq, 2016/06/09 17:17:27]

Acknowledged.

+    if (!m_suppressMouseEventsFromGestures) {
+        mouseDownEventResult = dispatchMouseEvent(EventTypeNames::mousedown, currentHitTest.innerNode(), gestureEvent.tapCount(), fakeMouseDown);
+        selectionController().initializeSelectionState();
+        if (mouseDownEventResult == WebInputEventResult::NotHandled)
+            mouseDownEventResult = handleMouseFocus(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest), InputDeviceCapabilities::firesTouchEventsSourceCapabilities());
+        if (mouseDownEventResult == WebInputEventResult::NotHandled)
+            mouseDownEventResult = handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest));
+    }
 
     if (currentHitTest.innerNode()) {
         ASSERT(gestureEvent.type() == PlatformEvent::GestureTap);
@@ -1912,11 +1931,14 @@ WebInputEventResult EventHandler::handleGestureTap(const GestureEventWithHitTest
         adjustedPoint = frameView->rootFrameToContents(gestureEvent.position());
         currentHitTest = hitTestResultInFrame(m_frame, adjustedPoint, hitType);
     }
+
     PlatformMouseEvent fakeMouseUp(gestureEvent.position(), gestureEvent.globalPosition(),
         LeftButton, PlatformEvent::MouseReleased, gestureEvent.tapCount(),
         static_cast<PlatformEvent::Modifiers>(modifiers),
-        PlatformMouseEvent::FromTouch,  gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
-    WebInputEventResult mouseUpEventResult = dispatchMouseEvent(EventTypeNames::mouseup, currentHitTest.innerNode(), gestureEvent.tapCount(), fakeMouseUp);
+        PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
+    WebInputEventResult mouseUpEventResult = WebInputEventResult::NotHandled;
+    if (!m_suppressMouseEventsFromGestures)
+        mouseUpEventResult = dispatchMouseEvent(EventTypeNames::mouseup, currentHitTest.innerNode(), gestureEvent.tapCount(), fakeMouseUp);
 
     WebInputEventResult clickEventResult = WebInputEventResult::NotHandled;
     if (m_clickNode) {
@@ -1933,7 +1955,7 @@ WebInputEventResult EventHandler::handleGestureTap(const GestureEventWithHitTest
         m_clickNode = nullptr;
     }
 
-    if (mouseUpEventResult == WebInputEventResult::NotHandled)
+    if (!m_suppressMouseEventsFromGestures && mouseUpEventResult == WebInputEventResult::NotHandled)

[Rick Byers, 2016/06/09 16:07:33]

nit: you can simplify this slightly and have the exact same behavior by
initializing mouseUpEventResult to handledSuppressed, then you don't need to
re-check m_suppressMouseEventsFromGestures here (and logically it makes sense -
if m_suppressMouseEventsFromGestures is true, then the mouseUp event WASN'T "not
handled", it was never sent in the first place which is what "suppressed"
means).

[mustaq, 2016/06/09 16:41:03]

Done.

Dave, are you okay with this? This only affects the canceled pointerdown case,
so no regression from caller's perspective.

[dtapuska, 2016/06/09 16:55:02]

That is alright. Like I said we could interpret it either way. 

         mouseUpEventResult = handleMouseReleaseEvent(MouseEventWithHitTestResults(fakeMouseUp, currentHitTest));
 
     WebInputEventResult eventResult = mergeEventResult(mergeEventResult(mouseDownEventResult, mouseUpEventResult), clickEventResult);
@@ -1960,6 +1982,8 @@ WebInputEventResult EventHandler::handleGestureLongPress(const GestureEventWithH
 
     m_longTapShouldInvokeContextMenu = false;
     if (m_frame->settings() && m_frame->settings()->touchDragDropEnabled() && m_frame->view()) {
+        // TODO(mustaq): Suppressing long-tap MouseEvents could break
+        // drag-drop. Will do separately because of the risk. crbug.com/606938.
         PlatformMouseEvent mouseDownEvent(adjustedPoint, gestureEvent.globalPosition(), LeftButton, PlatformEvent::MousePressed, 1,
             static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::LeftButtonDown),
             PlatformMouseEvent::FromTouch, WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
@@ -3089,7 +3113,6 @@ HitTestResult EventHandler::hitTestResultInFrame(LocalFrame* frame, const Layout
 WebInputEventResult EventHandler::handleTouchEvent(const PlatformTouchEvent& event)
 {
     TRACE_EVENT0("blink", "EventHandler::handleTouchEvent");
-
     return m_pointerEventManager.handleTouchEvents(event);
 }