Fixed & refactored mouse event firing at gesture context menu

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/input/GestureManager.h"
 
 #include "core/dom/Document.h"
 #include "core/editing/SelectionController.h"
 #include "core/events/GestureEvent.h"
 #include "core/frame/FrameHost.h"
@@ skipping 169 matching lines @@
         static_cast<PlatformEvent::Modifiers>(modifiers | PlatformEvent::LeftButtonDown),
         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::HandledSuppressed;
     if (!m_suppressMouseEventsFromGestures) {
         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));
     }
 
     if (currentHitTest.innerNode()) {
         DCHECK(gestureEvent.type() == PlatformEvent::GestureTap);
         HitTestResult result = currentHitTest;
         result.setToShadowHostIfInUserAgentShadowRoot();
         m_frame->chromeClient().onMouseDown(result.innerNode());
     }
@@ skipping 95 matching lines @@
             WebPointerProperties::Button::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;
+    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()
 {
     m_lastShowPressTimestamp = WTF::monotonicallyIncreasingTime();
 
     FrameView* view = m_frame->view();
     if (!view)
         return WebInputEventResult::NotHandled;
     if (ScrollAnimatorBase* scrollAnimator = view->existingScrollAnimator())
@@ skipping 16 matching lines @@
 
     return &m_frame->page()->frameHost();
 }
 
 double GestureManager::getLastShowPressTimestamp() const
 {
     return m_lastShowPressTimestamp;
 }
 
 } // namespace blink