Bug 212120 - Unparented web views should not do rendering updates (don't create DisplayLinks until we have a non-zero displayID)
Summary: Unparented web views should not do rendering updates (don't create DisplayLin...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-19 20:31 PDT by Simon Fraser (smfr)
Modified: 2021-03-30 16:42 PDT (History)
9 users (show)

See Also:


Attachments
Patch (24.58 KB, patch)
2020-05-19 22:29 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (24.63 KB, patch)
2020-05-19 22:45 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 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.
Comment 1 Simon Fraser (smfr) 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) {
Comment 2 Simon Fraser (smfr) 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
Comment 3 Simon Fraser (smfr) 2020-05-19 22:29:01 PDT
Created attachment 399811 [details]
Patch
Comment 4 EWS Watchlist 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
Comment 5 Simon Fraser (smfr) 2020-05-19 22:45:04 PDT
Created attachment 399813 [details]
Patch
Comment 6 Simon Fraser (smfr) 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?
Comment 7 Simon Fraser (smfr) 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.
Comment 8 Radar WebKit Bug Importer 2020-05-20 07:48:42 PDT
<rdar://problem/63446954>
Comment 9 Simon Fraser (smfr) 2020-05-20 10:03:00 PDT
Also CVDisplayLinkCreateWithCGDisplay(0) happens to do something (probably uses the primary display).
Comment 10 Simon Fraser (smfr) 2021-03-30 16:42:07 PDT
When fixing this maybe we can clean up DisplayUpdate::relevantForUpdateFrequency().