Bug 212120

Summary: Unparented web views should not do rendering updates (don't create DisplayLinks until we have a non-zero displayID)
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: berto, cdumez, cgarcia, ews-watchlist, gustavo, pvollan, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Simon Fraser (smfr)
Reported 2020-05-19 20:31:57 PDT
We shouldn't be trying to create display links for displayID of 0, but we do. All the time.
Attachments
Patch (24.58 KB, patch)
2020-05-19 22:29 PDT, Simon Fraser (smfr)
no flags
Patch (24.63 KB, patch)
2020-05-19 22:45 PDT, Simon Fraser (smfr)
no flags
Simon Fraser (smfr)
Comment 1 2020-05-19 20:32:15 PDT
diff --git a/Source/WebKit/UIProcess/mac/DisplayLink.cpp b/Source/WebKit/UIProcess/mac/DisplayLink.cpp index 9f0c10181957ca4dec980191a4647ad4123c5d08..169c4ed6f8ee5d38313f46eb7e08026d884b59d7 100644 --- a/Source/WebKit/UIProcess/mac/DisplayLink.cpp +++ b/Source/WebKit/UIProcess/mac/DisplayLink.cpp @@ -36,6 +36,7 @@ namespace WebKit { DisplayLink::DisplayLink(WebCore::PlatformDisplayID displayID) : m_displayID(displayID) { + ASSERT(displayID); ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer)); CVReturn error = CVDisplayLinkCreateWithCGDisplay(displayID, &m_displayLink); if (error) {
Simon Fraser (smfr)
Comment 2 2020-05-19 20:35:28 PDT
We try to make a DisplayRefreshMonitor before we've sent the displayID to the web process: * frame #2: 0x000000011872d111 WebKit`WebKit::DisplayRefreshMonitorMac::DisplayRefreshMonitorMac(this=0x0000000113bee268, displayID=0) at DrawingAreaMac.cpp:68:5 frame #3: 0x000000011872d1bb WebKit`WebKit::DisplayRefreshMonitorMac::DisplayRefreshMonitorMac(this=0x0000000113bee268, displayID=0) at DrawingAreaMac.cpp:67:1 frame #4: 0x000000011872d782 WebKit`WebKit::DisplayRefreshMonitorMac::create(displayID=0) at DrawingAreaMac.cpp:45:30 frame #5: 0x000000011872d729 WebKit`WebKit::DrawingArea::createDisplayRefreshMonitor(this=0x0000000113bed880, displayID=0) at DrawingAreaMac.cpp:108:12 frame #6: 0x0000000118637890 WebKit`WebKit::WebChromeClient::createDisplayRefreshMonitor(this=0x0000000113bff360, displayID=0) const at WebChromeClient.cpp:861:29 frame #7: 0x00000001269998a9 WebCore`WebCore::RenderingUpdateScheduler::createDisplayRefreshMonitor(this=0x0000000113b9b5e8, displayID=0) const at RenderingUpdateScheduler.cpp:123:49 frame #8: 0x0000000126ba6887 WebCore`WebCore::DisplayRefreshMonitor::create(client=0x0000000113b9b5e8) at DisplayRefreshMonitor.cpp:65:19 frame #9: 0x0000000126ba72d4 WebCore`WebCore::DisplayRefreshMonitorManager::monitorForClient(this=0x0000000129dc86c8, client=0x0000000113b9b5e8) at DisplayRefreshMonitorManager.cpp:54:20 frame #10: 0x0000000126ba778d WebCore`WebCore::DisplayRefreshMonitorManager::scheduleAnimation(this=0x0000000129dc86c8, client=0x0000000113b9b5e8) at DisplayRefreshMonitorManager.cpp:89:25 frame #11: 0x000000012699937a WebCore`WebCore::RenderingUpdateScheduler::scheduleAnimation(this=0x0000000113b9b5e8, preferredFramesPerSecond=60) at RenderingUpdateScheduler.cpp:61:58 frame #12: 0x0000000126999676 WebCore`WebCore::RenderingUpdateScheduler::scheduleTimedRenderingUpdate(this=0x0000000113b9b5e8) at RenderingUpdateScheduler.cpp:95:23 frame #13: 0x0000000126999a15 WebCore`WebCore::RenderingUpdateScheduler::scheduleRenderingUpdate(this=0x0000000113b9b5e8) at RenderingUpdateScheduler.cpp:152:9 frame #14: 0x00000001268f3e7d WebCore`WebCore::Page::scheduleRenderingUpdate(this=0x00000001158fa000) at Page.cpp:1337:32 frame #15: 0x00000001271a866e WebCore`WebCore::RenderLayerCompositor::scheduleRenderingUpdate(this=0x0000000113b73130) at RenderLayerCompositor.cpp:505:12 frame #16: 0x00000001271aa625 WebCore`WebCore::RenderLayerCompositor::ensureRootLayer(this=0x0000000113b73130) at RenderLayerCompositor.cpp:4005:17 frame #17: 0x00000001271a9ea7 WebCore`WebCore::RenderLayerCompositor::enableCompositingMode(this=0x0000000113b73130, enable=true) at RenderLayerCompositor.cpp:375:13 frame #18: 0x00000001271781fd WebCore`WebCore::RenderLayerCompositor::updateCompositingLayers(this=0x0000000113b73130, updateType=AfterStyleChange, updateRoot=0x0000000000000000) at RenderLayerCompositor.cpp:742:9 frame #19: 0x00000001271ab3ab WebCore`WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout(this=0x0000000113b73130) at RenderLayerCompositor.cpp:480:12 frame #20: 0x0000000126891697 WebCore`WebCore::FrameView::updateCompositingLayersAfterStyleChange(this=0x00000001157f4010) at FrameView.cpp:821:39 frame #21: 0x0000000125bdf682 WebCore`WebCore::Document::resolveStyle(this={ origin = Unique, url = , inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, type=Rebuild) at Document.cpp:2015:46 frame #22: 0x0000000125be1be8 WebCore`WebCore::Document::createRenderTree(this={ origin = Unique, url = , inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:2370:5 frame #23: 0x0000000125be1d72 WebCore`WebCore::Document::didBecomeCurrentDocumentInFrame(this={ origin = Unique, url = , inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:2381:9 frame #24: 0x0000000126885cc2 WebCore`WebCore::Frame::setDocument(this={ origin = Unique, url = , isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, newDocument=0x00007ffee0f4b340) at Frame.cpp:289:22 frame #25: 0x000000012663dedb WebCore`WebCore::DocumentWriter::begin(this=0x00000001159f6090, urlReference={ }, dispatch=false, ownerDocument={ origin = , url = , inMainFrame = Detached, backForwardCacheState = None }) at DocumentWriter.cpp:165:14 frame #26: 0x000000012663778e WebCore`WebCore::DocumentLoader::commitData(this=0x00000001159f6000, bytes=0x0000000000000000, length=0) at DocumentLoader.cpp:1080:34 frame #27: 0x00000001266371c3 WebCore`WebCore::DocumentLoader::finishedLoading(this=0x00000001159f6000) at DocumentLoader.cpp:445:13 frame #28: 0x0000000126642834 WebCore`WebCore::DocumentLoader::maybeLoadEmpty(this=0x00000001159f6000) at DocumentLoader.cpp:1799:5 frame #29: 0x00000001266429c5 WebCore`WebCore::DocumentLoader::startLoadingMainResource(this=0x00000001159f6000) at DocumentLoader.cpp:1813:9 frame #30: 0x0000000126694dc6 WebCore`WebCore::FrameLoader::init(this=0x0000000113be1340) at FrameLoader.cpp:344:34 frame #31: 0x0000000126884ce6 WebCore`WebCore::Frame::init(this={ origin = Unique, url = , isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }) at Frame.cpp:184:15 frame #32: 0x000000011870b7fc WebKit`WebKit::WebFrame::initWithCoreMainFrame(this=0x0000600002520688, page=0x00007fa879008208, coreFrame={ origin = Unique, url = , isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }) at WebFrame.cpp:111:18 frame #33: 0x00000001188b225e WebKit`WebKit::WebPage::WebPage(this=0x00007fa879008208, pageID=(m_identifier = 6), parameters=0x00007ffee0f4c9b8) at WebPage.cpp:592:18 frame #34: 0x00000001188b0a75 WebKit`WebKit::WebPage::WebPage(this=0x00007fa879008208, pageID=(m_identifier = 6), parameters=0x00007ffee0f4c9b8) at WebPage.cpp:479:1 frame #35: 0x00000001188b0981 WebKit`WebKit::WebPage::create(pageID=(m_identifier = 6), parameters=0x00007ffee0f4c9b8) at WebPage.cpp:388:39 frame #36: 0x00000001183ff7e4 WebKit`WebKit::WebProcess::createWebPage(this=0x0000000113bf3000, pageID=(m_identifier = 6), parameters=0x00007ffee0f4c9b8) at WebProcess.cpp:696:34 frame #37: 0x0000000118b5539c WebKit`void IPC::callMemberFunctionImpl<WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>, 0ul, 1ul>(object=0x0000000113bf3000, function=00 f7 3f 18 01 00 00 00 00 00 00 00 00 00 00 00, args=size=2, (null)=std::__1::index_sequence<0UL, 1UL> @ 0x00007ffee0f4c8e8)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) at HandleMessage.h:41:5 frame #38: 0x0000000118b53ec0 WebKit`void IPC::callMemberFunction<WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(args=size=2, object=0x0000000113bf3000, function=00 f7 3f 18 01 00 00 00 00 00 00 00 00 00 00 00)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)) at HandleMessage.h:47:5 frame #39: 0x0000000118b4e1de WebKit`void IPC::handleMessage<Messages::WebProcess::CreateWebPage, WebKit::WebProcess, void (WebKit::WebProcess::*)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)>(decoder=0x0000000113bde140, object=0x0000000113bf3000, function=00 f7 3f 18 01 00 00 00 00 00 00 00 00 00 00 00)(WTF::ObjectIdentifier<WebCore::PageIdentifierType>, WebKit::WebPageCreationParameters&&)) at HandleMessage.h:114:5 frame #40: 0x0000000118b4c23c WebKit`WebKit::WebProcess::didReceiveWebProcessMessage(this=0x0000000113bf3000, connection=0x0000000113be1000, decoder=0x0000000113bde140) at WebProcessMessageReceiver.cpp:294:9 frame #41: 0x00000001184001c6 WebKit`WebKit::WebProcess::didReceiveMessage(this=0x0000000113bf3000, connection=0x0000000113be1000, decoder=0x0000000113bde140) at WebProcess.cpp:761:9 frame #42: 0x000000011708386f WebKit`IPC::Connection::dispatchMessage(this=0x0000000113be1000, decoder=0x0000000113bde140) at Connection.cpp:1001:14 frame #43: 0x00000001170841a2 WebKit`IPC::Connection::dispatchMessage(this=0x0000000113be1000, message=unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> > @ 0x00007ffee0f4d3c0) at Connection.cpp:1070:9 frame #44: 0x0000000117084800 WebKit`IPC::Connection::dispatchOneIncomingMessage(this=0x0000000113be1000) at Connection.cpp:1139:5 frame #45: 0x00000001170a301e WebKit`IPC::Connection::enqueueIncomingMessage(this=0x0000000113bdd058)::$_7::operator()() at Connection.cpp:978:28 frame #46: 0x00000001170a2f2e WebKit`WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7, void>::call(this=0x0000000113bdd050) at Function.h:52:39 frame #47: 0x000000013afad052 JavaScriptCore`WTF::Function<void ()>::operator(this=0x00007ffee0f4d488)() const at Function.h:84:35 frame #48: 0x000000013b01c8e8 JavaScriptCore`WTF::RunLoop::performWork(this=0x0000000113bf5000) at RunLoop.cpp:119:9 frame #49: 0x000000013b01d2b1 JavaScriptCore`WTF::RunLoop::performWork(context=0x0000000113bf5000) at RunLoopCF.cpp:38:37 frame #50: 0x00007fff33c11f12 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
Simon Fraser (smfr)
Comment 3 2020-05-19 22:29:01 PDT
EWS Watchlist
Comment 4 2020-05-19 22:29:58 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Simon Fraser (smfr)
Comment 5 2020-05-19 22:45:04 PDT
Simon Fraser (smfr)
Comment 6 2020-05-19 23:17:39 PDT
Comment on attachment 399813 [details] Patch Need to figure out what to do for WK1 views without a window. Just don't schedule display refreshes? Will that break offscreen web views?
Simon Fraser (smfr)
Comment 7 2020-05-20 07:48:13 PDT
I think we have 3 options here for offscreen windows: 1. don't schedule display refreshes when there is no dispayID 2. fall back to the displayID of the main screen 3. use a timer 1) has the best intent (don't do display-related work when not on-screen) but I worry that it would break offscreen web views, which would stop firing rAF.
Radar WebKit Bug Importer
Comment 8 2020-05-20 07:48:42 PDT
Simon Fraser (smfr)
Comment 9 2020-05-20 10:03:00 PDT
Also CVDisplayLinkCreateWithCGDisplay(0) happens to do something (probably uses the primary display).
Simon Fraser (smfr)
Comment 10 2021-03-30 16:42:07 PDT
When fixing this maybe we can clean up DisplayUpdate::relevantForUpdateFrequency().
Note You need to log in before you can comment on or make changes to this bug.