Make LayoutState always be RAII

Source/core/rendering/RenderTable.cpp

Index: Source/core/rendering/RenderTable.cpp
diff --git a/Source/core/rendering/RenderTable.cpp b/Source/core/rendering/RenderTable.cpp
index 96744e3c41d120b5891f2df072dd4fa535c08e6d..ad63a1d4b9c76271d52ea5a02aa029d5c55be444 100644
--- a/Source/core/rendering/RenderTable.cpp
+++ b/Source/core/rendering/RenderTable.cpp
@@ -427,152 +427,150 @@ void RenderTable::layout()
     recalcBordersInRowDirection();
 
     LayoutRepainter repainter(*this, checkForRepaintDuringLayout());
-    LayoutStateMaintainer statePusher(*this, locationOffset());
-
-    setLogicalHeight(0);
-
-    LayoutUnit oldLogicalWidth = logicalWidth();
-    updateLogicalWidth();
-
     SubtreeLayoutScope layouter(this);
 
-    if (logicalWidth() != oldLogicalWidth) {
-        for (unsigned i = 0; i < m_captions.size(); i++)
-            layouter.setNeedsLayout(m_captions[i]);
-    }
-    // FIXME: The optimisation below doesn't work since the internal table
-    // layout could have changed.  we need to add a flag to the table
-    // layout that tells us if something has changed in the min max
-    // calculations to do it correctly.
-//     if ( oldWidth != width() || columns.size() + 1 != columnPos.size() )
-    m_tableLayout->layout();
-
-    LayoutUnit totalSectionLogicalHeight = 0;
-    LayoutUnit oldTableLogicalTop = 0;
-    for (unsigned i = 0; i < m_captions.size(); i++)
-        oldTableLogicalTop += m_captions[i]->logicalHeight() + m_captions[i]->marginBefore() + m_captions[i]->marginAfter();
-
-    bool collapsing = collapseBorders();
-
-    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
-        if (!child->needsLayout() && child->isBox())
-            toRenderBox(child)->markForPaginationRelayoutIfNeeded(layouter);
-        if (child->isTableSection()) {
-            RenderTableSection* section = toRenderTableSection(child);
-            if (m_columnLogicalWidthChanged)
-                layouter.setChildNeedsLayout(section);
-            section->layoutIfNeeded();
-            totalSectionLogicalHeight += section->calcRowLogicalHeight();
-            if (collapsing)
-                section->recalcOuterBorder();
-            ASSERT(!section->needsLayout());
-        } else if (child->isRenderTableCol()) {
-            child->layoutIfNeeded();
-            ASSERT(!child->needsLayout());
-        } else {
-            // FIXME: We should never have other type of children (they should be wrapped in an
-            // anonymous table section) but our code is too crazy and this can happen in practice.
-            // Until this is fixed, let's make sure we don't leave non laid out children in the tree.
-            child->layoutIfNeeded();
-        }
-    }
 
     // If any table section moved vertically, we will just repaint everything from that
     // section down (it is quite unlikely that any of the following sections
     // did not shift).
     bool sectionMoved = false;
     LayoutUnit movedSectionLogicalTop = 0;
