Bug 60537

Summary: containsFloats check missing from hasOverhangingFloats, causes null crash
Product: WebKit Reporter: Abhishek Arya <inferno>
Component: Layout and RenderingAssignee: Abhishek Arya <inferno>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, hyatt, jamesr, mitz, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch simon.fraser: review+

Description Abhishek Arya 2011-05-10 01:07:26 PDT
hasOverhangingFloats can produce true when we don't any floats at all. This happens because we have check like
bool hasOverhangingFloats() { return parent() && !hasColumns() && lowestFloatLogicalBottom() > logicalHeight(); }

lowestFloatLogicalBottom will return 0- when we dont have any floating objects. however our logicalheight can be less than zero when we have large bottom padding. so this check will return true incorrectly. patch upcoming.
Comment 1 Abhishek Arya 2011-05-10 01:20:24 PDT
I have repro, so we might not need this hack anymore.

void RenderBlock::repaintOverhangingFloats(bool paintAllDescendants)
{
    // Repaint any overhanging floats (if we know we're the one to paint them).
    if (hasOverhangingFloats()) {
        // We think that we must be in a bad state if m_floatingObjects is nil at this point, so
        // we assert on Debug builds and nil-check Release builds.
        ASSERT(m_floatingObjects);
        if (!m_floatingObjects)
            return;
Comment 2 Abhishek Arya 2011-05-10 01:30:30 PDT
stack::
#0  0x0000000001b40af1 in WebCore::RenderBlock::markSiblingsWithFloatsForLayout() ()
#1  0x0000000001b474ca in WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) ()
#2  0x0000000001bc8461 in WebCore::RenderObject::setStyle(WTF::PassRefPtr<WebCore::RenderStyle>) ()
#3  0x0000000001bc69af in WebCore::RenderObject::setAnimatableStyle(WTF::PassRefPtr<WebCore::RenderStyle>) ()
#4  0x000000000186ec86 in WebCore::Node::setRenderStyle(WTF::PassRefPtr<WebCore::RenderStyle>) ()
#5  0x000000000185b232 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
#6  0x000000000185b320 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
#7  0x000000000149ad69 in WebCore::HTMLFormControlElement::recalcStyle(WebCore::Node::StyleChange) ()
#8  0x000000000185b320 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
#9  0x000000000185b320 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
#10 0x000000000185b320 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
#11 0x000000000185b320 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) ()
Comment 3 Abhishek Arya 2011-05-10 07:52:46 PDT
Created attachment 92954 [details]
Patch
Comment 4 Abhishek Arya 2011-05-10 07:55:17 PDT
The crash was happening because m_floatingObjects was nil in markSiblingsWithFloatsForLayout and we returned true in overhangingfloats. Started after my http://trac.webkit.org/changeset/85876.

if (diff == StyleDifferenceLayout && s_canPropagateFloatIntoSibling && !canPropagateFloatIntoSibling && hasOverhangingFloats()) { 
 	268	        markAllDescendantsWithFloatsForLayout(); 
 	269	        markSiblingsWithFloatsForLayout();
Comment 5 Abhishek Arya 2011-05-10 07:55:57 PDT
Dan, can you please review.
Comment 6 Simon Fraser (smfr) 2011-05-10 09:24:56 PDT
<rdar://problem/9413395>
Comment 7 Abhishek Arya 2011-05-10 09:40:46 PDT
Committed r86160: <http://trac.webkit.org/changeset/86160>
Comment 8 WebKit Review Bot 2011-05-10 10:53:43 PDT
http://trac.webkit.org/changeset/86160 might have broken SnowLeopard Intel Release (WebKit2 Tests)
The following tests are not passing:
fast/frames/flattening/frameset-flattening-subframesets.html