Bug 67759 – Positioned Floats: Assertion hit in fast/block/positioning/positioned-float-layout-after-image-load.html

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ryosuke Niwa]

fast/block/positioning/positioned-float-layout-after-image-load.html started hitting an assertion since the build 2042 today.

See also:
http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r94686%20(2040)/results.html
http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r94700%20(2042)/results.html
http://build.webkit.org/builders/SnowLeopard%20Intel%20Debug%20%28Tests%29/builds/2042

ASSERTION FAILED: isPlaced()
/Volumes/Big/slave/snowleopard-intel-debug/build/Source/WebCore/rendering/RenderBlock.h(473) : WebCore::LayoutUnit WebCore::RenderBlock::FloatingObject::y() const
1   WebCore::RenderBlock::FloatingObject::y() const
2   WebCore::RenderBlock::logicalTopForFloat(WebCore::RenderBlock::FloatingObject const*) const
3   WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(WebCore::RenderBlock::FloatingObject*)
4   WebCore::RenderBlock::positionNewFloatOnLine(WebCore::RenderBlock::FloatingObject*, WebCore::RenderBlock::FloatingObject*, WebCore::LineWidth&)
5   WebCore::RenderBlock::LineBreaker::skipLeadingWhitespace(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo const&, WebCore::RenderBlock::FloatingObject*, WebCore::LineWidth&)
6   WebCore::RenderBlock::LineBreaker::nextLineBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, std::pair<WebCore::RenderText*, WebCore::LazyLineBreakIterator>&, WebCore::RenderBlock::FloatingObject*)
7   WebCore::RenderBlock::layoutRunsAndFloatsInRange(WebCore::LineLayoutState&, WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::InlineIterator const&, WebCore::BidiStatus const&)
8   WebCore::RenderBlock::layoutRunsAndFloats(WebCore::LineLayoutState&, bool)
9   WebCore::RenderBlock::layoutInlineChildren(bool, int&, int&)
10  WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
11  WebCore::RenderBlock::layout()
12  WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
13  WebCore::RenderBlock::layoutBlockChildren(bool, int&)
14  WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
15  WebCore::RenderBlock::layout()
16  WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
17  WebCore::RenderBlock::layoutBlockChildren(bool, int&)
18  WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
19  WebCore::RenderBlock::layout()
20  WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&)
21  WebCore::RenderBlock::layoutBlockChildren(bool, int&)
22  WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass)
23  WebCore::RenderBlock::layout()
24  WebCore::RenderView::layout()
25  WebCore::FrameView::layout(bool)
26  WebCore::Document::updateLayout()
27  WebCore::Document::updateLayoutIgnorePendingStylesheets()
28  WebCore::Element::offsetTop()
29  WebCore::jsElementOffsetTop(JSC::ExecState*, JSC::JSValue, JSC::Identifier const&)
30  JSC::PropertySlot::getValue(JSC::ExecState*, JSC::Identifier const&) const
31  JSC::JSValue::get(JSC::ExecState*, JSC::Identifier const&, JSC::PropertySlot&) const

[Ryosuke Niwa]

Suspicious revisions:
http://trac.webkit.org/changeset/94690
http://trac.webkit.org/changeset/94693
http://trac.webkit.org/changeset/94695
http://trac.webkit.org/changeset/94697
http://trac.webkit.org/changeset/94698

[Ryosuke Niwa]

The test was added by http://trac.webkit.org/changeset/94695.

[Csaba Osztrogonác]

Same assert on GTK and on Qt too.

[Csaba Osztrogonác]

*** Bug 67769 has been marked as a duplicate of this bug. ***

[Zoltan Horvath]

I rolled out in http://trac.webkit.org/changeset/94755.

[Ryosuke Niwa]

(In reply to comment #5)
> I rolled out in http://trac.webkit.org/changeset/94755.

Okay, thanks!

[Robert Hogan]

This ASSERT happens without r94755 - but the test I added, positioned-float-layout-after-image-load.html, triggers it. I agreed with smfr to reland r94755 without the test and the ASSERT can be worked separately here.

[Csaba Osztrogonác]

(In reply to comment #7)
> This ASSERT happens without r94755 - but the test I added, positioned-float-layout-after-image-load.html, triggers it. I agreed with smfr to reland r94755 without the test and the ASSERT can be worked separately here.

It's correct. +1 vote for relanding without the test.

[Robert Hogan]

(In reply to comment #0)
> 
> ASSERTION FAILED: isPlaced()
> /Volumes/Big/slave/snowleopard-intel-debug/build/Source/WebCore/rendering/RenderBlock.h(473) : WebCore::LayoutUnit WebCore::RenderBlock::FloatingObject::y() const
> 1   WebCore::RenderBlock::FloatingObject::y() const
> 2   WebCore::RenderBlock::logicalTopForFloat(WebCore::RenderBlock::FloatingObject const*) const
> 3   WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(WebCore::RenderBlock::FloatingObject*)

> 4   WebCore::RenderBlock::positionNewFloatOnLine(WebCore::RenderBlock::FloatingObject*, WebCore::RenderBlock::FloatingObject*, WebCore::LineWidth&)

This creates the floating object:

FloatingObject* newObj = new FloatingObject(o->style()->floating());

but it is not 'placed' with this constructor, it requires a frameRect for that.

It would get 'placed in positionNewFloats() but:

        // The containing block is responsible for positioning floats, so if we have floats in our
        // list that come from somewhere else, do not attempt to position them. Also don't attempt to handle
        // positioned floats, since the positioning layout code handles those.
        if (floatingObject->renderer()->containingBlock() != this || floatingObject->renderer()->isPositioned())
            continue;

prevents this. Because positionNewFloats() still returns true, it does not cause positionNewFloatOnLine() to return early and the assert is triggered.

The comment makes this look like the problem may be deeper than just returning early, so don't know if that's the right fix.

Created attachment 119710 [details]
Made line layout functions treat positioned floats like positioned objects

[Darin Adler]

Might even be worth a comment in those call sites saying that things both positioned and floating should be treated as positioned.

Fixed in <http://trac.webkit.org/r103206>.