Attachment 346018 Details for Bug 188144 – Patch

[patch] Patch

bug-188144-20180729012042.patch (text/plain), 5.50 KB, created by Wenson Hsieh on 2018-07-29 01:20:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-07-29 01:20:43 PDT

Size: 5.50 KB

patch

obsolete

>Subversion Revision: 234280
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e4b95a9787aafa447ed4ff94a5d75d517e480118..fda38ba13fa5e11df9ea048615fd75f94407a6b5 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-07-29  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        REGRESSION (r230817): Terrible performance and flashing Touch Bar when selecting text on Stash code review
>+        https://bugs.webkit.org/show_bug.cgi?id=188144
>+        <rdar://problem/42642489>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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 "mousemoved" 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 "mousemoved" 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
>+        "mousemoved" 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 mousemoved events are dispatched when a user
>+        with force-click-enabled hardware clicks and drags the mouse across the page.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::appendOrReplaceMouseEvent):
>+        (WebKit::WebPageProxy::handleMouseEvent):
>+
> 2018-07-26  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         String(View) should have a splitAllowingEmptyEntries function instead of a flag parameter
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 18d8b8449943bc870a15d829e450d2a63c68cead..c31154614aa49ee05e071c92a6bae91c781b7872 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -1944,21 +1944,41 @@ void WebPageProxy::setDragCaretRect(const IntRect& dragCaretRect)
> 
> #endif // ENABLE(DRAG_SUPPORT)
> 
>+static void appendOrReplaceMouseEvent(Deque<NativeWebMouseEvent>& queue, const NativeWebMouseEvent& incomingEvent)
>+{
>+    // 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, replace the pending mouse event with the new event.
>+    // Otherwise, simply append the event to the queue.
>+    Vector<NativeWebMouseEvent, 1> eventsAfterReplacedEvent;
>+    bool performedReplacement = false;
>+    while (queue.size() > 1) {
>+        auto lastEventType = queue.last().type();
>+        if (lastEventType != WebEvent::MouseMove && lastEventType != WebEvent::MouseForceChanged)
>+            break;
>+
>+        auto lastEvent = queue.takeLast();
>+        if (lastEvent.type() == incomingEvent.type()) {
>+            performedReplacement = true;
>+            break;
>+        }
>+
>+        eventsAfterReplacedEvent.append(lastEvent);
>+    }
>+
>+    LOG(MouseHandling, "UIProcess: %s mouse event %s (queue size %zu)", performedReplacement ? "replacing" : "enqueueing", webMouseEventTypeString(incomingEvent.type()), queue.size());
>+
>+    for (auto event = eventsAfterReplacedEvent.rbegin(); event != eventsAfterReplacedEvent.rend(); ++event)
>+        queue.append(*event);
>+
>+    queue.append(incomingEvent);
>+}
>+
> 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);
>-    }
>+    appendOrReplaceMouseEvent(m_mouseEventQueue, event);
>     if (m_mouseEventQueue.size() == 1) // Otherwise, called from DidReceiveEvent message handler.
>         processNextQueuedMouseEvent();
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 188144: 346018 | 346024 | 346045 | 346047 | 346088 | 346091