Bug 85512 – ASSERT(!m_zOrderListsDirty) is triggering in Safari

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Julien Chaffraix]

Copied from bug 84920, smfr posted 2 stacktraces where the ASSERT is hit:

#0  0x0000000105634ed5 in WebCore::RenderLayer::negZOrderList (this=0x121d27018) at RenderLayer.h:398
#1  0x0000000105652249 in WebCore::RenderLayerCompositor::layerHas3DContent (this=0x121a1b070, layer=0x121d27018) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebCore/rendering/RenderLayerCompositor.cpp:2237
#2  0x0000000105652334 in WebCore::RenderLayerCompositor::layerHas3DContent (this=0x121a1b070, layer=0x121a02a18) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebCore/rendering/RenderLayerCompositor.cpp:2250
#3  0x0000000105652334 in WebCore::RenderLayerCompositor::layerHas3DContent (this=0x121a1b070, layer=0x121a04168) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebCore/rendering/RenderLayerCompositor.cpp:2250
#4  0x0000000105652185 in WebCore::RenderLayerCompositor::has3DContent (this=0x121a1b070) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebCore/rendering/RenderLayerCompositor.cpp:1324
#5  0x0000000104a09191 in WebCore::FrameView::isSoftwareRenderable (this=0x121a19ec0) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebCore/page/FrameView.cpp:850
#6  0x0000000102787169 in -[WebView(WebPrivate) _isSoftwareRenderable] (self=0x10fb72320, _cmd=0x7fff8c2e64c6) at /Volumes/DataSSD/Development/apple/webkit/WebKit.git/Source/WebKit/mac/WebView/WebView.mm:2469
#7  0x000000010059012b in -[WebView(SafariSnapshotGeneration) createImageForRect:inSubview:] (self=0x10fb72320, _cmd=0x7fff8c2e63de, rectToCapture={origin = {x = 0, y = 0}, size = {width = 974, height = 887}}, subview=0x110915dd0) at /Volumes/WebKit/Internal/Safari/mac/SafariWebViewSnapshotGeneration.mm:65

(per discussion, this one is unfortunately untestable)

Another one:

>  1 com.apple.WebCore              0x10b70291d WebCore::RenderLayer::negZOrderList() const + 0x5d (RenderLayer.h:398)
   2 com.apple.WebCore              0x10b712732 WebCore::RenderLayerBacking::hasVisibleNonCompositingDescendantLayers() const + 0x122 (RenderLayerBacking.cpp:928)
   3 com.apple.WebCore              0x10b712ea5 WebCore::RenderLayerBacking::paintsChildren() const + 0x55 (RenderLayerBacking.cpp:841)
   4 com.apple.WebCore              0x10b713017 WebCore::RenderLayerBacking::isSimpleContainerCompositingLayer() const + 0x67 (RenderLayerBacking.cpp:857)
   5 com.apple.WebCore              0x10b712b89 WebCore::RenderLayerBacking::containsPaintedContent() const + 0x19 (RenderLayerBacking.cpp:952)
   6 com.apple.WebCore              0x10b712ad9 WebCore::RenderLayerBacking::updateDrawsContent() + 0x19 (RenderLayerBacking.cpp:611)
   7 com.apple.WebCore              0x10b71fdda WebCore::RenderLayerCompositor::rootLayerAttachmentChanged() + 0x6a (RenderLayerCompositor.cpp:2199)
   8 com.apple.WebCore              0x10b71e2f7 WebCore::RenderLayerCompositor::detachRootLayer() + 0x1d7 (RenderLayerCompositor.cpp:2185)
   9 com.apple.WebCore              0x10b71e115 WebCore::RenderLayerCompositor::willMoveOffscreen() + 0x45 (RenderLayerCompositor.cpp:1269)
  10 com.apple.WebCore              0x10b8548db WebCore::RenderView::willMoveOffscreen() + 0x4b (RenderView.cpp:887)
  11 com.apple.WebCore              0x10abd6fe5 WebCore::FrameView::willMoveOffscreen() + 0x65 (FrameView.cpp:877)
  12 com.apple.WebCore              0x10b572776 WebCore::Page::willMoveOffscreen() + 0x56 (Page.cpp:704)
  13 com.apple.WebKit2              0x108cc9cd1 WebKit::WebPage::setIsInWindow(bool) + 0x71 (WebPage.cpp:1700)
  14 com.apple.WebKit2              0x108cc7f92 WebKit::WebPage::WebPage(unsigned long long, WebKit::WebPageCreationParameters const&) + 0xa12 (WebPage.cpp:298)
  15 com.apple.WebKit2              0x108cc7575 WebKit::WebPage::WebPage(unsigned long long, WebKit::WebPageCreationParameters const&) + 0x25 (WebPage.cpp:312)
  16 com.apple.WebKit2              0x108cc74a1 WebKit::WebPage::create(unsigned long long, WebKit::WebPageCreationParameters const&) + 0x41 (WebPage.cpp:176)
  17 com.apple.WebKit2              0x108d7e54c WebKit::WebProcess::createWebPage(unsigned long long, WebKit::WebPageCreationParameters const&) + 0xec (WebProcess.cpp:530)
  18 com.apple.WebKit2              0x108d96518 void CoreIPC::callMemberFunction<WebKit::WebProcess, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&), unsigned long long, WebKit::WebPageCreationParameters>(CoreIPC::Arguments2<unsigned long long, WebKit::WebPageCreationParameters> const&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&)) + 0x98 (HandleMessage.h:26)
  19 com.apple.WebKit2              0x108d94c91 void CoreIPC::handleMessage<Messages::WebProcess::CreateWebPage, WebKit::WebProcess, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&)>(CoreIPC::ArgumentDecoder*, WebKit::WebProcess*, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&)) + 0x91 (HandleMessage.h:303)
  20 com.apple.WebKit2              0x108d94322 WebKit::WebProcess::didReceiveWebProcessMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 0xb2 (WebProcessMessageReceiver.cpp:94)
  21 com.apple.WebKit2              0x108d7e9bb WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 0x5b (WebProcess.cpp:604)
  22 com.apple.WebKit2              0x108c2c17e WebKit::WebConnectionToUIProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 0x15e (WebConnectionToUIProcess.cpp:88)
  23 com.apple.WebKit2              0x108c2c1cd non-virtual thunk to WebKit::WebConnectionToUIProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*) + 0x3d
  24 com.apple.WebKit2              0x108ad655c CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::ArgumentDecoder>&) + 0x15c (Connection.cpp:692)
  25 com.apple.WebKit2              0x108ad8ca8 CoreIPC::Connection::dispatchMessages() + 0xc8 (Connection.cpp:720)
  26 com.apple.WebKit2              0x108adf752 WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*) + 0x72 (Functional.h:173)
  27 com.apple.WebKit2              0x108adf6d5 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void (CoreIPC::Connection*)>::operator()() + 0x35 (Functional.h:405)
  28 com.apple.WebCore              0x10b89f155 WTF::Function<void ()>::operator()() const + 0x85 (Functional.h:613)
  29 com.apple.WebCore              0x10b89eee7 WebCore::RunLoop::performWork() + 0x87 (RunLoop.cpp:66)
  30 com.apple.WebCore              0x10b8a0270 WebCore::RunLoop::performWork(void*) + 0x60 (RunLoopMac.mm:65)

