Changeset 234384 in webkit

Timestamp:

Jul 30, 2018 2:47:31 PM (6 years ago)

Author:

Wenson Hsieh

Message:

REGRESSION (r230817): Terrible performance when selecting text on Stash code review
​https://bugs.webkit.org/show_bug.cgi?id=188144
<rdar://problem/42642489>

Reviewed by Darin Adler.

After r230817, mouse events were serially dispatched to the web process and handled before the subsequent mouse
event. However, this resulted in rapid-fire mouse move events filling up the mouse event queue in the case where
mouse move events were being handled by the web process at a slower rate than the UI process was enqueueing
them. To mitigate this, r231511 introduced a mechanism for replacing the most recently enqueued mouse move event
with an incoming mouse move event.

However, when a user with a force-click-enabled trackpad performs a mouse drag, a rapid stream of
"mouseforcechanged" events is interleaved alongside the stream of "mousemove" events. This renders r231511
ineffective, since the most recently queued event is often a "mouseforcechanged" event instead of a "mousemove".
On the stash code review page, this can result in hundreds of mouse events being backed up in the mouse event
queue, causing perceived slowness when selecting text.

To fix this, we extend the mechanism introduced in r231511, such that it is capable of replacing both
"mouseforcechanged" and "mousemove" events in the queue. Rather than consider only the most recently queued
item, we instead find the most recently queued event that matches the type of the incoming event, remove it from
the queue, and then append the incoming event to the end of the queue. To avoid the risk of removing the only
"mousemove" or "mouseforcechanged" event in the middle of a mouse down and mouse up, we also bail when searching
backwards for an event to replace if we come across any event that is neither of these types.

This effectively throttles the rate at which mouseforcechanged or mousemove events are dispatched when a user
with force-click-enabled hardware clicks and drags the mouse across the page.

• UIProcess/WebPageProxy.cpp:

(WebKit::removeOldRedundantEvent):
(WebKit::WebPageProxy::handleMouseEvent):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-07-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r230817): Terrible performance when selecting text on Stash code review
+        https://bugs.webkit.org/show_bug.cgi?id=188144
+        <rdar://problem/42642489>
+
+        Reviewed by Darin Adler.
+
+        After r230817, mouse events were serially dispatched to the web process and handled before the subsequent mouse
+        event. However, this resulted in rapid-fire mouse move events filling up the mouse event queue in the case where
+        mouse move events were being handled by the web process at a slower rate than the UI process was enqueueing
+        them. To mitigate this, r231511 introduced a mechanism for replacing the most recently enqueued mouse move event
+        with an incoming mouse move event.
+
+        However, when a user with a force-click-enabled trackpad performs a mouse drag, a rapid stream of
+        "mouseforcechanged" events is interleaved alongside the stream of "mousemove" events. This renders r231511
+        ineffective, since the most recently queued event is often a "mouseforcechanged" event instead of a "mousemove".
+        On the stash code review page, this can result in hundreds of mouse events being backed up in the mouse event
+        queue, causing perceived slowness when selecting text.
+
+        To fix this, we extend the mechanism introduced in r231511, such that it is capable of replacing both
+        "mouseforcechanged" and "mousemove" events in the queue. Rather than consider only the most recently queued
+        item, we instead find the most recently queued event that matches the type of the incoming event, remove it from
+        the queue, and then append the incoming event to the end of the queue. To avoid the risk of removing the only
+        "mousemove" or "mouseforcechanged" event in the middle of a mouse down and mouse up, we also bail when searching
+        backwards for an event to replace if we come across any event that is neither of these types.
+
+        This effectively throttles the rate at which mouseforcechanged or mousemove events are dispatched when a user
+        with force-click-enabled hardware clicks and drags the mouse across the page.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::removeOldRedundantEvent):
+        (WebKit::WebPageProxy::handleMouseEvent):
+
 2018-07-30  Devin Rousso  <webkit@devinrousso.com>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1946 +1946 @@
 #endif // ENABLE(DRAG_SUPPORT)
 
+static bool removeOldRedundantEvent(Deque<NativeWebMouseEvent>& queue, WebEvent::Type incomingEventType)
+{
+    if (incomingEventType != WebEvent::MouseMove && incomingEventType != WebEvent::MouseForceChanged)
+        return false;
+
+    auto it = queue.rbegin();
+    auto end = queue.rend();
+
+    // Must not remove the first event in the deque, since it is already being dispatched.
+    if (it != end)
+        --end;
+
+    for (; it != end; ++it) {
+        auto type = it->type();
+        if (type == incomingEventType) {
+            queue.remove(--it.base());
+            return true;
+        }
+        if (type != WebEvent::MouseMove && type != WebEvent::MouseForceChanged)
+            break;
+    }
+    return false;
+}
+
 void WebPageProxy::handleMouseEvent(const NativeWebMouseEvent& event)
 {
@@ -1951 +1975 @@
         return;
 
-    // If we receive multiple mousemove events and the most recent mousemove event has not been
-    // sent to WebProcess for processing, replace the pending mousemove event with a new one.
-    if (event.type() == WebEvent::MouseMove && m_mouseEventQueue.size() > 1 && m_mouseEventQueue.last().type() == WebEvent::MouseMove) {
-        LOG(MouseHandling, "UIProcess: updated pending mousemove event (queue size %zu)", m_mouseEventQueue.size());
-        m_mouseEventQueue.removeLast();
-        m_mouseEventQueue.append(event);
-    } else {
-        LOG(MouseHandling, "UIProcess: enqueued mouse event %s (queue size %zu)", webMouseEventTypeString(event.type()), m_mouseEventQueue.size());
-        m_mouseEventQueue.append(event);
-    }
+    // If we receive multiple mousemove or mouseforcechanged events and the most recent mousemove or mouseforcechanged event
+    // (respectively) has not yet been sent to WebProcess for processing, remove the pending mouse event and insert the new
+    // event in the queue.
+    bool didRemoveEvent = removeOldRedundantEvent(m_mouseEventQueue, event.type());
+    m_mouseEventQueue.append(event);
+
+#if LOG_DISABLED
+    UNUSED_PARAM(didRemoveEvent);
+#else
+    LOG(MouseHandling, "UIProcess: %s mouse event %s (queue size %zu)", didRemoveEvent ? "replaced" : "enqueued", webMouseEventTypeString(event.type()), m_mouseEventQueue.size());
+#endif
+
     if (m_mouseEventQueue.size() == 1) // Otherwise, called from DidReceiveEvent message handler.
         processNextQueuedMouseEvent();