Bug 104518 – [CoordinatedGraphics] Assertion hit in WebKit::LayerTreeRenderer::setLayerState()

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

ietestcenter/css3/valuesandunits/units-010.htm has just hit the following assertion on our EFL WK2 Debug build bot:
STDERR: ASSERTION FAILED: m_rootLayerID != InvalidCoordinatedLayerID
STDERR: /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp(331) : void WebKit::LayerTreeRenderer::setLayerState(WebKit::CoordinatedLayerID, const WebKit::CoordinatedLayerInfo&)
STDERR: 1   0x7f8341a6a348 WebKit::LayerTreeRenderer::setLayerState(unsigned int, WebKit::CoordinatedLayerInfo const&)
STDERR: 2   0x7f8341a68a93 WTF::FunctionWrapper<void (WebKit::LayerTreeRenderer::*)(unsigned int, WebKit::CoordinatedLayerInfo const&)>::operator()(WebKit::LayerTreeRenderer*, unsigned int, WebKit::CoordinatedLayerInfo const&)
STDERR: 3   0x7f8341a680a4 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::LayerTreeRenderer::*)(unsigned int, WebKit::CoordinatedLayerInfo const&)>, void (WebKit::LayerTreeRenderer*, unsigned int, WebKit::CoordinatedLayerInfo)>::operator()()
STDERR: 4   0x7f8348e56a62 WTF::Function<void ()>::operator()() const
STDERR: 5   0x7f8341a6c034 WebKit::LayerTreeRenderer::syncRemoteContent()
STDERR: 6   0x7f8341a692db WebKit::LayerTreeRenderer::paintToCurrentGLContext(WebCore::TransformationMatrix const&, float, WebCore::FloatRect const&, unsigned int)
STDERR: 7   0x7f8341ba9a18 EwkViewImpl::displayTimerFired(WebCore::Timer<EwkViewImpl>*)
STDERR: 8   0x7f8341bb103e WebCore::Timer<EwkViewImpl>::fired()
STDERR: 9   0x7f834514fad2 WebCore::ThreadTimers::sharedTimerFiredInternal()
STDERR: 10  0x7f834514f9f3 WebCore::ThreadTimers::sharedTimerFired()
STDERR: 11  0x7f8345bb2a19
STDERR: 12  0x7f8340cf533e _ecore_timer_expired_call
STDERR: 13  0x7f8340cf550b _ecore_timer_expired_timers_call
STDERR: 14  0x7f8340cf2421
STDERR: 15  0x7f8340cf2ab7 ecore_main_loop_begin
STDERR: 16  0x433ba9 WTR::TestController::platformRunUntil(bool&, double)
STDERR: 17  0x41ecd2 WTR::TestController::runUntil(bool&, WTR::TestController::TimeoutDuration)
STDERR: 18  0x42d654 WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*)
STDERR: 19  0x42617b WTR::TestInvocation::dumpResults()
STDERR: 20  0x425ddf WTR::TestInvocation::invoke()
STDERR: 21  0x41ea0a WTR::TestController::runTest(char const*)
STDERR: 22  0x41eb43 WTR::TestController::runTestingServerLoop()
STDERR: 23  0x41ebdd WTR::TestController::run()
STDERR: 24  0x41c513 WTR::TestController::TestController(int, char const**)
STDERR: 25  0x433d42 main
STDERR: 26  0x7f833f85376d __libc_start_main
STDERR: 27  0x41ae39
STDERR: LEAK: 49 WebCoreNode

[Dongseong Hwang]

Thanks for reporting. I'll investigate

[Dongseong Hwang]

I can not reproduce yet on EFL WK2 Debug.
I don't understand how it happens yet too.
I need to take a time to investigate.

[Dongseong Hwang]

