Bug 104518 - [CoordinatedGraphics] Assertion hit in WebKit::LayerTreeRenderer::setLayerState()
Summary: [CoordinatedGraphics] Assertion hit in WebKit::LayerTreeRenderer::setLayerSta...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on:
Blocks: 103843 103959
  Show dependency treegraph
 
Reported: 2012-12-10 00:27 PST by Chris Dumez
Modified: 2012-12-17 16:28 PST (History)
6 users (show)

See Also:


Attachments
Patch (6.35 KB, patch)
2012-12-12 21:02 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (6.34 KB, patch)
2012-12-16 20:59 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (6.34 KB, patch)
2012-12-16 23:51 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2012-12-17 15:32 PST, Dongseong Hwang
noam: review+
noam: commit-queue-
Details | Formatted Diff | Diff
Patch (6.58 KB, patch)
2012-12-17 15:45 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-12-10 00:27:53 PST
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
Comment 1 Dongseong Hwang 2012-12-10 01:01:59 PST
Thanks for reporting. I'll investigate
Comment 2 Dongseong Hwang 2012-12-10 01:47:48 PST
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.
Comment 3 Dongseong Hwang 2012-12-10 03:06:26 PST
(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.
Comment 4 Dongseong Hwang 2012-12-12 21:02:16 PST
Created attachment 179190 [details]
Patch
Comment 5 Dongseong Hwang 2012-12-13 16:14:56 PST
ping.

It blocks Bug 103959
Comment 6 Noam Rosenthal 2012-12-14 08:39:41 PST
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?
Comment 7 Dongseong Hwang 2012-12-14 09:18:29 PST
(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.
Comment 8 Noam Rosenthal 2012-12-15 10:28:25 PST
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?
Comment 9 Dongseong Hwang 2012-12-15 19:07:39 PST
(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.
Comment 10 Noam Rosenthal 2012-12-15 19:09:57 PST
(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.
Comment 11 Dongseong Hwang 2012-12-16 01:58:15 PST
(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. :)
Comment 12 Dongseong Hwang 2012-12-16 19:48:46 PST
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()
Comment 13 Dongseong Hwang 2012-12-16 20:59:39 PST
Created attachment 179685 [details]
Patch
Comment 14 Dongseong Hwang 2012-12-16 21:00:24 PST
(In reply to comment #13)
> Created an attachment (id=179685) [details]
> Patch

Change ChangeLog from RenderLayerBacking to RenderLayerCompositor.
Comment 15 Build Bot 2012-12-16 23:27:32 PST
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
Comment 16 Dongseong Hwang 2012-12-16 23:51:03 PST
Created attachment 179694 [details]
Patch
Comment 17 Dongseong Hwang 2012-12-16 23:52:24 PST
(In reply to comment #16)
> Created an attachment (id=179694) [details]
> Patch

mac ews failed flaky, so reposted.
Comment 18 Dongseong Hwang 2012-12-17 15:32:30 PST
Created attachment 179814 [details]
Patch
Comment 19 Dongseong Hwang 2012-12-17 15:33:34 PST
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.
Comment 20 Noam Rosenthal 2012-12-17 15:37:27 PST
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.
Comment 21 Dongseong Hwang 2012-12-17 15:45:17 PST
Created attachment 179816 [details]
Patch
Comment 22 Dongseong Hwang 2012-12-17 15:48:22 PST
(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!
Comment 23 WebKit Review Bot 2012-12-17 16:27:58 PST
Comment on attachment 179816 [details]
Patch

Clearing flags on attachment: 179816

Committed r137958: <http://trac.webkit.org/changeset/137958>
Comment 24 WebKit Review Bot 2012-12-17 16:28:03 PST
All reviewed patches have been landed.  Closing bug.