[webkit-reviews] review denied: [Bug 101857] Updating mouse cursor on style changes without emitting fake mousemove event : [Attachment 190475] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Aivo Paas
<aivopaas at gmail.com>'s request for review:
Bug 101857: Updating mouse cursor on style changes without emitting fake
mousemove event
https://bugs.webkit.org/show_bug.cgi?id=101857

Attachment 190475: Patch
https://bugs.webkit.org/attachment.cgi?id=190475&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=190475&action=review


> Source/WebCore/page/EventHandler.cpp:149
> +const double cursorUpdateInterval = 0.02;

Calling this "internal" makes it sounds like the timer repeats. It would be
better as "delay".

> Source/WebCore/page/EventHandler.cpp:1254
> +    HitTestRequest request(HitTestRequest::ReadOnly);
> +    HitTestResult result(view->windowToContents(m_lastKnownMousePosition));
> +    m_frame->document()->renderView()->hitTest(request, result);

I think you should call 
m_frame->document()->updateLayoutIgnorePendingStylesheets(); before this code.
Layout can be delayed on timers, and you need to ensure that layout is up to
date before hit testing.

Alternatively (perhaps preferably), you could just bail if
FrameView::needsLayout() returns true, butI wonder if that would result in
cases where the cursor update would be postponed forever.