Make LayoutState always be RAII

Source/core/rendering/RenderTable.cpp

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 391 matching lines @@
 }
 
 void RenderTable::simplifiedNormalFlowLayout()
 {
     for (RenderTableSection* section = topSection(); section; section = sectionBelow(section)) {
         section->layoutIfNeeded();
         section->computeOverflowFromCells();
     }
 }
 
-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);
 
     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();
@@ skipping 19 matching lines @@
             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;
-
     // 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);
@@ skipping 68 matching lines @@
     // table can be containing block of positioned elements.
     // FIXME: Only pass true if width or height changed.
     layoutPositionedObjects(true);
 
     updateLayerTransform();
 
     // Layout was changed, so probably borders too.
     invalidateCollapsedBorders();
 
     computeOverflow(clientLogicalBottom());
+}
 
-    statePusher.pop();
+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.

+
+    layoutContents(layouter, sectionMoved, movedSectionLogicalTop);
 
     if (view()->layoutState()->pageLogicalHeight())
         setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(*this, logicalTop()));
 
     bool didFullRepaint = repainter.repaintAfterLayout();
     // Repaint with our new bounds if they are different from our old bounds.
     if (!RuntimeEnabledFeatures::repaintAfterLayoutEnabled()
         && !didFullRepaint && sectionMoved) {
         if (style()->isHorizontalWritingMode())
             repaintRectangle(LayoutRect(visualOverflowRect().x(), movedSectionLogicalTop, visualOverflowRect().width(), visualOverflowRect().maxY() - movedSectionLogicalTop));
@@ skipping 871 matching lines @@
 const BorderValue& RenderTable::tableEndBorderAdjoiningCell(const RenderTableCell* cell) const
 {
     ASSERT(cell->isFirstOrLastCellInRow());
     if (hasSameDirectionAs(cell->row()))
         return style()->borderEnd();
 
     return style()->borderStart();
 }
 
 }