Bug 223648 – Add DisplayRefreshMonitorFactory

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Simon Fraser (smfr)]

Gets createDisplayRefreshMonitor() off of DisplayRefreshMonitorClient

[Simon Fraser (smfr)]

Created attachment 424045 [details]
Patch

[Chris Dumez]

Comment on attachment 424045 [details]
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::WebProcessProxy::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.

[Simon Fraser (smfr)]

Created attachment 424059 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 424064 [details]
Patch

[EWS]

Committed r274929: <https://commits.webkit.org/r274929>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424064 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75766987>