[webkit-reviews] review granted: [Bug 223648] Add DisplayRefreshMonitorFactory : [Attachment 424045] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 23 12:25:06 PDT 2021
Chris Dumez <cdumez at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 223648: Add DisplayRefreshMonitorFactory
https://bugs.webkit.org/show_bug.cgi?id=223648
Attachment 424045: Patch
https://bugs.webkit.org/attachment.cgi?id=424045&action=review
--- Comment #2 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 424045
--> https://bugs.webkit.org/attachment.cgi?id=424045
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=424045&action=review
r=me
> Source/WebCore/loader/EmptyClients.cpp:147
> +class EmptyDisplayRefreshMonitorFactory : public
DisplayRefreshMonitorFactory {
Can the class be marked as final?
> Source/WebCore/page/RenderingUpdateScheduler.h:37
> +class RenderingUpdateScheduler : public DisplayRefreshMonitorClient {
Can this class be marked as final?
> Source/WebCore/platform/graphics/GraphicsLayerUpdater.h:43
> +class GraphicsLayerUpdater : public DisplayRefreshMonitorClient {
Can this class be marked as final?
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.cpp:34
> +
I don't think we usually have this empty line in between
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.cpp:44
> +{
ASSERT(isMainRunLoop()); ? since I assume DisplayLinkObserverID::generate() is
not thread safe.
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.cpp:49
> +
WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessPro
xy::StopDisplayLink(m_observerID, displayID()), 0);
Is there a chance requestRefreshCallback() has not been called yet? If so, is
there a way to early return to avoid the IPC is that case?
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.cpp:61
> + this->m_firstCallbackInCurrentRunloop = true;
this-> should not be needed.
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.h:36
> +class DisplayRefreshMonitorMac : public WebCore::DisplayRefreshMonitor {
Can this class be marked as final?
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.h:45
> + void displayLinkFired() override;
Can this be final?
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.h:46
> + bool requestRefreshCallback() override;
ditto
> Source/WebKit/WebProcess/WebPage/mac/DisplayRefreshMonitorMac.h:51
> + bool hasRequestedRefreshCallback() const override { return
m_hasSentMessage; }
ditto.
More information about the webkit-reviews
mailing list