- Revision
- 224653
- Author
- za...@apple.com
- Date
- 2017-11-09 16:31:24 -0800 (Thu, 09 Nov 2017)
Log Message
[LayoutState cleanup] Remove explicit pop from LayoutState
https://bugs.webkit.org/show_bug.cgi?id=179509
<rdar://problem/35454323>
Reviewed by Antti Koivisto.
Both relayoutForPagination()/relayoutToAvoidWidows() call layout recursively which requires
manual layout state pop. This patch addresses this issue by constructing a new LayoutState object
for the positioned descendants.
Covered by existing tests.
* rendering/LayoutState.cpp:
(WebCore::LayoutStateMaintainer::~LayoutStateMaintainer):
(WebCore::LayoutStateMaintainer::pop): Deleted.
* rendering/LayoutState.h:
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::layoutBlock):
(WebCore::RenderBlockFlow::relayoutToAvoidWidows):
(WebCore::RenderBlockFlow::relayoutForPagination):
* rendering/RenderBlockFlow.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (224652 => 224653)
--- trunk/Source/WebCore/ChangeLog 2017-11-09 23:45:29 UTC (rev 224652)
+++ trunk/Source/WebCore/ChangeLog 2017-11-10 00:31:24 UTC (rev 224653)
@@ -1,3 +1,27 @@
+2017-11-09 Zalan Bujtas <za...@apple.com>
+
+ [LayoutState cleanup] Remove explicit pop from LayoutState
+ https://bugs.webkit.org/show_bug.cgi?id=179509
+ <rdar://problem/35454323>
+
+ Reviewed by Antti Koivisto.
+
+ Both relayoutForPagination()/relayoutToAvoidWidows() call layout recursively which requires
+ manual layout state pop. This patch addresses this issue by constructing a new LayoutState object
+ for the positioned descendants.
+
+ Covered by existing tests.
+
+ * rendering/LayoutState.cpp:
+ (WebCore::LayoutStateMaintainer::~LayoutStateMaintainer):
+ (WebCore::LayoutStateMaintainer::pop): Deleted.
+ * rendering/LayoutState.h:
+ * rendering/RenderBlockFlow.cpp:
+ (WebCore::RenderBlockFlow::layoutBlock):
+ (WebCore::RenderBlockFlow::relayoutToAvoidWidows):
+ (WebCore::RenderBlockFlow::relayoutForPagination):
+ * rendering/RenderBlockFlow.h:
+
2017-11-09 Chris Dumez <cdu...@apple.com>
Implement real post 'install' event steps of the Install algorithm (steps 14+)
Modified: trunk/Source/WebCore/rendering/LayoutState.cpp (224652 => 224653)
--- trunk/Source/WebCore/rendering/LayoutState.cpp 2017-11-09 23:45:29 UTC (rev 224652)
+++ trunk/Source/WebCore/rendering/LayoutState.cpp 2017-11-10 00:31:24 UTC (rev 224653)
@@ -283,15 +283,6 @@
LayoutStateMaintainer::~LayoutStateMaintainer()
{
- // FIXME: Remove conditional pop.
- if (!m_didCallPop)
- pop();
-}
-
-void LayoutStateMaintainer::pop()
-{
- ASSERT(!m_didCallPop);
- m_didCallPop = true;
if (!m_didPushLayoutState)
return;
m_context.popLayoutState();
Modified: trunk/Source/WebCore/rendering/LayoutState.h (224652 => 224653)
--- trunk/Source/WebCore/rendering/LayoutState.h 2017-11-09 23:45:29 UTC (rev 224652)
+++ trunk/Source/WebCore/rendering/LayoutState.h 2017-11-10 00:31:24 UTC (rev 224653)
@@ -141,12 +141,9 @@
explicit LayoutStateMaintainer(RenderBox&, LayoutSize offset, bool disableState = false, LayoutUnit pageHeight = 0, bool pageHeightChanged = false);
~LayoutStateMaintainer();
- void pop();
-
private:
LayoutContext& m_context;
bool m_paintOffsetCacheIsDisabled { false };
- bool m_didCallPop { false };
bool m_didPushLayoutState { false };
};
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (224652 => 224653)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2017-11-09 23:45:29 UTC (rev 224652)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2017-11-10 00:31:24 UTC (rev 224653)
@@ -477,6 +477,7 @@
LayoutUnit repaintLogicalTop = 0;
LayoutUnit repaintLogicalBottom = 0;
+ LayoutUnit maxFloatLogicalBottom = 0;
const RenderStyle& styleToUse = style();
{
LayoutStateMaintainer statePusher(*this, locationOffset(), hasTransform() || hasReflection() || styleToUse.isFlippedBlocksWritingMode(), pageLogicalHeight, pageLogicalHeightChanged);
@@ -501,7 +502,6 @@
setHasMarginAfterQuirk(styleToUse.hasMarginAfterQuirk());
setPaginationStrut(0);
}
- LayoutUnit maxFloatLogicalBottom = 0;
if (!firstChild() && !isAnonymousBlock())
setChildrenInline(true);
if (childrenInline())
@@ -508,28 +508,33 @@
layoutInlineChildren(relayoutChildren, repaintLogicalTop, repaintLogicalBottom);
else
layoutBlockChildren(relayoutChildren, maxFloatLogicalBottom);
+ }
- // Expand our intrinsic height to encompass floats.
- LayoutUnit toAdd = borderAndPaddingAfter() + scrollbarLogicalHeight();
- if (lowestFloatLogicalBottom() > (logicalHeight() - toAdd) && createsNewFormattingContext())
- setLogicalHeight(lowestFloatLogicalBottom() + toAdd);
+ // Expand our intrinsic height to encompass floats.
+ LayoutUnit toAdd = borderAndPaddingAfter() + scrollbarLogicalHeight();
+ if (lowestFloatLogicalBottom() > (logicalHeight() - toAdd) && createsNewFormattingContext())
+ setLogicalHeight(lowestFloatLogicalBottom() + toAdd);
+ if (relayoutForPagination() || relayoutToAvoidWidows()) {
+ ASSERT(!shouldBreakAtLineToAvoidWidow());
+ return;
+ }
- if (relayoutForPagination(statePusher) || relayoutToAvoidWidows(statePusher)) {
- ASSERT(!shouldBreakAtLineToAvoidWidow());
- return;
- }
+ // Calculate our new height.
+ LayoutUnit oldHeight = logicalHeight();
+ LayoutUnit oldClientAfterEdge = clientLogicalBottom();
- // Calculate our new height.
- LayoutUnit oldHeight = logicalHeight();
- LayoutUnit oldClientAfterEdge = clientLogicalBottom();
+ // Before updating the final size of the flow thread make sure a forced break is applied after the content.
+ // This ensures the size information is correctly computed for the last auto-height fragment receiving content.
+ if (is<RenderFragmentedFlow>(*this))
+ downcast<RenderFragmentedFlow>(*this).applyBreakAfterContent(oldClientAfterEdge);
- // Before updating the final size of the flow thread make sure a forced break is applied after the content.
- // This ensures the size information is correctly computed for the last auto-height fragment receiving content.
- if (is<RenderFragmentedFlow>(*this))
- downcast<RenderFragmentedFlow>(*this).applyBreakAfterContent(oldClientAfterEdge);
+ updateLogicalHeight();
+ LayoutUnit newHeight = logicalHeight();
+ {
+ // FIXME: This could be removed once relayoutForPagination()/relayoutToAvoidWidows() either stop recursing or we manage to
+ // re-order them.
+ LayoutStateMaintainer statePusher(*this, locationOffset(), hasTransform() || hasReflection() || styleToUse.isFlippedBlocksWritingMode(), pageLogicalHeight, pageLogicalHeightChanged);
- updateLogicalHeight();
- LayoutUnit newHeight = logicalHeight();
if (oldHeight != newHeight) {
if (oldHeight > newHeight && maxFloatLogicalBottom > newHeight && !childrenInline()) {
// One of our children's floats may have become an overhanging float for us. We need to look for it.
@@ -545,14 +550,13 @@
bool heightChanged = (previousHeight != newHeight);
if (heightChanged)
relayoutChildren = true;
-
layoutPositionedObjects(relayoutChildren || isDocumentElementRenderer());
+ }
+ // Add overflow from children (unless we're multi-column, since in that case all our child overflow is clipped anyway).
+ computeOverflow(oldClientAfterEdge);
- // Add overflow from children (unless we're multi-column, since in that case all our child overflow is clipped anyway).
- computeOverflow(oldClientAfterEdge);
+ fitBorderToLinesIfNeeded();
- fitBorderToLinesIfNeeded();
- }
auto* state = view().frameView().layoutContext().layoutState();
if (state && state->pageLogicalHeight())
setPageLogicalOffset(state->pageLogicalOffset(this, logicalTop()));
@@ -1796,12 +1800,11 @@
rareBlockFlowData()->m_lineBreakToAvoidWidow = -1;
}
-bool RenderBlockFlow::relayoutToAvoidWidows(LayoutStateMaintainer& statePusher)
+bool RenderBlockFlow::relayoutToAvoidWidows()
{
if (!shouldBreakAtLineToAvoidWidow())
return false;
- statePusher.pop();
setEverHadLayout(true);
layoutBlock(false);
return true;
@@ -3498,7 +3501,7 @@
m_lineBoxes.paint(this, paintInfo, paintOffset);
}
-bool RenderBlockFlow::relayoutForPagination(LayoutStateMaintainer& statePusher)
+bool RenderBlockFlow::relayoutForPagination()
{
if (!multiColumnFlow() || !multiColumnFlow()->shouldRelayoutForPagination())
return false;
@@ -3532,8 +3535,6 @@
neededRelayout = true;
multiColumnFlow()->setChildNeedsLayout(MarkOnlyThis);
setChildNeedsLayout(MarkOnlyThis);
- if (firstPass)
- statePusher.pop();
layoutBlock(false);
}
firstPass = false;
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (224652 => 224653)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.h 2017-11-09 23:45:29 UTC (rev 224652)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h 2017-11-10 00:31:24 UTC (rev 224653)
@@ -33,7 +33,6 @@
namespace WebCore {
class FloatWithRect;
-class LayoutStateMaintainer;
class LineBreaker;
class LineInfo;
class RenderMultiColumnFlow;
@@ -258,7 +257,7 @@
void clearDidBreakAtLineToAvoidWidow();
void setDidBreakAtLineToAvoidWidow();
bool didBreakAtLineToAvoidWidow() const { return hasRareBlockFlowData() && rareBlockFlowData()->m_didBreakAtLineToAvoidWidow; }
- bool relayoutToAvoidWidows(LayoutStateMaintainer&);
+ bool relayoutToAvoidWidows();
RootInlineBox* lineGridBox() const { return hasRareBlockFlowData() ? rareBlockFlowData()->m_lineGridBox.get() : nullptr; }
void setLineGridBox(std::unique_ptr<RootInlineBox> box)
@@ -601,7 +600,7 @@
void updateFragmentForLine(RootInlineBox*) const;
// Pagination routines.
- bool relayoutForPagination(LayoutStateMaintainer&);
+ bool relayoutForPagination();
bool hasRareBlockFlowData() const { return m_rareBlockFlowData.get(); }
RenderBlockFlowRareData* rareBlockFlowData() const { ASSERT_WITH_SECURITY_IMPLICATION(hasRareBlockFlowData()); return m_rareBlockFlowData.get(); }