Title: [224653] trunk/Source/WebCore
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(); }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to