REGRESSION (r17068): Repro crash due to painting without layout

• Component: Layout and Rendering
• Status: RESOLVED FIXED

The URL crashes WebKit or causes an assertion failure as it tries to paint a table that is still marked as needing layout. Breaking in gdb reveals that the entire render tree is marked for layout except for the root RenderView (which is why the draw message from AppKit doesn't trigger layout; this logic in -[WebCoreFrameBridge needsLayout] seems to be wrong anyway since there can be a pending subtree relayout, but I don't think this is the case here).

RenderBlock::layoutPositionedObjects() calls setChildNeedsLayout() on the positioned objects without passing false for the markParents flag. This is wrong and it leads to the block (in this case, the RenderView) having its child re-marked as needing layout after the block finished laying out its children.

While it's nice that most objects other than tables manage to paint while needing layout, it's a condition that shouldn't occur. Perhaps this can be asserted in some of the more popular paint() methods.

I wonder if it's possible to make a reduction that will crash builds from before r17068. I think it should be possible.

Created attachment 11605 [details]
Reduction (crashes when Test is clicked)

Here's the connection to r17068: RenderObject::setStyle() takes care of reassigning positioned objects to their containing block when a container's position property changes. It has logic to determine the current containing block for it absolutely positioned children, which needs to be in sync with the corresponding case in containingBlock(). r17068 changed containingBlock() and container() by eliminating the !isRoot() check, but left it in setStyle():
                    while (cb && (cb->style()->position() == StaticPosition || (cb->isInline() && !cb->isReplaced())) && !cb->isRoot() && !cb->isRenderView()) {

Having said that, the two other issues (marking parents when you shouldn't and -[WebCoreFrameBridge needsLayout] disregarding subtree layout) are still valid in my opinion.

(In reply to comment #2)
> the two other issues [...] are still valid
> in my opinion.

Actually, marking parents shouldn't be a problem since there shouldn't be intermediate containers between the caller and the positioned object. It's still a waste of time to mark parents.

Created attachment 11607 [details]
Change containing block logic in setStyle() to match containingBlock()

No test regressions.

[Dave Hyatt]

Comment on attachment 11607 [details]
Change containing block logic in setStyle() to match containingBlock()

r=me

[Alexey Proskuryakov]

Committed revision 17986.