Bug 199808 – Crash under DisplayRefreshMonitorManager::displayWasUpdated()

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

[Chris Dumez]

50 WebCore: WebCore::DisplayRefreshMonitorManager::displayWasUpdated(unsigned int) <==
        50 WebKit: IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
          50 WebKit: WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14, void>::call()
            50 JavaScriptCore: WTF::RunLoop::performWork()
              50 JavaScriptCore: WTF::RunLoop::performWork(void*)
                50 CoreFoundation: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
                  50 CoreFoundation: __CFRunLoopDoSource0
                    50 CoreFoundation: __CFRunLoopDoSources0
                      50 CoreFoundation: __CFRunLoopRun
                        50 CoreFoundation: CFRunLoopRunSpecific
                          50 Foundation: -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
                            50 Foundation: -[NSRunLoop(NSRunLoop) run]
                              50 libxpc.dylib: _xpc_objc_main.cold.4
                                50 libxpc.dylib: _xpc_objc_main
                                  50 libxpc.dylib: xpc_main
                                    50 WebKit: WebKit::XPCServiceMain(int, char const**)
                                      50 libdyld.dylib: start

[Chris Dumez]

<rdar://problem/53070144>

[Chris Dumez]

Created attachment 374151 [details]
Patch

[Geoffrey Garen]

Comment on attachment 374151 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 374151 [details]
Patch

Clearing flags on attachment: 374151

Committed r247459: <https://trac.webkit.org/changeset/247459>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

Comment on attachment 374151 [details]
Patch

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

> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:134
> -    for (const auto& monitorWrapper : m_monitors) {
> -        auto& monitor = monitorWrapper.monitor;
> +    Vector<RefPtr<DisplayRefreshMonitor>> monitors = WTF::map(m_monitors, [](auto& monitorWrapper) {
> +        return monitorWrapper.monitor;
> +    });
> +    for (auto& monitor : monitors) {

Isn't PlatformDisplayID unique for every monitor? If so, why do we need to loop through all the monitors if we find one whose displayID matches the argument displayID? Can't we just break/return when we find the first one:

if (displayID == monitor->displayID()) {
    if (monitor->hasRequestedRefreshCallback())
        monitor->displayLinkFired();
    break;
}

DisplayRefreshMonitorManager::unregisterClient() has similar logic like the proposed one so it does not need to copy the monitors before looping through them.