Bug 185425 - REGRESSION(r230743): Mousemove events are not coalesced properly, mousemove/drag is very laggy
Summary: REGRESSION(r230743): Mousemove events are not coalesced properly, mousemove/d...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL: https://bl.ocks.org/mbostock/4343214
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-08 09:00 PDT by BJ Burg
Modified: 2019-02-06 09:18 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.73 KB, patch)
2018-05-08 09:16 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
For EWS (2.74 KB, patch)
2018-05-08 13:26 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
For EWS (2.68 KB, patch)
2018-05-08 14:06 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2018-05-08 09:00:01 PDT
Oops.
Comment 1 BJ Burg 2018-05-08 09:04:53 PDT
<rdar://problem/39323336>
Comment 2 BJ Burg 2018-05-08 09:16:39 PDT
Created attachment 339824 [details]
Patch
Comment 3 Simon Fraser (smfr) 2018-05-08 10:43:58 PDT
Comment on attachment 339824 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339824&action=review

> Source/WebKit/UIProcess/WebPageProxy.cpp:1929
> +    bool lastQueuedEventWasAMouseMove = m_mouseEventQueue.size() > 1 && m_mouseEventQueue.last().type() == WebEvent::MouseMove;

Why not size() > 0 ?
I would call it lastQueuedEventWasMouseMove or do away with the variable entirely.
Comment 4 BJ Burg 2018-05-08 12:22:29 PDT
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 339824 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339824&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:1929
> > +    bool lastQueuedEventWasAMouseMove = m_mouseEventQueue.size() > 1 && m_mouseEventQueue.last().type() == WebEvent::MouseMove;
> 
> Why not size() > 0 ?

Events are dequeued when WebProcess is done handling them.

If size 0, then nothing is being processed in WebProcess.
If size 1, we sent the event and are waiting on the reply.
If size 2, then the first event is being processed and the second event is queued and will be sent to WebProcess when the previous event is retired.

So if there is only one event and it's a mousemove, we can't update it because it's already been sent.

> I would call it lastQueuedEventWasMouseMove or do away with the variable
> entirely.
Comment 5 BJ Burg 2018-05-08 13:26:55 PDT
Created attachment 339863 [details]
For EWS
Comment 6 BJ Burg 2018-05-08 14:06:54 PDT
Created attachment 339874 [details]
For EWS
Comment 7 BJ Burg 2018-05-08 14:19:40 PDT
Committed r231511: <https://trac.webkit.org/changeset/231511>
Comment 8 Lucas Forschler 2019-02-06 09:18:49 PST
Mass move bugs into the DOM component.