[webkit-reviews] review granted: [Bug 135082] [iOS][WK2] Improve event throttling for Scroll Events : [Attachment 235168] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 135082: [iOS][WK2] Improve event throttling for Scroll Events
https://bugs.webkit.org/show_bug.cgi?id=135082

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

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


> Source/WebCore/page/ChromeClient.h:239
> +    virtual double eventThrottlingDelay() { return 0; };

Should this use std::chrono something?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2881
> +	   m_estimatedMainThreadLatency = m_estimatedMainThreadLatency * 0.80 +
elapsed * 0.20;

I'm not sure the "main thread" here is interesting.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4472
> +    m_estimatedMainThreadLatency = 1 / 60.;

60.0?

> Source/WebKit2/WebProcess/WebPage/WebPage.h:514
> +    double eventThrottlingDelay() const;

std::chrono would tell me what the units are.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:754
> +    void updateVisibleContentRects(const VisibleContentRectUpdateInfo&,
double);

Please give the double a name.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1680
> +    if (m_isInStableState || m_estimatedMainThreadLatency <= 1. / 60)

Why not 1/60.0 like you used before?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1683
> +    return std::min<double>(m_estimatedMainThreadLatency * 2, 1);

Explain the * 2 in a comment. What units are we in here?