[webkit-reviews] review requested: [Bug 225834] Allow wheel events to be coalesced during scroll deceleration : [Attachment 428692] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has asked  for review:
Bug 225834: Allow wheel events to be coalesced during scroll deceleration
https://bugs.webkit.org/show_bug.cgi?id=225834

Attachment 428692: Patch

https://bugs.webkit.org/attachment.cgi?id=428692&action=review




--- Comment #4 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 428692
  --> https://bugs.webkit.org/attachment.cgi?id=428692
Patch

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

> Source/WebKit/Shared/WebWheelEventCoalescer.cpp:90
> +bool WebWheelEventCoalescer::isInMomentumPhase(const WebWheelEvent& event)

This feels like an odd name for what this checks. It should either have a more
descriptive name or at least have a comment explaining things.

> Source/WebKit/Shared/WebWheelEventCoalescer.cpp:151
> +    if (isInMomentumPhase(event) && shouldCoalesceEventsDuringDeceleration()
&& lastEventInterval && lastEventInterval) {

"lastEventInterval && lastEventInterval" seems duplicated.

> Source/WebKit/Shared/WebWheelEventCoalescer.cpp:152
> +	   static constexpr double
momentumVelocityEventFrequencyReductionThreashold = 320.0; // Points per
second.

No need for the static here.

> Source/WebKit/Shared/WebWheelEventCoalescer.cpp:155
> +	   static constexpr auto maxCoalescingInterval =
WebCore::FullSpeedAnimationInterval;

No need for the static here.

> Source/WebKit/UIProcess/WebPageProxy.cpp:2829
> +    auto framesPerSecond =
m_process->processPool().nominalFramesPerSecondForDisplay(*m_displayID);

Not new, but it is still bizarre this on the process pool.