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 589ea2d62281fe3dbfd0edd12dbe023fa1c1056d..822c579a6e319dbd45a56c8774eff1774c90bc7d 100644
--- a/Source/core/rendering/RenderTable.cpp
+++ b/Source/core/rendering/RenderTable.cpp
@@ -409,24 +409,8 @@ void RenderTable::simplifiedNormalFlowLayout()
     }
 }
 
-void RenderTable::layout()
+void RenderTable::layoutContents(SubtreeLayoutScope& layouter, bool& sectionMoved, LayoutUnit& movedSectionLogicalTop)
 {
-    // Note: RenderTable is handled differently than other RenderBlocks and the LayoutScope
-    //       must be created before the table begins laying out.
-    FastTextAutosizer::LayoutScope fastTextAutosizerLayoutScope(this);
-    ASSERT(needsLayout());
-
-    LayoutRectRecorder recorder(*this);
-
-    if (simplifiedLayout())
-        return;
-
-    recalcSectionsIfNeeded();
-    // FIXME: We should do this recalc lazily in borderStart/borderEnd so that we don't have to make sure
-    // to call this before we call borderStart/borderEnd to avoid getting a stale value.
-    recalcBordersInRowDirection();
-
-    LayoutRepainter repainter(*this, checkForRepaintDuringLayout());
     LayoutStateMaintainer statePusher(*this, locationOffset());
 
     setLogicalHeight(0);
@@ -434,8 +418,6 @@ void RenderTable::layout()
     LayoutUnit oldLogicalWidth = logicalWidth();
     updateLogicalWidth();
 
-    SubtreeLayoutScope layouter(this);
-
     if (logicalWidth() != oldLogicalWidth) {
         for (unsigned i = 0; i < m_captions.size(); i++)
             layouter.setNeedsLayout(m_captions[i]);
@@ -475,12 +457,6 @@ void RenderTable::layout()
         }
     }
 
-    // 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;
-
     // FIXME: Collapse caption margin.
     if (!m_captions.isEmpty()) {
         for (unsigned i = 0; i < m_captions.size(); i++) {
@@ -569,8 +545,35 @@ void RenderTable::layout()
     invalidateCollapsedBorders();
 
     computeOverflow(clientLogicalBottom());
+}
+
+void RenderTable::layout()
+{
+    // Note: RenderTable is handled differently than other RenderBlocks and the LayoutScope
+    //       must be created before the table begins laying out.
+    FastTextAutosizer::LayoutScope fastTextAutosizerLayoutScope(this);
+    ASSERT(needsLayout());
+
+    LayoutRectRecorder recorder(*this);
+
+    if (simplifiedLayout())
+        return;
+
+    recalcSectionsIfNeeded();
+    // FIXME: We should do this recalc lazily in borderStart/borderEnd so that we don't have to make sure
+    // to call this before we call borderStart/borderEnd to avoid getting a stale value.
+    recalcBordersInRowDirection();
+
+    LayoutRepainter repainter(*this, checkForRepaintDuringLayout());
+    SubtreeLayoutScope layouter(this);
+
+    // 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;

[Julien - ping for review, 2014/03/13 17:44:27]

Really not a huge fan of these variables (along with the side effect logic in
layoutContents).

It's a pity we can't just move the repainting logic inside layoutContent as it
would this awkwardness.

 
-    statePusher.pop();
+    layoutContents(layouter, sectionMoved, movedSectionLogicalTop);
 
     if (view()->layoutState()->pageLogicalHeight())
         setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(*this, logicalTop()));