PointerEvents for long-press: fix double firing & canceling MEs

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

Index: third_party/WebKit/Source/core/input/GestureManager.cpp
diff --git a/third_party/WebKit/Source/core/input/GestureManager.cpp b/third_party/WebKit/Source/core/input/GestureManager.cpp
index 2ac482815ca50c85dcf664618d8124a06caae12f..f8e1e91fb931858ccb3d6b4a84e7da6fba592b8d 100644
--- a/third_party/WebKit/Source/core/input/GestureManager.cpp
+++ b/third_party/WebKit/Source/core/input/GestureManager.cpp
@@ -104,7 +104,7 @@ WebInputEventResult GestureManager::handleGestureEventInFrame(const GestureEvent
     case PlatformEvent::GestureLongTap:
         return handleGestureLongTap(targetedEvent);
     case PlatformEvent::GestureTwoFingerTap:
-        return m_frame->eventHandler().sendContextMenuEventForGesture(targetedEvent);
+        return handleGestureTwoFingerTap(targetedEvent);
     case PlatformEvent::GesturePinchBegin:
     case PlatformEvent::GesturePinchEnd:
     case PlatformEvent::GesturePinchUpdate:
@@ -266,7 +266,7 @@ WebInputEventResult GestureManager::handleGestureLongPress(const GestureEventWit
         return WebInputEventResult::HandledSystem;
     }
 
-    return m_frame->eventHandler().sendContextMenuEventForGesture(targetedEvent);
+    return sendContextMenuEventForGesture(targetedEvent);
 }
 
 WebInputEventResult GestureManager::handleGestureLongTap(const GestureEventWithHitTestResults& targetedEvent)
@@ -274,12 +274,61 @@ WebInputEventResult GestureManager::handleGestureLongTap(const GestureEventWithH
 #if !OS(ANDROID)
     if (m_longTapShouldInvokeContextMenu) {
         m_longTapShouldInvokeContextMenu = false;
-        return m_frame->eventHandler().sendContextMenuEventForGesture(targetedEvent);
+        return sendContextMenuEventForGesture(targetedEvent);
     }
 #endif
     return WebInputEventResult::NotHandled;
 }
 
+WebInputEventResult GestureManager::handleGestureTwoFingerTap(const GestureEventWithHitTestResults& targetedEvent)
+{
+    return sendContextMenuEventForGesture(targetedEvent);
+}
+
+WebInputEventResult GestureManager::sendContextMenuEventForGesture(const GestureEventWithHitTestResults& targetedEvent)
+{
+    const PlatformGestureEvent& gestureEvent = targetedEvent.event();
+    unsigned modifiers = gestureEvent.getModifiers();
+
+    if (!m_suppressMouseEventsFromGestures) {
+        // Send MouseMoved event prior to handling (https://crbug.com/485290).
+        PlatformMouseEvent fakeMouseMove(gestureEvent.position(), gestureEvent.globalPosition(),
+            NoButton, PlatformEvent::MouseMoved, /* clickCount */ 0,
+            static_cast<PlatformEvent::Modifiers>(modifiers),
+            PlatformMouseEvent::FromTouch, gestureEvent.timestamp(), WebPointerProperties::PointerType::Mouse);
+
+        m_frame->eventHandler().dispatchMouseEvent(EventTypeNames::mousemove, targetedEvent.hitTestResult().innerNode(), 0, fakeMouseMove);
+    }
+
+    PlatformEvent::EventType eventType = PlatformEvent::MousePressed;
+    if (m_frame->settings() && m_frame->settings()->showContextMenuOnMouseUp())
+        eventType = PlatformEvent::MouseReleased;
+
+    // To simulate right-click behavior, we send a right mouse down and then
+    // context menu event.
+    // TODO(crbug.com/579564): Maybe we should not send mouse down at all
+    PlatformMouseEvent mouseEvent(targetedEvent.event().position(), targetedEvent.event().globalPosition(), RightButton, eventType, 1,
+        static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::RightButtonDown),
+        PlatformMouseEvent::FromTouch, WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
+
+    if (!m_suppressMouseEventsFromGestures && m_frame->view()) {
+        MouseEventWithHitTestResults mev(mouseEvent, targetedEvent.hitTestResult());

[dtapuska, 2016/07/18 21:16:31]

Is it safe to reuse the hit test result here? The other code in EventHandler
doesn't. What if the DOM mutates do to the mouse move? What about targeting
scrollbars on the two finger tap?

[mustaq, 2016/07/19 15:50:27]

Brought back the hittest, got distracted by the fixme comment.
I followed Rick's suggestion here which makes more sense to me: keeping only
the minimal effects of mouse for gestures. Do you think we should call
passMousePressEventToScrollbar() for all affected gestures (long-press,
long-tap & two-finger tap)?

[mustaq, 2016/07/19 15:52:32]

Note that we don't passMousePressEventToScrollbar() on GestureTap.

[mustaq, 2016/07/20 15:54:41]

Bad news: the cherry-picking of DOM events is causing a regression in CrOS:
long-press menu doesn't appear until touch-release.

I will go back to a minimal fix as in Patch1 (except that I will use the
"FromTouch" value in the PlatformMouseEvent instead of a bool param), to have
the fix landed before the hackatahon next week.

+        WebInputEventResult eventResult = m_frame->eventHandler().dispatchMouseEvent(EventTypeNames::mousedown, mev.innerNode(), /* clickCount */ 0, mouseEvent);
+
+        if (eventResult == WebInputEventResult::NotHandled) {
+            InputDeviceCapabilities* sourceCapabilities = mouseEvent.getSyntheticEventType() == PlatformMouseEvent::FromTouch ? InputDeviceCapabilities::firesTouchEventsSourceCapabilities() : InputDeviceCapabilities::doesntFireTouchEventsSourceCapabilities();
+            eventResult = m_frame->eventHandler().handleMouseFocus(mev, sourceCapabilities);
+        }
+
+        if (eventResult == WebInputEventResult::NotHandled)
+            m_frame->eventHandler().handleMousePressEvent(mev);
+    }
+
+    return m_frame->eventHandler().sendContextMenuEvent(mouseEvent);
+    // We do not need to send a corresponding mouse release because in case of
+    // right-click, the context menu takes capture and consumes all events.
+}
+
 WebInputEventResult GestureManager::handleGestureShowPress()
 {
     m_lastShowPressTimestamp = WTF::monotonicallyIncreasingTime();