Changeset 38227 in webkit

Timestamp:

Nov 7, 2008 11:17:08 AM (15 years ago)

Author:

Simon Fraser

Message:

2008-11-07 Simon Fraser <Simon Fraser>

Reviewed by Dan Bernstein

​https://bugs.webkit.org/show_bug.cgi?id=22122

Use a stack-based object to simplify the pushLayoutState/popLayoutState
code. LayoutStateMaintainer either pushes in the constructor, or allows
an explicit push() later. Both cases require an explicit pop().

• rendering/RenderBlock.cpp: (WebCore::RenderBlock::layoutBlock): (WebCore::RenderBlock::layoutOnlyPositionedObjects):
• rendering/RenderContainer.cpp: (WebCore::RenderContainer::layout):
• rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::layoutBlock):
• rendering/RenderTable.cpp: (WebCore::RenderTable::layout):
• rendering/RenderTableRow.cpp: (WebCore::RenderTableRow::layout):
• rendering/RenderTableSection.cpp: (WebCore::RenderTableSection::setCellWidths): (WebCore::RenderTableSection::calcRowHeight): (WebCore::RenderTableSection::layoutRows):
• rendering/RenderView.h: (WebCore::LayoutStateMaintainer::LayoutStateMaintainer): (WebCore::LayoutStateMaintainer::~LayoutStateMaintainer): (WebCore::LayoutStateMaintainer::pop): (WebCore::LayoutStateMaintainer::push): (WebCore::LayoutStateMaintainer::didPush):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderBlock.cpp (modified) (4 diffs)
• rendering/RenderContainer.cpp (modified) (2 diffs)
• rendering/RenderFlexibleBox.cpp (modified) (2 diffs)
• rendering/RenderTable.cpp (modified) (2 diffs)
• rendering/RenderTableRow.cpp (modified) (2 diffs)
• rendering/RenderTableSection.cpp (modified) (8 diffs)
• rendering/RenderView.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-11-07  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dan Bernstein
+
+        https://bugs.webkit.org/show_bug.cgi?id=22122
+        
+        Use a stack-based object to simplify the pushLayoutState/popLayoutState
+        code. LayoutStateMaintainer either pushes in the constructor, or allows
+        an explicit push() later. Both cases require an explicit pop().
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::layoutBlock):
+        (WebCore::RenderBlock::layoutOnlyPositionedObjects):
+        * rendering/RenderContainer.cpp:
+        (WebCore::RenderContainer::layout):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutBlock):
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::layout):
+        * rendering/RenderTableRow.cpp:
+        (WebCore::RenderTableRow::layout):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::setCellWidths):
+        (WebCore::RenderTableSection::calcRowHeight):
+        (WebCore::RenderTableSection::layoutRows):
+        * rendering/RenderView.h:
+        (WebCore::LayoutStateMaintainer::LayoutStateMaintainer):
+        (WebCore::LayoutStateMaintainer::~LayoutStateMaintainer):
+        (WebCore::LayoutStateMaintainer::pop):
+        (WebCore::LayoutStateMaintainer::push):
+        (WebCore::LayoutStateMaintainer::didPush):
+
 2008-11-07  Tor Arne Vestbø  <tavestbo@trolltech.com>
 

trunk/WebCore/rendering/RenderBlock.cpp

@@ -593 +593 @@
     }
 
-    bool hadColumns = m_hasColumns;
-    if (!hadColumns && !hasReflection())
-        view()->pushLayoutState(this, IntSize(xPos(), yPos()));
-    else
-        view()->disableLayoutState();
+    LayoutStateMaintainer statePusher(view(), this, IntSize(xPos(), yPos()), !m_hasColumns && !hasReflection());
 
     int oldWidth = m_width;
@@ -726 +722 @@
     }
 
-    if (!hadColumns && !hasReflection())
-        view()->popLayoutState();
-    else
-        view()->enableLayoutState();
+    statePusher.pop();
 
     // Update our scroll information if we're overflow:auto/scroll/hidden now that we know if
@@ -1395 +1388 @@
         return false;
 
-    if (!m_hasColumns)
-        view()->pushLayoutState(this, IntSize(xPos(), yPos()));
-    else
-        view()->disableLayoutState();
+    LayoutStateMaintainer statePusher(view(), this, IntSize(xPos(), yPos()), !m_hasColumns);
 
     if (needsPositionedMovementLayout()) {
@@ -1409 +1399 @@
     layoutPositionedObjects(false);
 
-    if (!m_hasColumns)
-        view()->popLayoutState();
-    else
-        view()->enableLayoutState();
+    statePusher.pop();
 
     if (hasOverflowClip())

trunk/WebCore/rendering/RenderContainer.cpp

@@ -521 +521 @@
     ASSERT(needsLayout());
 
-    view()->pushLayoutState(this, IntSize(m_x, m_y));
+    LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y));
 
     RenderObject* child = m_firstChild;
@@ -530 +530 @@
     }
 
-    view()->popLayoutState();
+    statePusher.pop();
     setNeedsLayout(false);
 }

trunk/WebCore/rendering/RenderFlexibleBox.cpp

@@ -220 +220 @@
     }
 
-    if (!hasReflection())
-        view()->pushLayoutState(this, IntSize(m_x, m_y));
-    else
-        view()->disableLayoutState();
+    LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y), !hasReflection());
 
     int previousWidth = m_width;
