Bug 148956 - [GTK] ASSERTION FAILED: !m_inUpdateBackingStoreState in DrawingAreaImpl::display() after DrawingAreaImpl::forceRepaint()
Summary: [GTK] ASSERTION FAILED: !m_inUpdateBackingStoreState in DrawingAreaImpl::disp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-08 08:23 PDT by Carlos Alberto Lopez Perez
Modified: 2015-09-25 02:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.37 KB, patch)
2015-09-15 04:36 PDT, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2015-09-08 08:23:34 PDT
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
[...]
Comment 1 Carlos Alberto Lopez Perez 2015-09-08 08:44:07 PDT
Marked accordingly in http://trac.webkit.org/changeset/189487
Comment 2 Carlos Garcia Campos 2015-09-15 04:27:37 PDT
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.
Comment 3 Carlos Garcia Campos 2015-09-15 04:36:52 PDT
Created attachment 261187 [details]
Patch
Comment 4 Zan Dobersek 2015-09-25 02:27:26 PDT
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.
Comment 5 Carlos Garcia Campos 2015-09-25 02:37:58 PDT
(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!
Comment 6 Carlos Garcia Campos 2015-09-25 02:42:49 PDT
Committed r190238: <http://trac.webkit.org/changeset/190238>