+    {
+        LayoutStateMaintainer statePusher(*this, locationOffset());
 

[esprehn, 2014/03/20 01:10:55]

This scope is soooo long. This really needs to be made into smaller functions.

-    // FIXME: Collapse caption margin.
-    if (!m_captions.isEmpty()) {
-        for (unsigned i = 0; i < m_captions.size(); i++) {
-            if (m_captions[i]->style()->captionSide() == CAPBOTTOM)
-                continue;
-            layoutCaption(m_captions[i]);
-        }
-        if (logicalHeight() != oldTableLogicalTop) {
-            sectionMoved = true;
-            movedSectionLogicalTop = min(logicalHeight(), oldTableLogicalTop);
+        setLogicalHeight(0);
+
+        LayoutUnit oldLogicalWidth = logicalWidth();
+        updateLogicalWidth();
+
+        if (logicalWidth() != oldLogicalWidth) {
+            for (unsigned i = 0; i < m_captions.size(); i++)
+                layouter.setNeedsLayout(m_captions[i]);
         }
-    }
+        // FIXME: The optimisation below doesn't work since the internal table
+        // layout could have changed. We need to add a flag to the table
+        // layout that tells us if something has changed in the min max
+        // calculations to do it correctly.
+        // if ( oldWidth != width() || columns.size() + 1 != columnPos.size() )
+        m_tableLayout->layout();
+
+        LayoutUnit totalSectionLogicalHeight = 0;
+        LayoutUnit oldTableLogicalTop = 0;
+        for (unsigned i = 0; i < m_captions.size(); i++)
+            oldTableLogicalTop += m_captions[i]->logicalHeight() + m_captions[i]->marginBefore() + m_captions[i]->marginAfter();
 
-    LayoutUnit borderAndPaddingBefore = borderBefore() + (collapsing ? LayoutUnit() : paddingBefore());
-    LayoutUnit borderAndPaddingAfter = borderAfter() + (collapsing ? LayoutUnit() : paddingAfter());
+        bool collapsing = collapseBorders();
 
-    setLogicalHeight(logicalHeight() + borderAndPaddingBefore);
+        for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
+            if (child->isTableSection()) {
+                RenderTableSection* section = toRenderTableSection(child);
+                if (m_columnLogicalWidthChanged)
+                    layouter.setChildNeedsLayout(section);
+                section->layoutIfNeeded();
+                totalSectionLogicalHeight += section->calcRowLogicalHeight();
+                if (collapsing)
+                    section->recalcOuterBorder();
+                ASSERT(!section->needsLayout());
+            } else if (child->isRenderTableCol()) {
+                child->layoutIfNeeded();
+                ASSERT(!child->needsLayout());
+            } else {
+                // FIXME: We should never have other type of children (they should be wrapped in an
+                // anonymous table section) but our code is too crazy and this can happen in practice.
+                // Until this is fixed, let's make sure we don't leave non laid out children in the tree.
+                child->layoutIfNeeded();
+            }
+        }
 
-    if (!isOutOfFlowPositioned())
-        updateLogicalHeight();
+        // FIXME: Collapse caption margin.
+        if (!m_captions.isEmpty()) {
+            for (unsigned i = 0; i < m_captions.size(); i++) {
+                if (m_captions[i]->style()->captionSide() == CAPBOTTOM)
+                    continue;
+                layoutCaption(m_captions[i]);
+            }
+            if (logicalHeight() != oldTableLogicalTop) {
+                sectionMoved = true;
+                movedSectionLogicalTop = min(logicalHeight(), oldTableLogicalTop);
+            }
+        }
 
-    LayoutUnit computedLogicalHeight = 0;
+        LayoutUnit borderAndPaddingBefore = borderBefore() + (collapsing ? LayoutUnit() : paddingBefore());
+        LayoutUnit borderAndPaddingAfter = borderAfter() + (collapsing ? LayoutUnit() : paddingAfter());
 
-    Length logicalHeightLength = style()->logicalHeight();
-    if (logicalHeightLength.isIntrinsic() || (logicalHeightLength.isSpecified() && logicalHeightLength.isPositive()))
-        computedLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
+        setLogicalHeight(logicalHeight() + borderAndPaddingBefore);
 
-    Length logicalMaxHeightLength = style()->logicalMaxHeight();
-    if (logicalMaxHeightLength.isIntrinsic() || (logicalMaxHeightLength.isSpecified() && !logicalMaxHeightLength.isNegative())) {
-        LayoutUnit computedMaxLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMaxHeightLength);
-        computedLogicalHeight = min(computedLogicalHeight, computedMaxLogicalHeight);
-    }
+        if (!isOutOfFlowPositioned())
+            updateLogicalHeight();
 
-    Length logicalMinHeightLength = style()->logicalMinHeight();
-    if (logicalMinHeightLength.isIntrinsic() || (logicalMinHeightLength.isSpecified() && !logicalMinHeightLength.isNegative())) {
-        LayoutUnit computedMinLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMinHeightLength);
-        computedLogicalHeight = max(computedLogicalHeight, computedMinLogicalHeight);
-    }
+        LayoutUnit computedLogicalHeight = 0;
 
-    distributeExtraLogicalHeight(floorToInt(computedLogicalHeight - totalSectionLogicalHeight));
+        Length logicalHeightLength = style()->logicalHeight();
+        if (logicalHeightLength.isIntrinsic() || (logicalHeightLength.isSpecified() && logicalHeightLength.isPositive()))
+            computedLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
 
-    for (RenderTableSection* section = topSection(); section; section = sectionBelow(section))
-        section->layoutRows();
+        Length logicalMaxHeightLength = style()->logicalMaxHeight();
+        if (logicalMaxHeightLength.isIntrinsic() || (logicalMaxHeightLength.isSpecified() && !logicalMaxHeightLength.isNegative())) {
+            LayoutUnit computedMaxLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMaxHeightLength);
+            computedLogicalHeight = min(computedLogicalHeight, computedMaxLogicalHeight);
+        }
 
