Changeset 273264 in webkit

Timestamp:

Feb 22, 2021 11:12:03 AM (3 years ago)

Author:

svillar@igalia.com

Message:

REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
​https://bugs.webkit.org/show_bug.cgi?id=222202
<rdar://problem/74537782>

Reviewed by Simon Fraser.

PerformanceTests:

New performance test for nested column flexboxes with percentage heights.

• Layout/nested-column-flexboxes-relative-height.html: Added.

Source/WebCore:

The problem was that we were doing the initial layout for the children of the flex container twice in those cases where
the child inline axis was not the main axis (for example with column flex containers in horizontal writing modes).
Refactored the code (specially the way we clear overriding sizes) so that we only do it once. This saves tons of layouts
in pages with nested column flexboxes with relative heights.

No new tests as there is no change in functionality, we're removing duplicate extra layouts. We're however adding a new
performance test for column flexboxes with percentage heights. With this patch we go from 3.5 layout/s to 145 layout/s
which is ~4000% better.

Inspired by Blink's crrev.com/c/1614058 by <​cbiesinger@chromium.org>.

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild): Do not unconditionally clear overriding sizes. Also removed
relayoutChildren which is now unused. Do not layout the item, that should have been done in
computeInnerFlexBaseSizeForChild() before. Added ASSERTs to verify that child's intrinsic main size was cached as
a consequence of the previous layout.
(WebCore::RenderFlexibleBox::constructFlexItem): Do not pass relayoutChildren to computeInnerFlexBaseSizeForChild. Also no
need to update it after laying out the child.

• rendering/RenderFlexibleBox.h:

Location:

trunk

Files:

• LayoutTests/TestExpectations (modified) (1 diff)
• PerformanceTests/ChangeLog (modified) (1 diff)
• PerformanceTests/Layout/nested-column-flexboxes-relative-height.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (5 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (1 diff)

trunk/LayoutTests/TestExpectations

@@ -3916 +3916 @@
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-017.html [ ImageOnlyFailure ]
+webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-017.html [ Failure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]

trunk/PerformanceTests/ChangeLog

@@ +1 @@
+2021-02-22  Sergio Villar Senin  <svillar@igalia.com>
+
+        REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
+        https://bugs.webkit.org/show_bug.cgi?id=222202
+        <rdar://problem/74537782>
+
+        Reviewed by Simon Fraser.
+
+        New performance test for nested column flexboxes with percentage heights.
+
+        * Layout/nested-column-flexboxes-relative-height.html: Added.
+
 2021-02-18  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-02-22  Sergio Villar Senin  <svillar@igalia.com>
+
+        REGRESSION (r266695): twitch.tv: when in fullscreen, WebKit continually does 350ms layouts. Firefox and Chrome do not
+        https://bugs.webkit.org/show_bug.cgi?id=222202
+        <rdar://problem/74537782>
+
+        Reviewed by Simon Fraser.
+
+        The problem was that we were doing the initial layout for the children of the flex container twice in those cases where
+        the child inline axis was not the main axis (for example with column flex containers in horizontal writing modes).
+        Refactored the code (specially the way we clear overriding sizes) so that we only do it once. This saves tons of layouts
+        in pages with nested column flexboxes with relative heights.
+
+        No new tests as there is no change in functionality, we're removing duplicate extra layouts. We're however adding a new
+        performance test for column flexboxes with percentage heights. With this patch we go from 3.5 layout/s to 145 layout/s
+        which is ~4000% better.
+
+        Inspired by Blink's crrev.com/c/1614058 by <cbiesinger@chromium.org>.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild): Do not unconditionally clear overriding sizes. Also removed
+        relayoutChildren which is now unused. Do not layout the item, that should have been done in
+        computeInnerFlexBaseSizeForChild() before. Added ASSERTs to verify that child's intrinsic main size was cached as
+        a consequence of the previous layout.
+        (WebCore::RenderFlexibleBox::constructFlexItem): Do not pass relayoutChildren to computeInnerFlexBaseSizeForChild. Also no
+        need to update it after laying out the child.
+        * rendering/RenderFlexibleBox.h:
+
 2021-02-22  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -923 +923 @@
 
     
-LayoutUnit RenderFlexibleBox::computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding, bool relayoutChildren)
-{
-    child.clearOverridingContentSize();
-    
+LayoutUnit RenderFlexibleBox::computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding)
+{
     Length flexBasis = flexBasisForChild(child);
     if (childMainSizeIsDefinite(child, flexBasis))
@@ -936 +934 @@
     }
 
-    // The flex basis is indefinite (=auto), so we need to compute the actual
-    // width of the child. For the logical width axis we just use the preferred
-    // width; for the height we need to lay out the child.
+    // The flex basis is indefinite (=auto), so we need to compute the actual width of the child.
     LayoutUnit mainAxisExtent;
     if (!mainAxisIsChildInlineAxis(child)) {
-        updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
-        if (child.needsLayout() || relayoutChildren || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
-            if (!child.needsLayout())
-                child.setChildNeedsLayout(MarkOnlyThis);
-            child.layoutIfNeeded();
-            cacheChildMainSize(child);
-        }
+        ASSERT(!child.needsLayout());
+        ASSERT(m_intrinsicSizeAlongMainAxis.contains(&child));
         mainAxisExtent = m_intrinsicSizeAlongMainAxis.get(&child);
     } else {
@@ -1300 +1291 @@
 FlexItem RenderFlexibleBox::constructFlexItem(RenderBox& child, bool relayoutChildren)
 {
+    child.clearOverridingContentSize();
     if (childHasIntrinsicMainAxisSize(child)) {
         // If this condition is true, then computeMainAxisExtentForChild will call
@@ -1318 +1310 @@
             child.layoutIfNeeded();
             cacheChildMainSize(child);
-            relayoutChildren = false;
             child.clearOverridingContainingBlockContentSize();
         }
@@ -1324 +1315 @@
     
     LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
-    LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding, relayoutChildren);
+    LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
     LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(child, childInnerFlexBaseSize);
     LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -147 +147 @@
     LayoutUnit computeMainSizeFromAspectRatioUsing(const RenderBox& child, Length crossSizeLength) const;
     void setFlowAwareLocationForChild(RenderBox& child, const LayoutPoint&);
-    LayoutUnit computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding, bool relayoutChildren);
+    LayoutUnit computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding);
     void adjustAlignmentForChild(RenderBox& child, LayoutUnit);
     ItemPosition alignmentForChild(const RenderBox& child) const;