Reset drag state variables on mouse up

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 39b426486a0639eba217e8c0ba1b7182be1cd34e..fba7033769e61402e09d12b0623f42c6df1b260d 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -432,12 +432,7 @@ WebInputEventResult EventHandler::handleMouseReleaseEvent(const MouseEventWithHi
     if (controller && controller->autoscrollInProgress())
         m_scrollManager.stopAutoscroll();
 
-    // Used to prevent mouseMoveEvent from initiating a drag before
-    // the mouse is pressed again.
-    m_mousePressed = false;
-    m_capturesDragging = false;
-    m_mouseDownMayStartDrag = false;
-    m_mouseDownMayStartAutoscroll = false;
+    clearMouseDragState();
 
     return selectionController().handleMouseReleaseEvent(event, m_dragStartPos) ? WebInputEventResult::HandledSystem : WebInputEventResult::NotHandled;
 }
@@ -1206,6 +1201,8 @@ WebInputEventResult EventHandler::handleMouseReleaseEvent(const PlatformMouseEve
 
     if (eventResult == WebInputEventResult::NotHandled)
         eventResult = handleMouseReleaseEvent(mev);
+    else
+        clearMouseDragState();

[dtapuska, 2016/06/09 21:18:58]

what about the code path for gesture tap that has a similar flow?

Also there are a few places that assign m_mousePressed = false; do we need to
every clear the drag state in those?

I'm wondering if we took the code path out of handleMouseReleaseEvent and always
did the clearMouseDragState whether that would be more logical?

Or should we put an AutoReset like object on the stack of the
handleMouseReleaseEvent(...) so clearMouseDragState is always cleared at the end
of its return?

[mustaq, 2016/06/10 15:44:04]

handleMousePressEvent(MouseEventWithHitTestResults) is called from this method
and handleGestureTap. Clearing the state unconditionally at both these callers
should work (so we won't need it in
handleMousePressEvent(MouseEventWithHitTestResults).

[Navid Zolghadr, 2016/06/10 19:19:55]

I added the call as suggested to the tap flow as well.

To be honest with you I don't know about all those state variable. I need to
look deeper which was in my plan when I start refactoring the mouse code out of
EventHandler. So I don't have a good answer right now what each of these
variables mean.

Is calling the clean right from the function okay or do you like those object
cleanup methods?

[mustaq, 2016/06/10 19:30:16]

I am indifferent about AutoReset here, mainly for slightly better readability
w/o it. Dave's call.

 
     invalidateClick();
 
@@ -1383,6 +1380,16 @@ WebInputEventResult EventHandler::performDragAndDrop(const PlatformMouseEvent& e
     return result;
 }
 
+void EventHandler::clearMouseDragState()
+{
+    // Used to prevent mouseMoveEvent from initiating a drag before
+    // the mouse is pressed again.
+    m_mousePressed = false;
+    m_capturesDragging = false;
+    m_mouseDownMayStartDrag = false;
+    m_mouseDownMayStartAutoscroll = false;
+}
+
 void EventHandler::clearDragState()
 {
     m_scrollManager.stopAutoscroll();