-    if (!topSection() && computedLogicalHeight > totalSectionLogicalHeight && !document().inQuirksMode()) {
-        // Completely empty tables (with no sections or anything) should at least honor specified height
-        // in strict mode.
-        setLogicalHeight(logicalHeight() + computedLogicalHeight);
-    }
+        Length logicalMinHeightLength = style()->logicalMinHeight();
+        if (logicalMinHeightLength.isIntrinsic() || (logicalMinHeightLength.isSpecified() && !logicalMinHeightLength.isNegative())) {
+            LayoutUnit computedMinLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMinHeightLength);
+            computedLogicalHeight = max(computedLogicalHeight, computedMinLogicalHeight);
+        }
 
-    LayoutUnit sectionLogicalLeft = style()->isLeftToRightDirection() ? borderStart() : borderEnd();
-    if (!collapsing)
-        sectionLogicalLeft += style()->isLeftToRightDirection() ? paddingStart() : paddingEnd();
+        distributeExtraLogicalHeight(floorToInt(computedLogicalHeight - totalSectionLogicalHeight));
 
-    // position the table sections
-    RenderTableSection* section = topSection();
-    while (section) {
-        if (!sectionMoved && section->logicalTop() != logicalHeight()) {
-            sectionMoved = true;
-            movedSectionLogicalTop = min(logicalHeight(), section->logicalTop()) + (style()->isHorizontalWritingMode() ? section->visualOverflowRect().y() : section->visualOverflowRect().x());
+        for (RenderTableSection* section = topSection(); section; section = sectionBelow(section))
+            section->layoutRows();
+
+        if (!topSection() && computedLogicalHeight > totalSectionLogicalHeight && !document().inQuirksMode()) {
+            // Completely empty tables (with no sections or anything) should at least honor specified height
+            // in strict mode.
+            setLogicalHeight(logicalHeight() + computedLogicalHeight);
         }
-        section->setLogicalLocation(LayoutPoint(sectionLogicalLeft, logicalHeight()));
 
-        setLogicalHeight(logicalHeight() + section->logicalHeight());
-        section = sectionBelow(section);
-    }
+        LayoutUnit sectionLogicalLeft = style()->isLeftToRightDirection() ? borderStart() : borderEnd();
+        if (!collapsing)
+            sectionLogicalLeft += style()->isLeftToRightDirection() ? paddingStart() : paddingEnd();
 
-    setLogicalHeight(logicalHeight() + borderAndPaddingAfter);
+        // position the table sections
+        RenderTableSection* section = topSection();
+        while (section) {
+            if (!sectionMoved && section->logicalTop() != logicalHeight()) {
+                sectionMoved = true;
+                movedSectionLogicalTop = min(logicalHeight(), section->logicalTop()) + (style()->isHorizontalWritingMode() ? section->visualOverflowRect().y() : section->visualOverflowRect().x());
+            }
+            section->setLogicalLocation(LayoutPoint(sectionLogicalLeft, logicalHeight()));
 
-    for (unsigned i = 0; i < m_captions.size(); i++) {
-        if (m_captions[i]->style()->captionSide() != CAPBOTTOM)
-            continue;
-        layoutCaption(m_captions[i]);
-    }
+            setLogicalHeight(logicalHeight() + section->logicalHeight());
+            section = sectionBelow(section);
+        }
 
-    if (isOutOfFlowPositioned())
-        updateLogicalHeight();
+        setLogicalHeight(logicalHeight() + borderAndPaddingAfter);
 
-    // table can be containing block of positioned elements.
-    // FIXME: Only pass true if width or height changed.
-    layoutPositionedObjects(true);
+        for (unsigned i = 0; i < m_captions.size(); i++) {
+            if (m_captions[i]->style()->captionSide() != CAPBOTTOM)
+                continue;
+            layoutCaption(m_captions[i]);
+        }
 
-    updateLayerTransform();
+        if (isOutOfFlowPositioned())
+            updateLogicalHeight();
 
-    // Layout was changed, so probably borders too.
-    invalidateCollapsedBorders();
+        // table can be containing block of positioned elements.
+        // FIXME: Only pass true if width or height changed.
+        layoutPositionedObjects(true);
 
-    computeOverflow(clientLogicalBottom());
+        updateLayerTransform();
 
-    statePusher.pop();
+        // Layout was changed, so probably borders too.
+        invalidateCollapsedBorders();
+
+        computeOverflow(clientLogicalBottom());
+    }
 
     if (view()->layoutState()->pageLogicalHeight())
         setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(*this, logicalTop()));