Fixed & refactored mouse event firing at gesture context menu

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 8d4d8390eb015b979de2c88dc43261cbc08c0766..a1aa32035c43f8a94b42d338232d25da45742e57 100644
--- a/third_party/WebKit/Source/core/input/GestureManager.cpp
+++ b/third_party/WebKit/Source/core/input/GestureManager.cpp
@@ -187,7 +187,7 @@ WebInputEventResult GestureManager::handleGestureTap(const GestureEventWithHitTe
         mouseDownEventResult = m_frame->eventHandler().dispatchMouseEvent(EventTypeNames::mousedown, currentHitTest.innerNode(), gestureEvent.tapCount(), fakeMouseDown);
         m_selectionController->initializeSelectionState();
         if (mouseDownEventResult == WebInputEventResult::NotHandled)
-            mouseDownEventResult = m_frame->eventHandler().handleMouseFocus(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest), InputDeviceCapabilities::firesTouchEventsSourceCapabilities());
+            mouseDownEventResult = m_frame->eventHandler().handleMouseFocus(currentHitTest, InputDeviceCapabilities::firesTouchEventsSourceCapabilities());
         if (mouseDownEventResult == WebInputEventResult::NotHandled)
             mouseDownEventResult = m_frame->eventHandler().handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown, currentHitTest));
     }
@@ -303,20 +303,26 @@ WebInputEventResult GestureManager::sendContextMenuEventForGesture(const Gesture
     PlatformEvent::EventType eventType = PlatformEvent::MousePressed;
     if (m_frame->settings() && m_frame->settings()->showContextMenuOnMouseUp())
         eventType = PlatformEvent::MouseReleased;
+    PlatformMouseEvent mouseEvent(targetedEvent.event().position(), targetedEvent.event().globalPosition(), WebPointerProperties::Button::NoButton,
+        eventType, /* clickCount */ 0, static_cast<PlatformEvent::Modifiers>(modifiers), PlatformMouseEvent::FromTouch,
+        WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
 
-    // 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(), WebPointerProperties::Button::Right, eventType, 1,
-        static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::RightButtonDown),
-        PlatformMouseEvent::FromTouch, WTF::monotonicallyIncreasingTime(), WebPointerProperties::PointerType::Mouse);
-    if (!m_suppressMouseEventsFromGestures) {
-        // FIXME: Send HitTestResults to avoid redundant hit tests.
-        m_frame->eventHandler().handleMousePressEvent(mouseEvent);
+    if (!m_suppressMouseEventsFromGestures && m_frame->view()) {
+        HitTestRequest request(HitTestRequest::Active);

[bokan, 2016/08/26 16:55:14]

We're doing another hit test here right? Can we not reuse the result from the
GestureEvent?

[mustaq, 2016/08/26 17:19:03]

This is because the mousemove dispatched above can change the DOM.

[bokan, 2016/08/26 17:31:54]

Ah, got it.

+        LayoutPoint documentPoint = m_frame->view()->rootFrameToContents(targetedEvent.event().position());
+        MouseEventWithHitTestResults mev = m_frame->document()->prepareMouseEvent(request, documentPoint, mouseEvent);
+
+        WebInputEventResult eventResult = m_frame->eventHandler().dispatchMouseEvent(
+            EventTypeNames::mousedown, mev.innerNode(), /* clickCount */ 0, mouseEvent);
+
+        if (eventResult == WebInputEventResult::NotHandled)
+            eventResult = m_frame->eventHandler().handleMouseFocus(mev.hitTestResult(), InputDeviceCapabilities::firesTouchEventsSourceCapabilities());
+
+        if (eventResult == WebInputEventResult::NotHandled)
+            m_frame->eventHandler().handleMousePressEvent(mev);

[Navid Zolghadr, 2016/08/29 15:19:50]

It seems we have changed the flow quite a bit here. For example previously that
we were calling handleMousePressEvent(PlatformMouseEvent) at first we were doing
some bookkeeping of m_mousePressNode or selection related stuff here
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/input/Eve...

But now we are not doing any of that. Wouldn't that make any problem? Here is
one scenario which might have changed after this CL.

m_mousePressNode is used as the scrolling node when it is set and no other
element has the focus in scrollManager from the call here. So if we have no
scrollable element on a page except a small div somewhere. Now if you tap the
div. Then try to scroll with mouse wheel or keyboard. Before this change I
believe that div would scroll and after this change nothing would scroll I
believe.

     }
 
     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()