(In reply to comment #2)
> I can not reproduce yet on EFL WK2 Debug.
> I don't understand how it happens yet too.
> I need to take a time to investigate.

I reproduced in both Qt and EFL but I can not understand yet.

It occurs very flaky. I'm making a minimal reproducing test. It may takes time.

[Dongseong Hwang]

Created attachment 179190 [details]
Patch

[Dongseong Hwang]

ping.

It blocks Bug 103959

[Noam Rosenthal]

Comment on attachment 179190 [details]
Patch

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

> Source/WebKit2/ChangeLog:13
> +        RenderLayerBacking can call GraphicsLayer::flushCompositingState() regardless
> +        of CoordinatedLayerTreeHost and it breaks our assumption. It means that
> +        CoordinatedGraphicsLayer can send messages although m_waitingForUIProcess in
> +        CoordinatedLayerTreeHost is true.

Have you investigated why RenderLayerBacking calls flushCompositingState, and if it's safe to return early in that situation?

[Dongseong Hwang]

(In reply to comment #6)
> (From update of attachment 179190 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179190&action=review
> 
> > Source/WebKit2/ChangeLog:13
> > +        RenderLayerBacking can call GraphicsLayer::flushCompositingState() regardless
> > +        of CoordinatedLayerTreeHost and it breaks our assumption. It means that
> > +        CoordinatedGraphicsLayer can send messages although m_waitingForUIProcess in
> > +        CoordinatedLayerTreeHost is true.
> 
> Have you investigated why RenderLayerBacking calls flushCompositingState, and if it's safe to return early in that situation?

I should say RenderLayerCompositor can call flushCompositingState().

For example,
1. RenderLayerCompositor::attachRootLayer can cause calling GraphicsLayer::flushCompositingState
void RenderLayerCompositor::attachRootLayer(RootLayerAttachment attachment)
{
    ...    
    if (m_shouldFlushOnReattach) {
        flushPendingLayerChanges(true); // this method finally call rootLayer->flushCompositingState(visibleRect);
        m_shouldFlushOnReattach = false;
    }
}

2. FrameView::paintContents can cause calling GraphicsLayer::flushCompositingState
void FrameView::paintContents(GraphicsContext* p, const IntRect& rect)
{
   ...
#if USE(ACCELERATED_COMPOSITING)
    if (!p->paintingDisabled() && !document->printing())
        flushCompositingStateForThisFrame(m_frame.get());
#endif
   ...
}


And I think return early is safe, because we remember we need to sync if we return early.
For example, if we return early because RenderLayerCompositor calls flushCompositingState, we don't set m_shouldSyncLayerState to false. So we can send messages to UI Process at actual flush time by timer of CoordinatedLayerTreeHost.

[Noam Rosenthal]

Comment on attachment 179190 [details]
Patch

FrameView::paintContents should be getting called during flush, because that's when we paint the root layer.
attachRootLayer should not be called if we're in forceCOmpositingMode, except for the first attach.

This seems a bit like a speculative fix - does the bug actually happen?

[Dongseong Hwang]

(In reply to comment #8)
> (From update of attachment 179190 [details])
> FrameView::paintContents should be getting called during flush, because that's when we paint the root layer.
> attachRootLayer should not be called if we're in forceCOmpositingMode, except for the first attach.
> 
> This seems a bit like a speculative fix - does the bug actually happen?

Reproducing this bug via run_launcher is pretty hard.
However we can reproduce via run_webkit_test.
If we test via "./run_webkit_test --qt -2 --debug compositing", about 2 of 280 crash very flaky. But after this patch, I can not reproduce.

Above examples can not cover whole control flows about RenderLayerCompositor::flushCompositingState. I just presented 2 examples.
I think calling attachRootLayer for the first attach can cause this crash.

[Noam Rosenthal]

(In reply to comment #9)
> Above examples can not cover whole control flows about RenderLayerCompositor::flushCompositingState. I just presented 2 examples.
> I think calling attachRootLayer for the first attach can cause this crash.

We should not call a sync after attach when in forceCompositingMode.

[Dongseong Hwang]

(In reply to comment #10)
> (In reply to comment #9)
> > Above examples can not cover whole control flows about RenderLayerCompositor::flushCompositingState. I just presented 2 examples.
> > I think calling attachRootLayer for the first attach can cause this crash.
> 
> We should not call a sync after attach when in forceCompositingMode.

Ok, I'll find better way to do. :)

[Dongseong Hwang]

especially layout test often calls RenderLayerCompositor::flushCompositingState regardless of the timer of CoordinatedLayerTreeHost.
I think writing workaround code about these three cases is vulnerable because we can not discourage any future requirement does not directly call RenderLayerCompositor::flushCompositingState.
IMO, this patch is good to prevent from sending messages at any time. Could you suggest better solution?


#1 - the most frequent (146/338)
STDERR: /media/WDDisk/workspace/WebKit/WebKitEFL/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp(450) : virtual void WebCore::CoordinatedGraphicsLayer::flushCompositingState(const WebCore::FloatRect&)
STDERR: 1   0x7f82d8f694c4 WebCore::CoordinatedGraphicsLayer::flushCompositingState(WebCore::FloatRect const&)
STDERR: 2   0x7f82d52e4550 WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool)
STDERR: 3   0x7f82d52e7919 WebCore::RenderLayerCompositor::layerTreeAsText(unsigned int)
STDERR: 4   0x7f82d4fc64b8 WebCore::Frame::layerTreeAsText(unsigned int) const
STDERR: 5   0x7f905619d11d WebCore::Internals::layerTreeAsText(WebCore::Document*, unsigned int, int&) const
STDERR: 6   0x7f905619d06a WebCore::Internals::layerTreeAsText(WebCore::Document*, int&) const
STDERR: 7   0x7f905617ea42 WebCore::jsInternalsPrototypeFunctionLayerTreeAsText(JSC::ExecState*)

#2 - often case - compositing/images/truncated-direct-png-image.html , compositing/regions/webkit-flow-renderer-layer-compositing.html , compositing/document-background-color.html , compositing/text-on-scaled-surface.html , compositing/repaint/clipped-layer-size-change.html , compositing/text-on-scaled-layer.html , compositing/geometry/fixed-position-composited-page-scale-smaller-than-viewport.html , compositing/tile-cache-must-flatten.html , compositing/background-color/background-color-text-change.html , compositing/background-color/background-color-simple.html , compositing/background-color/background-color-content-clip.html , compositing/background-color/background-color-container.html , compositing/background-color/background-color-text-clip.html , compositing/background-color/background-color-change-to-text.html , compositing/background-color/background-color-padding-clip.html , compositing/background-color/background-color-padding-change.html , compositing/background-color/background-color-alpha.html , compositing/background-color/background-color-composite.html
STDERR: /media/WDDisk/workspace/WebKit/WebKitEFL/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp(450) : virtual void WebCore::CoordinatedGraphicsLayer::flushCompositingState(const WebCore::FloatRect&)
STDERR: 1   0x7f1f252874c4 WebCore::CoordinatedGraphicsLayer::flushCompositingState(WebCore::FloatRect const&)
STDERR: 2   0x7f1f21602550 WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool)
STDERR: 3   0x7f1f212efeb3 WebCore::FrameView::flushCompositingStateForThisFrame(WebCore::Frame*)
STDERR: 4   0x7f1f212f8d33 WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&)
STDERR: 5   0x7f1f213988e5 WebCore::ScrollView::paint(WebCore::GraphicsContext*, WebCore::IntRect const&)
STDERR: 6   0x7f1f212f922a WebCore::FrameView::paintContentsForSnapshot(WebCore::GraphicsContext*, WebCore::IntRect const&, WebCore::FrameView::SelectionInSnaphot, WebCore::FrameView::CoordinateSpaceForSnapshot)
STDERR: 7   0x7f1f2526d446 WebKit::WebPage::scaledSnapshotWithOptions(WebCore::IntRect const&, double, unsigned int)
STDERR: 8   0x7f1f251d5450 WKBundlePageCreateSnapshotWithOptions
STDERR: 9   0x7f1ec97eec8d WTR::InjectedBundlePage::dump()
STDERR: 10  0x7f1ec97f4043 WTR::InjectedBundlePage::frameDidChangeLocation(OpaqueWKBundleFrame const*, bool)
STDERR: 11  0x7f1ec97eeede WTR::InjectedBundlePage::didFinishLoadForFrame(OpaqueWKBundleFrame const*)
STDERR: 12  0x7f1ec97ecdef WTR::InjectedBundlePage::didFinishLoadForFrame(OpaqueWKBundlePage const*, OpaqueWKBundleFrame const*, void const**, void const*)
STDERR: 13  0x7f1f251cb163 WebKit::InjectedBundlePageLoaderClient::didFinishLoadForFrame(WebKit::WebPage*, WebKit::WebFrame*, WTF::RefPtr<WebKit::APIObject>&)
STDERR: 14  0x7f1f25238612 WebKit::WebFrameLoaderClient::dispatchDidFinishLoad()
STDERR: 15  0x7f1f211e6544 WebCore::FrameLoader::checkLoadCompleteForThisFrame()
STDERR: 16  0x7f1f211e7108 WebCore::FrameLoader::checkLoadComplete()
STDERR: 17  0x7f1f211e00e5 WebCore::FrameLoader::checkCompleted()
STDERR: 18  0x7f1f211dfe84 WebCore::FrameLoader::loadDone()
STDERR: 19  0x7f1f2125e4c0 WebCore::CachedResourceLoader::loadDone(WebCore::CachedResource*)
STDERR: 20  0x7f1f21214104 WebCore::SubresourceLoader::releaseResources()
STDERR: 21  0x7f1f2120eac8 WebCore::ResourceLoader::didFinishLoading(double)
STDERR: 22  0x7f1f21213cb4 WebCore::SubresourceLoader::didFinishLoading(double)
STDERR: 23  0x7f1f2120f241 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double)

#3 - rare case - compositing/iframes/enter-compositing-iframe.html , compositing/visibility/hidden-iframe.html
STDERR: /media/WDDisk/workspace/WebKit/WebKitEFL/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp(450) : virtual void WebCore::CoordinatedGraphicsLayer::flushCompositingState(const WebCore::FloatRect&)
STDERR: 1   0x7f270c76e4c4 WebCore::CoordinatedGraphicsLayer::flushCompositingState(WebCore::FloatRect const&)
STDERR: 2   0x7f2708ae9550 WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool)
STDERR: 3   0x7f2708af0c9b WebCore::RenderLayerCompositor::attachRootLayer(WebCore::RenderLayerCompositor::RootLayerAttachment)
STDERR: 4   0x7f2708af0647 WebCore::RenderLayerCompositor::ensureRootLayer()
STDERR: 5   0x7f2708ae8f51 WebCore::RenderLayerCompositor::enableCompositingMode(bool)
STDERR: 6   0x7f2708aea3ed WebCore::RenderLayerCompositor::updateBacking(WebCore::RenderLayer*, WebCore::RenderLayerCompositor::CompositingChangeRepaint)
STDERR: 7   0x7f2708aea837 WebCore::RenderLayerCompositor::updateLayerCompositingState(WebCore::RenderLayer*, WebCore::RenderLayerCompositor::CompositingChangeRepaint)
STDERR: 8   0x7f2708ad1f88 WebCore::RenderLayer::styleChanged(WebCore::StyleDifference, WebCore::RenderStyle const*)
STDERR: 9   0x7f2708afa343 WebCore::RenderLayerModelObject::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
STDERR: 10  0x7f2708a36829 WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
STDERR: 11  0x7f27089c6060 WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
STDERR: 12  0x7f2708b2700e WebCore::RenderObject::setStyle(WTF::PassRefPtr<WebCore::RenderStyle>)
STDERR: 13  0x7f2708b26772 WebCore::RenderObject::setAnimatableStyle(WTF::PassRefPtr<WebCore::RenderStyle>)
STDERR: 14  0x7f270827c551 WebCore::Element::recalcStyle(WebCore::Node::StyleChange)
STDERR: 15  0x7f270827c8a8 WebCore::Element::recalcStyle(WebCore::Node::StyleChange)
STDERR: 16  0x7f270827c8a8 WebCore::Element::recalcStyle(WebCore::Node::StyleChange)
STDERR: 17  0x7f270821878e WebCore::Document::recalcStyle(WebCore::Node::StyleChange)
STDERR: 18  0x7f2708218a6a WebCore::Document::updateStyleIfNeeded()

[Dongseong Hwang]

Created attachment 179685 [details]
Patch

[Dongseong Hwang]

(In reply to comment #13)
> Created an attachment (id=179685) [details]
> Patch

Change ChangeLog from RenderLayerBacking to RenderLayerCompositor.

[Build Bot]

Comment on attachment 179685 [details]
Patch

Attachment 179685 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15378548

New failing tests:
fast/frames/sandboxed-iframe-attribute-parsing.html

[Dongseong Hwang]

Created attachment 179694 [details]
Patch

[Dongseong Hwang]

(In reply to comment #16)
> Created an attachment (id=179694) [details]
> Patch

mac ews failed flaky, so reposted.

[Dongseong Hwang]

Created attachment 179814 [details]
Patch

[Dongseong Hwang]

Comment on attachment 179814 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:444
> +            client()->notifyFlushRequired(this);

Add this line to make more robust code.

[Noam Rosenthal]

Comment on attachment 179814 [details]
Patch

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

LGTM. Please revise the Changelog and commit when you can follow up.

> Source/WebKit2/ChangeLog:18
> +

Add:
We fix this by ensuring that we perform the layer flush only in the code path originating from CoordinatedLayerTreeHost.

[Dongseong Hwang]

Created attachment 179816 [details]
Patch

[Dongseong Hwang]

(In reply to comment #20)
> (From update of attachment 179814 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179814&action=review

Thank you for review.

> LGTM. Please revise the Changelog and commit when you can follow up.
> 
> > Source/WebKit2/ChangeLog:18
> > +
> 
> Add:
> We fix this by ensuring that we perform the layer flush only in the code path originating from CoordinatedLayerTreeHost.

Good advice!

[WebKit Review Bot]

Comment on attachment 179816 [details]
Patch

Clearing flags on attachment: 179816

Committed r137958: <http://trac.webkit.org/changeset/137958>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.