Bug 148956 – [GTK] ASSERTION FAILED: !m_inUpdateBackingStoreState in DrawingAreaImpl::display() after DrawingAreaImpl::forceRepaint()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Alberto Lopez Perez]

The following tests are crashing on the Debug build of WebKitGTK+ because of a failed assertion:

Regressions: Unexpected crashes (9)
  css3/viewport-percentage-lengths/viewport-percentage-lengths-anonymous-block.html [ Crash ]
  css3/viewport-percentage-lengths/viewport-percentage-lengths-percent-size-child.html [ Crash ]
  css3/viewport-percentage-lengths/viewport-percentage-lengths-relative-font-size.html [ Crash ]
  css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html [ Crash ]
  fast/css-grid-layout/flex-content-sized-columns-resize.html [ Crash ]
  fast/dom/Window/window-resize-contents.html [ Crash ]
  fast/dom/rtl-scroll-to-leftmost-and-resize.html [ Crash ]
  fast/dynamic/window-resize-scrollbars-test.html [ Crash ]
  fast/fixed-layout/fixed-layout.html [ Crash ]


Backtrace (for fast/css-grid-layout/flex-content-sized-columns-resize.html):
(gdb) bt
#0  WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
#1  0x00007f2b8aba22e1 in WebKit::DrawingAreaImpl::display (this=0x1752720) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:564
#2  0x00007f2b8aba2084 in WebKit::DrawingAreaImpl::forceRepaint (this=0x1752720) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:220
#3  0x00007f2b8a9f397d in WebKit::WebPage::forceRepaintWithoutCallback() () at ../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2625
#4  0x00007f2b8a8bdf6d in WKBundlePageForceRepaint (page=0x7f2b730054c0) at ../../Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:471
#5  0x00007f2b302a491c in WTR::InjectedBundlePage::dump (this=0x186f520) at ../../Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:851
#6  0x00007f2b302b47b7 in WTR::TestRunner::notifyDone (this=0x7f2b732f0348) at ../../Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:163
#7  0x00007f2b302cab3a in WTR::JSTestRunner::notifyDone (context=0x7fff9730d3d0, thisObject=0x7f2b730d3080, argumentCount=0, arguments=0x7fff9730cf40, 
    exception=0x7fff9730cf08) at DerivedSources/InjectedBundle/JSTestRunner.cpp:249
#8  0x00007f2b85293169 in JSC::APICallbackFunction::call<JSC::JSCallbackFunction> (exec=0x7fff9730d3d0) at ../../Source/JavaScriptCore/API/APICallbackFunction.h:61
#9  0x00007f2b85c50e66 in JSC::LLInt::handleHostCall (execCallee=0x7fff9730d3d0, pc=0x7f2b732e7a70, callee=..., kind=JSC::CodeForCall)
    at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1064
#10 0x00007f2b85c51e4a in JSC::LLInt::setUpCall (execCallee=0x7fff9730d3d0, pc=0x7f2b732e7a70, kind=JSC::CodeForCall, calleeAsValue=..., callLinkInfo=0x7f2b7334f6b8)
    at ../../Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1110
[...]

[Carlos Alberto Lopez Perez]

Marked accordingly in http://trac.webkit.org/changeset/189487

[Carlos Garcia Campos]

This is because those tests (or at least fast/css-grid-layout/flex-content-sized-columns-resize.html, I haven't looked at the others but I guess it's the same thing), call notifyDone in the onresize event handler. InjectedBundlePage::dump() always calls WKBundlePageForceRepaint() before dumping, but I'm not even sure that's actually needed with current WTR implementation. A layout would be enough, we don't really need to do the actual display. The thing is that when the view is resized DrawingAreaImpl::updateBackingStoreState() is called, if the size has changed, the FrameView::resize() method is called and all children are resized, so the onsresize handlers happen at that point, before the m_inUpdateBackingStoreState is set to false again. For WTR we could probably just return early from froceReapaint() when m_inUpdateBackingStoreState is true, because in that case we know the layout is updated because of the resize and the actual display is not really needed. But the UI process can also request a force repaint, so we could wait until the backing store update is done and then force the repaint. For WTR is will happen after the the dump, but it shouldn't be a problem.

[Carlos Garcia Campos]

Created attachment 261187 [details]
Patch

[Zan Dobersek]

Comment on attachment 261187 [details]
Patch

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

Looks fine, sorry about the grammar nits.

> Source/WebKit2/ChangeLog:10
> +        before dumping, When the view is resized DrawingAreaImpl::updateBackingStoreState()

Incorrect comma.

> Source/WebKit2/ChangeLog:12
> +        method is called and all children are resized, so the onsresize

onsresize.

> Source/WebKit2/ChangeLog:20
> +        and then force the repaint. For WTR it will happen after the the

Double the.

[Carlos Garcia Campos]

(In reply to comment #4)
> Comment on attachment 261187 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261187&action=review
> 
> Looks fine, sorry about the grammar nits.

:-)

> > Source/WebKit2/ChangeLog:10
> > +        before dumping, When the view is resized DrawingAreaImpl::updateBackingStoreState()
> 
> Incorrect comma.
> 
> > Source/WebKit2/ChangeLog:12
> > +        method is called and all children are resized, so the onsresize
> 
> onsresize.
> 
> > Source/WebKit2/ChangeLog:20
> > +        and then force the repaint. For WTR it will happen after the the
> 
> Double the.

Thanks!

[Carlos Garcia Campos]

Committed r190238: <http://trac.webkit.org/changeset/190238>