diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 51aa18e82909a1bf31d1ebb85eb566d19949d633..627f22aad093f877bb897f8b919657fb461b1e2d 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,35 @@ +2008-11-07 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): + 2008-11-07 Tor Arne Vestbø Fix the QtWebKit build on Mac diff --git a/WebCore/rendering/RenderBlock.cpp b/WebCore/rendering/RenderBlock.cpp index e46638635ffc193f2d350f296ffdfb91a6b41ded..aee698c74c3bdc0389bb7dfdcd45cc027a6d8c05 100644 --- a/WebCore/rendering/RenderBlock.cpp +++ b/WebCore/rendering/RenderBlock.cpp @@ -592,11 +592,7 @@ void RenderBlock::layoutBlock(bool relayoutChildren) oldOutlineBox = absoluteOutlineBox(); } - 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; int oldColumnWidth = desiredColumnWidth(); @@ -725,10 +721,7 @@ void RenderBlock::layoutBlock(bool relayoutChildren) } } - 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 // we overflow or not. @@ -1394,10 +1387,7 @@ bool RenderBlock::layoutOnlyPositionedObjects() if (!posChildNeedsLayout() || normalChildNeedsLayout() || selfNeedsLayout()) 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()) { tryLayoutDoingPositionedMovementOnly(); @@ -1408,10 +1398,7 @@ bool RenderBlock::layoutOnlyPositionedObjects() // All we have to is lay out our positioned objects. layoutPositionedObjects(false); - if (!m_hasColumns) - view()->popLayoutState(); - else - view()->enableLayoutState(); + statePusher.pop(); if (hasOverflowClip()) m_layer->updateScrollInfoAfterLayout(); diff --git a/WebCore/rendering/RenderContainer.cpp b/WebCore/rendering/RenderContainer.cpp index 982560130154c33f9ecb9d6f354de043dac3c9f3..8c3bbca6af53272633c5481a8194fc80b5643c3c 100644 --- a/WebCore/rendering/RenderContainer.cpp +++ b/WebCore/rendering/RenderContainer.cpp @@ -520,7 +520,7 @@ void RenderContainer::layout() { ASSERT(needsLayout()); - view()->pushLayoutState(this, IntSize(m_x, m_y)); + LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y)); RenderObject* child = m_firstChild; while (child) { @@ -529,7 +529,7 @@ void RenderContainer::layout() child = child->nextSibling(); } - view()->popLayoutState(); + statePusher.pop(); setNeedsLayout(false); } diff --git a/WebCore/rendering/RenderFlexibleBox.cpp b/WebCore/rendering/RenderFlexibleBox.cpp index aff7f9ba92edf461fdc5e91c9fa06fb037fcabf5..edd1e046f3ee4ec1df3b4d706fdaa7a6ef7fd56e 100644 --- a/WebCore/rendering/RenderFlexibleBox.cpp +++ b/WebCore/rendering/RenderFlexibleBox.cpp @@ -219,10 +219,7 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren) oldOutlineBox = absoluteOutlineBox(); } - 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; int previousHeight = m_height; @@ -308,10 +305,7 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren) } } - if (!hasReflection()) - view()->popLayoutState(); - else - view()->enableLayoutState(); + statePusher.pop(); // Update our scrollbars if we're overflow:auto/scroll/hidden now that we know if // we overflow or not. diff --git a/WebCore/rendering/RenderTable.cpp b/WebCore/rendering/RenderTable.cpp index 01e420e25429e8c9b911d04f4ef649aa307a3a0e..f1508e45e77aa41ac15e3ff8f9c4c4ef1ed8bb6c 100644 --- a/WebCore/rendering/RenderTable.cpp +++ b/WebCore/rendering/RenderTable.cpp @@ -265,7 +265,7 @@ void RenderTable::layout() oldOutlineBox = absoluteOutlineBox(); } - view()->pushLayoutState(this, IntSize(m_x, m_y)); + LayoutStateMaintainer statePusher(view(), this, IntSize(m_x, m_y)); m_height = 0; m_overflowHeight = 0; @@ -425,7 +425,7 @@ void RenderTable::layout() } } - view()->popLayoutState(); + statePusher.pop(); bool didFullRepaint = true; // Repaint with our new bounds if they are different from our old bounds. diff --git a/WebCore/rendering/RenderTableRow.cpp b/WebCore/rendering/RenderTableRow.cpp index 4e9323d7b2dd8a7e6fdbf30e8135b9b285ae1b6a..7fe88eb224c21d2258a935a7247e35e024674ab4 100644 --- a/WebCore/rendering/RenderTableRow.cpp +++ b/WebCore/rendering/RenderTableRow.cpp @@ -124,7 +124,7 @@ void RenderTableRow::layout() ASSERT(needsLayout()); // Table rows do not add translation. - view()->pushLayoutState(this, IntSize()); + LayoutStateMaintainer statePusher(view(), this, IntSize()); for (RenderObject* child = firstChild(); child; child = child->nextSibling()) { if (child->isTableCell()) { @@ -148,7 +148,7 @@ void RenderTableRow::layout() } } - view()->popLayoutState(); + statePusher.pop(); setNeedsLayout(false); } diff --git a/WebCore/rendering/RenderTableSection.cpp b/WebCore/rendering/RenderTableSection.cpp index eae5713e6859abbf653efcd756fd17fc654c2fd1..02a192d17817a0a0cc46e587de14ea5409cf6fd5 100644 --- a/WebCore/rendering/RenderTableSection.cpp +++ b/WebCore/rendering/RenderTableSection.cpp @@ -257,8 +257,9 @@ void RenderTableSection::addCell(RenderTableCell* cell, RenderObject* row) void RenderTableSection::setCellWidths() { Vector& columnPos = table()->columnPositions(); - bool pushedLayoutState = false; + LayoutStateMaintainer statePusher(view()); + for (int i = 0; i < m_gridRows; i++) { Row& row = *m_grid[i].row; int cols = row.size(); @@ -279,11 +280,10 @@ void RenderTableSection::setCellWidths() if (w != oldWidth) { 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(); } @@ -292,8 +292,7 @@ void RenderTableSection::setCellWidths() } } - if (pushedLayoutState) - view()->popLayoutState(); + statePusher.pop(); // only pops if we pushed } int RenderTableSection::calcRowHeight() @@ -301,7 +300,8 @@ int RenderTableSection::calcRowHeight() RenderTableCell* cell; int spacing = table()->vBorderSpacing(); - bool pushedLayoutState = false; + + LayoutStateMaintainer statePusher(view()); m_rowPos.resize(m_gridRows + 1); m_rowPos[0] = spacing; @@ -330,11 +330,10 @@ int RenderTableSection::calcRowHeight() int indx = max(r - cell->rowSpan() + 1, 0); 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); cell->setChildNeedsLayout(true, false); @@ -373,8 +372,7 @@ int RenderTableSection::calcRowHeight() m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[r]); } - if (pushedLayoutState) - view()->popLayoutState(); + statePusher.pop(); return m_rowPos[m_gridRows]; } @@ -456,7 +454,7 @@ int RenderTableSection::layoutRows(int toAdd) int vspacing = table()->vBorderSpacing(); 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++) { // Set the row's x/y position and width/height. @@ -575,7 +573,7 @@ int RenderTableSection::layoutRows(int toAdd) } } - view()->popLayoutState(); + statePusher.pop(); m_height = m_rowPos[totalRows]; m_overflowHeight = max(m_overflowHeight, m_height); diff --git a/WebCore/rendering/RenderView.h b/WebCore/rendering/RenderView.h index 6536979079a5ac36a745ad429628fd0f05ed2c99..db579b1e5d3d5e93465d147b7e7e084e3f3958bb 100644 --- a/WebCore/rendering/RenderView.h +++ b/WebCore/rendering/RenderView.h @@ -157,6 +157,64 @@ private: unsigned m_layoutStateDisableCount; }; +// 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 #endif // RenderView_h