Flex layout triggers excessive layout on height percentage descendants
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Nov 2019 03:16:44 +0000 (03:16 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Nov 2019 03:16:44 +0000 (03:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204319
<rdar://problem/57236652>

Reviewed by Simon Fraser.

This is very similar to r252562, except in this case the layout is explicitly triggered by the flex layout logic.
The patch ensures that we don't try to lay out percent height descendants with out-of-flow ancestors (see r252562 or webkit.org/b/204255 for more info).
(Unfortunately this is not testable but the subsequent repaint fix will include a test which would fail if we regressed this.)

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::hasPercentHeightDescendants const):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
* rendering/RenderFlexibleBox.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252620 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 283e155..8305c79 100644 (file)
@@ -1,3 +1,20 @@
+2019-11-18  Zalan Bujtas  <zalan@apple.com>
+
+        Flex layout triggers excessive layout on height percentage descendants
+        https://bugs.webkit.org/show_bug.cgi?id=204319
+        <rdar://problem/57236652>
+
+        Reviewed by Simon Fraser.
+
+        This is very similar to r252562, except in this case the layout is explicitly triggered by the flex layout logic.
+        The patch ensures that we don't try to lay out percent height descendants with out-of-flow ancestors (see r252562 or webkit.org/b/204255 for more info).
+        (Unfortunately this is not testable but the subsequent repaint fix will include a test which would fail if we regressed this.)
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::hasPercentHeightDescendants const):
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
+        * rendering/RenderFlexibleBox.h:
+
 2019-11-18  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Local Resource Overrides: allow substitution based on a url pattern
index 6485a69..c5dc039 100644 (file)
@@ -1533,6 +1533,33 @@ Overflow RenderFlexibleBox::crossAxisOverflowForChild(const RenderBox& child) co
     return child.style().overflowX();
 }
 
+bool RenderFlexibleBox::hasPercentHeightDescendants(const RenderBox& renderer) const
+{
+    // FIXME: This function can be removed soon after webkit.org/b/204318 is fixed. 
+    if (!is<RenderBlock>(renderer))
+        return false;
+    auto& renderBlock = downcast<RenderBlock>(renderer);
+    if (!renderBlock.hasPercentHeightDescendants())
+        return false;
+
+    auto* percentHeightDescendants = renderBlock.percentHeightDescendants();
+    if (!percentHeightDescendants)
+        return false;
+
+    for (auto it = percentHeightDescendants->begin(), end = percentHeightDescendants->end(); it != end; ++it) {
+        bool hasOutOfFlowAncestor = false;
+        for (auto* ancestor = (*it)->containingBlock(); ancestor && ancestor != &renderBlock; ancestor = ancestor->containingBlock()) {
+            if (ancestor->isOutOfFlowPositioned()) {
+                hasOutOfFlowAncestor = true;
+                break;
+            }
+        }
+        if (!hasOutOfFlowAncestor)
+            return true;
+    }
+    return false;
+}
+
 void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, Vector<FlexItem>& children, LayoutUnit availableFreeSpace, bool relayoutChildren, Vector<LineContext>& lineContexts)
 {
     ContentPosition position = style().resolvedJustifyContentPosition(contentAlignmentNormalBehavior());
@@ -1569,7 +1596,7 @@ void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, Vect
         // We may have already forced relayout for orthogonal flowing children in
         // computeInnerFlexBaseSizeForChild.
         bool forceChildRelayout = relayoutChildren && !m_relaidOutChildren.contains(&child);
-        if (child.isRenderBlock() && downcast<RenderBlock>(child).hasPercentHeightDescendants()) {
+        if (!forceChildRelayout && hasPercentHeightDescendants(child)) {
             // Have to force another relayout even though the child is sized
             // correctly, because its descendants are not sized correctly yet. Our
             // previous layout of the child was done without an override height set.
index df67496..8eb1dc7 100644 (file)
@@ -188,6 +188,8 @@ private:
     void appendChildFrameRects(ChildFrameRects&);
     void repaintChildrenDuringLayoutIfMoved(const ChildFrameRects&);
 
+    bool hasPercentHeightDescendants(const RenderBox&) const;
+
     // This is used to cache the preferred size for orthogonal flow children so we
     // don't have to relayout to get it
     HashMap<const RenderBox*, LayoutUnit> m_intrinsicSizeAlongMainAxis;