Title: [234384] trunk/Source/WebKit
- Revision
- 234384
- Author
- wenson_hs...@apple.com
- Date
- 2018-07-30 14:47:31 -0700 (Mon, 30 Jul 2018)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (234383 => 234384)
--- trunk/Source/WebKit/ChangeLog 2018-07-30 21:27:43 UTC (rev 234383)
+++ trunk/Source/WebKit/ChangeLog 2018-07-30 21:47:31 UTC (rev 234384)
@@ -1,3 +1,37 @@
+2018-07-30 Wenson Hsieh <wenson_hs...@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 <web...@devinrousso.com>
Add missing CoreGraphics SPI
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (234383 => 234384)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-07-30 21:27:43 UTC (rev 234383)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-07-30 21:47:31 UTC (rev 234384)
@@ -1945,21 +1945,47 @@
#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)
{
if (!isValid())
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();
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