@@ -309 +306 @@
     }
 
-    if (!hasReflection())
-        view()->popLayoutState();
-    else
-        view()->enableLayoutState();
+    statePusher.pop();
 
     // Update our scrollbars if we're overflow:auto/scroll/hidden now that we know if

trunk/WebCore/rendering/RenderTable.cpp

@@ -266 +266 @@
     }
     
-    view()->pushLayoutState(this, IntSize(m_x, m_y));
+    LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y));
 
     m_height = 0;
@@ -426 +426 @@
     }
 
-    view()->popLayoutState();
+    statePusher.pop();
 
     bool didFullRepaint = true;

trunk/WebCore/rendering/RenderTableRow.cpp

@@ -125 +125 @@
 
     // Table rows do not add translation.
-    view()->pushLayoutState(this, IntSize());
+    LayoutStateMaintainer statePusher(view(), this, IntSize());
 
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
@@ -149 +149 @@
     }
 
-    view()->popLayoutState();
+    statePusher.pop();
     setNeedsLayout(false);
 }

trunk/WebCore/rendering/RenderTableSection.cpp

@@ -258 +258 @@
 {
     Vector<int>& columnPos = table()->columnPositions();
-    bool pushedLayoutState = false;
-
+
+    LayoutStateMaintainer statePusher(view());
+    
     for (int i = 0; i < m_gridRows; i++) {
         Row& row = *m_grid[i].row;
@@ -280 +281 @@
                 cell->setNeedsLayout(true);
                 if (!table()->selfNeedsLayout() && cell->checkForRepaintDuringLayout()) {
-                    if (!pushedLayoutState) {
+                    if (!statePusher.didPush()) {
                         // Technically, we should also push state for the row, but since
                         // rows don't push a coordinate transform, that's not necessary.
-                        view()->pushLayoutState(this, IntSize(m_x, m_y));
-                        pushedLayoutState = true;
+                        statePusher.push(this, IntSize(m_x, m_y));
                     }
                     cell->repaint();
@@ -293 +293 @@
     }
     
-    if (pushedLayoutState)
-        view()->popLayoutState();
+    statePusher.pop();  // only pops if we pushed
 }
 
@@ -302 +301 @@
 
     int spacing = table()->vBorderSpacing();
-    bool pushedLayoutState = false;
+
+    LayoutStateMaintainer statePusher(view());
 
     m_rowPos.resize(m_gridRows + 1);
@@ -331 +331 @@
 
             if (cell->overrideSize() != -1) {
-                if (!pushedLayoutState) {
+                if (!statePusher.didPush()) {
                     // Technically, we should also push state for the row, but since
                     // rows don't push a coordinate transform, that's not necessary.
-                    view()->pushLayoutState(this, IntSize(m_x, m_y));
-                    pushedLayoutState = true;
+                    statePusher.push(this, IntSize(m_x, m_y));
                 }
                 cell->setOverrideSize(-1);
@@ -374 +373 @@
     }
 
-    if (pushedLayoutState)
-        view()->popLayoutState();
+    statePusher.pop();
 
     return m_rowPos[m_gridRows];
@@ -457 +455 @@
     int nEffCols = table()->numEffCols();
 
-    view()->pushLayoutState(this, IntSize(m_x, m_y));
+    LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y));
 
     for (int r = 0; r < totalRows; r++) {
@@ -576 +574 @@
     }
 
-    view()->popLayoutState();
+    statePusher.pop();
 
     m_height = m_rowPos[totalRows];

trunk/WebCore/rendering/RenderView.h

@@ -158 +158 @@
 };
 
+// Stack-based class to assist with LayoutState push/pop
+class LayoutStateMaintainer : Noncopyable {
+public:
+    // ctor to push now
+    LayoutStateMaintainer(RenderView* view, RenderBox* root, IntSize offset, bool shouldPush = true)
+        : m_view(view)
+        , m_shouldPushPop(shouldPush)
+        , m_didStart(false)
+        , m_didEnd(false)
+    {
+        push(root, offset);
+    }
+    
+    // ctor to maybe push later
+    LayoutStateMaintainer(RenderView* view)
+        : m_view(view)
+        , m_shouldPushPop(true)
+        , m_didStart(false)
+        , m_didEnd(false)
+    {
+    }
+    
+    ~LayoutStateMaintainer()
+    {
+        ASSERT(m_didStart == m_didEnd);   // if this fires, it means that someone did a push(), but forgot to pop().
+    }
+
+    void pop()
+    {
+        if (m_didStart) {
+            ASSERT(!m_didEnd);
+            if (m_shouldPushPop)
+                m_view->popLayoutState();
+            else
+                m_view->enableLayoutState();
+            m_didEnd = true;
+        }
+    }
+
+    void push(RenderBox* root, IntSize offset)
+    {
+        ASSERT(!m_didStart);
+        if (m_shouldPushPop)
+            m_view->pushLayoutState(root, offset);
+        else
+            m_view->disableLayoutState();
+        m_didStart = true;
+    }
+    
+    bool didPush() const { return m_didStart; }
+
+private:
+    RenderView* m_view;
+    bool m_shouldPushPop : 1;   // true if we should push/pop, rather than disable/enable
+    bool m_didStart : 1;        // true if we did a push or disable
+    bool m_didEnd : 1;          // true if we popped or re-enabled
+};
+
 } // namespace WebCore