(I wonder if there is a way of testing this code path too)

The best fix is to introduce an iterator to make sure we don't forget to update the z-index lists.

[Julien Chaffraix]

Created attachment 140056 [details]
Quick 'n' dirty fix 1. Don't have time to do the iterator, will file a bug about that.

[Simon Fraser (smfr)]

Comment on attachment 140056 [details]
Quick 'n' dirty fix 1. Don't have time to do the iterator, will file a bug about that.

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

> Source/WebCore/rendering/RenderLayerBacking.cpp:910
>  bool RenderLayerBacking::hasVisibleNonCompositingDescendantLayers() const
>  {
> +    m_owningLayer->updateLayerListsIfNeeded();

Not so sure about this one. Ideally we'd never be calling this with stale z-order lists. I think a better fix is to have RenderLayerCompositor::rootLayerAttachmentChanged() do no work if m_rootLayerAttachment == RootLayerUnattached;

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2232
> +    const_cast<RenderLayer*>(layer)->updateLayerListsIfNeeded();
> +

This one is fine.

[Julien Chaffraix]

Comment on attachment 140056 [details]
Quick 'n' dirty fix 1. Don't have time to do the iterator, will file a bug about that.

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

>> Source/WebCore/rendering/RenderLayerBacking.cpp:910
>> +    m_owningLayer->updateLayerListsIfNeeded();
> 
> Not so sure about this one. Ideally we'd never be calling this with stale z-order lists. I think a better fix is to have RenderLayerCompositor::rootLayerAttachmentChanged() do no work if m_rootLayerAttachment == RootLayerUnattached;

rootLayerAttachmentChanged is only called a 2 places and only one is expected to call with m_rootLayerAttachment == RootLayerUnattached: detachRootLayer. If I follow your suggestion, it basically means to remove the call to rootLayerAttachmentChanged in detachRootLayer. I don't know this code enough to say if this is fine.

[Simon Fraser (smfr)]

Comment on attachment 140056 [details]
Quick 'n' dirty fix 1. Don't have time to do the iterator, will file a bug about that.

We can fix that up later. I'm OK with this patch.

[Julien Chaffraix]

Created attachment 140099 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 140099 [details]
Patch for landing

Clearing flags on attachment: 140099

Committed r116032: <http://trac.webkit.org/changeset/116032>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.