Bug 76405 – Hit ASSERTION FAILED: table()->collapseBorders() on techcrunch.com

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

[Simon Fraser (smfr)]

ASSERTION FAILED: table()->collapseBorders()
/Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderTableSection.cpp(1363) : void WebCore::RenderTableSection::setCachedCollapsedBorder(const WebCore::RenderTableCell *, WebCore::CollapsedBorderSide, WebCore::CollapsedBorderValue)
1   0x104e05461 WebCore::RenderTableSection::setCachedCollapsedBorder(WebCore::RenderTableCell const*, WebCore::CollapsedBorderSide, WebCore::CollapsedBorderValue)
2   0x104df6837 WebCore::RenderTableCell::collapsedStartBorder(WebCore::IncludeBorderColorOrNot) const
3   0x104df9a54 WebCore::RenderTableCell::collectBorderValues(WTF::Vector<WebCore::CollapsedBorderValue, 0ul>&) const
4   0x104dee03f WebCore::RenderTable::recalcCollapsedBorders()
5   0x104dee75b WebCore::RenderTable::paintObject(WebCore::PaintInfo&, WebCore::IntPoint const&)
6   0x104dee48d WebCore::RenderTable::paint(WebCore::PaintInfo&, WebCore::IntPoint const&)
7   0x104c1e3bf WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::IntPoint const&)
8   0x104c1e006 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::IntPoint const&)
9   0x104c1ea05 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint const&)
10  0x104c1c81d WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::IntPoint const&)
11  0x104c1e3bf WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::IntPoint const&)
12  0x104c1e006 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::IntPoint const&)
13  0x104c1ea05 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint const&)
14  0x104c1c81d WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::IntPoint const&)
15  0x104c1e3bf WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::IntPoint const&)
16  0x104c1e006 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::IntPoint const&)
17  0x104c1ea05 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint const&)
18  0x104c1c81d WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::IntPoint const&)
19  0x104c1e3bf WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::IntPoint const&)
20  0x104c1e006 WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::IntPoint const&)
21  0x104c1ea05 WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::IntPoint const&)
22  0x104c1c81d WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::IntPoint const&)
23  0x104cff2ae WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
24  0x104cfe9c6 WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
25  0x104cfe0f5 WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
26  0x104d0034f WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
27  0x104cff591 WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
28  0x104cfe9c6 WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
29  0x104cfe0f5 WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
30  0x104cfda4c WebCore::RenderLayer::paint(WebCore::GraphicsContext*, WebCore::IntRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, unsigned int)
31  0x1041df339 WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&)

[Andras Becsi]

The assertion is also reached when loading the following sites:

http://www.aftenposten.no/nyheter/uriks/usavalg/Ber-velgerne-stemme-pa-kandidat-som-trakk-seg-6743588.html#.TxW3rr41i2M

http://www.guardian.co.uk/books/booksblog/2012/jan/17/stephen-king-face-cover

This is reproducible with the Qt MiniBrowser and only happens when fixed layout is used (touch mode).

[Robert Hogan]

The ASSERT has exposed a situation where collapsed borders are being calculated for a cell that is not in a table with collapsed borders! The problem seems to be with:

void RenderTable::recalcCollapsedBorders()
{
    if (m_collapsedBordersValid)
        return;
    m_collapsedBordersValid = true;
    m_collapsedBorders.clear();
    RenderObject* stop = nextInPreOrderAfterChildren();
    for (RenderObject* o = firstChild(); o && o != stop; o = o->nextInPreOrder()) {
        if (o->isTableCell())
            toRenderTableCell(o)->collectBorderValues(m_collapsedBorders);
    }
    RenderTableCell::sortBorderValues(m_collapsedBorders);
}

It is collecting border values for cells that are not in the RenderTable - I suspect a nested table.

[Julien Chaffraix]

(In reply to comment #2)
> The ASSERT has exposed a situation where collapsed borders are being calculated for a cell that is not in a table with collapsed borders! The problem seems to be with:
> 
> void RenderTable::recalcCollapsedBorders()
> {
>     if (m_collapsedBordersValid)
>         return;
>     m_collapsedBordersValid = true;
>     m_collapsedBorders.clear();
>     RenderObject* stop = nextInPreOrderAfterChildren();
>     for (RenderObject* o = firstChild(); o && o != stop; o = o->nextInPreOrder()) {
>         if (o->isTableCell())
>             toRenderTableCell(o)->collectBorderValues(m_collapsedBorders);
>     }
>     RenderTableCell::sortBorderValues(m_collapsedBorders);
> }

Just a side note, this code is really not optimized as it does not use the table's render tree structure (RenderTableSection -> RenderTableRow -> RenderTableCell). The side effect that you pointed out is that it will happily recurse into a nested table.

I suggested on IRC to fix this loop either by detecting and skipping nested tables or by not recursing into cells. Actually making it use the rendering structure would achieve the same result and clean up the code so that would be my choice.

[Jon Lee]

gmail.com also.

[Robert Hogan]

Created attachment 122982 [details]
Patch

[Julien Chaffraix]

Comment on attachment 122982 [details]
Patch

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

> Source/WebCore/rendering/RenderTable.cpp:451
> +    for (RenderObject* o = firstChild(); o; o = o->nextSibling()) {
> +        if (!o->isTableSection())

|section| would be a better name for |o|.

> Source/WebCore/rendering/RenderTable.cpp:459
> +                toRenderTableCell(cell)->collectBorderValues(m_collapsedBorders);

Would be nice to add an ASSERT here that cell->table() == this (we never want to collect borders from any other table).

[Robert Hogan]

Committed r105433: <http://trac.webkit.org/changeset/105433>

[Csaba Osztrogonác]

Comment on attachment 122982 [details]
Patch

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

Could you fix the broken builds?

>> Source/WebCore/rendering/RenderTable.cpp:459
>> +                toRenderTableCell(cell)->collectBorderValues(m_collapsedBorders);
> 
> Would be nice to add an ASSERT here that cell->table() == this (we never want to collect borders from any other table).

This assert broke debug builds:

../../../../Source/WebCore/rendering/RenderTable.cpp: In member function ‘void WebCore::RenderTable::recalcCollapsedBorders()’:
../../../../Source/WebCore/rendering/RenderTable.cpp:459: error: ‘class WebCore::RenderObject’ has no member named ‘table’

[Csaba Osztrogonác]

Reopen, because it broke debug builds.

[Csaba Osztrogonác]

Fixed by http://trac.webkit.org/changeset/105438