- Revision
- 275873
- Author
- svil...@igalia.com
- Date
- 2021-04-13 02:14:23 -0700 (Tue, 13 Apr 2021)
Log Message
[css-flexbox] CDC COVID Vaccine Tracker: Safari garbles data table
https://bugs.webkit.org/show_bug.cgi?id=222581
Reviewed by Zalan Bujtas.
PerformanceTests:
* Layout/nested-flexboxes-percentage-flex-basis.html: Added.
Source/WebCore:
With nested flexboxes in which the flex-basis is a percentage the current code that stretches the item forces a relayout
of the children because it thinks it has percentage height descendants. That happens because we call
computePercentageLogicalHeights() with a mock percentage length to check whether a size is definite and that call performs
the addPercentageHeightDescendants() call. We should avoid calling the latter in those cases as we're just trying to
figure out whether we can compute the flex-basis used value or not.
Adding a new parameter to the aforementioned method so that the percentage height descendants map could be left untouched
in those cases where we just want to test size definiteness.
Apart from making the implementation more correct this brings some performance improvements for the cases described in the
first paragraph as it can be seen in the provided performance test (24.5% of improvement).
* rendering/RenderBox.cpp:
(WebCore::RenderBox::computePercentageLogicalHeight const):
* rendering/RenderBox.h:
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::canComputePercentageFlexBasis const):
(WebCore::RenderFlexibleBox::childMainSizeIsDefinite const):
(WebCore::RenderFlexibleBox::useChildOverridingMainSizeForPercentageResolution):
* rendering/RenderFlexibleBox.h:
LayoutTests:
* TestExpectations: Unskipped a WPT test which is now passing.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (275872 => 275873)
--- trunk/LayoutTests/ChangeLog 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/LayoutTests/ChangeLog 2021-04-13 09:14:23 UTC (rev 275873)
@@ -1,3 +1,12 @@
+2021-04-12 Sergio Villar Senin <svil...@igalia.com>
+
+ [css-flexbox] CDC COVID Vaccine Tracker: Safari garbles data table
+ https://bugs.webkit.org/show_bug.cgi?id=222581
+
+ Reviewed by Zalan Bujtas.
+
+ * TestExpectations: Unskipped a WPT test which is now passing.
+
2021-04-13 Antoine Quint <grao...@webkit.org>
calc() simplification for a multiplication should apply the multiplication to each value of an addition
Modified: trunk/LayoutTests/TestExpectations (275872 => 275873)
--- trunk/LayoutTests/TestExpectations 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/LayoutTests/TestExpectations 2021-04-13 09:14:23 UTC (rev 275873)
@@ -4018,9 +4018,6 @@
webkit.org/b/221479 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-003a.html [ ImageOnlyFailure ]
webkit.org/b/221479 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-004a.html [ ImageOnlyFailure ]
-# Percentages in flex-basis in nested flexboxes.
-webkit.org/b/222581 imported/w3c/web-platform-tests/css/css-flexbox/flex-basis-011.html [ ImageOnlyFailure ]
-
# break request in flexbox.
webkit.org/b/221480 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-break-request-horiz-001a.html [ ImageOnlyFailure ]
webkit.org/b/221480 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-break-request-horiz-001b.html [ ImageOnlyFailure ]
Modified: trunk/PerformanceTests/ChangeLog (275872 => 275873)
--- trunk/PerformanceTests/ChangeLog 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/PerformanceTests/ChangeLog 2021-04-13 09:14:23 UTC (rev 275873)
@@ -1,3 +1,12 @@
+2021-04-12 Sergio Villar Senin <svil...@igalia.com>
+
+ [css-flexbox] CDC COVID Vaccine Tracker: Safari garbles data table
+ https://bugs.webkit.org/show_bug.cgi?id=222581
+
+ Reviewed by Zalan Bujtas.
+
+ * Layout/nested-flexboxes-percentage-flex-basis.html: Added.
+
2021-03-17 Myles C. Maxfield <mmaxfi...@apple.com>
Laying out complex text in columns is O(n^2)
Added: trunk/PerformanceTests/Layout/nested-flexboxes-percentage-flex-basis.html (0 => 275873)
--- trunk/PerformanceTests/Layout/nested-flexboxes-percentage-flex-basis.html (rev 0)
+++ trunk/PerformanceTests/Layout/nested-flexboxes-percentage-flex-basis.html 2021-04-13 09:14:23 UTC (rev 275873)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<head>
+<style>
+.flex {
+ display: flex;
+ flex: 1 0 100%;
+}
+.col { flex-direction: column; }
+.row { flex-direction: row; }
+</style>
+<script src=""
+<script>
+function startTest() {
+ document.body.offsetHeight;
+
+ var index = 0;
+ PerfTestRunner.measureRunsPerSecond({run: function() {
+ document.body.style.width = ++index % 2 ? "99%" : "98%";
+ document.body.offsetHeight;
+ }});
+}
+</script>
+</head>
+<body _onload_="startTest()">
+<div class="flex row" style="flex: none;">
+ <div class="flex col">
+ <div class="flex row">
+ <div class="flex col">
+ <div class="flex row">
+ <div class="flex col">
+ <div class="flex row">
+ <div class="flex col">
+ <div class="flex row">
+ <div class="flex col">
+ <div class="flex row"></div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
+</div>
+</body>
Property changes on: trunk/PerformanceTests/Layout/nested-flexboxes-percentage-flex-basis.html
___________________________________________________________________
Added: svn:eol-style
+LF
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Modified: trunk/Source/WebCore/ChangeLog (275872 => 275873)
--- trunk/Source/WebCore/ChangeLog 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/Source/WebCore/ChangeLog 2021-04-13 09:14:23 UTC (rev 275873)
@@ -1,3 +1,31 @@
+2021-04-12 Sergio Villar Senin <svil...@igalia.com>
+
+ [css-flexbox] CDC COVID Vaccine Tracker: Safari garbles data table
+ https://bugs.webkit.org/show_bug.cgi?id=222581
+
+ Reviewed by Zalan Bujtas.
+
+ With nested flexboxes in which the flex-basis is a percentage the current code that stretches the item forces a relayout
+ of the children because it thinks it has percentage height descendants. That happens because we call
+ computePercentageLogicalHeights() with a mock percentage length to check whether a size is definite and that call performs
+ the addPercentageHeightDescendants() call. We should avoid calling the latter in those cases as we're just trying to
+ figure out whether we can compute the flex-basis used value or not.
+
+ Adding a new parameter to the aforementioned method so that the percentage height descendants map could be left untouched
+ in those cases where we just want to test size definiteness.
+
+ Apart from making the implementation more correct this brings some performance improvements for the cases described in the
+ first paragraph as it can be seen in the provided performance test (24.5% of improvement).
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::computePercentageLogicalHeight const):
+ * rendering/RenderBox.h:
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::canComputePercentageFlexBasis const):
+ (WebCore::RenderFlexibleBox::childMainSizeIsDefinite const):
+ (WebCore::RenderFlexibleBox::useChildOverridingMainSizeForPercentageResolution):
+ * rendering/RenderFlexibleBox.h:
+
2021-04-13 Ryosuke Niwa <rn...@webkit.org>
Use WTF::compactMap in HTMLSlotElement
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (275872 => 275873)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2021-04-13 09:14:23 UTC (rev 275873)
@@ -3102,7 +3102,7 @@
return scrollsOverflowY && !child.shouldTreatChildAsReplacedInTableCells() && (!cell.style().logicalHeight().isAuto() || !cell.table()->style().logicalHeight().isAuto());
}
-Optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length& height) const
+Optional<LayoutUnit> RenderBox::computePercentageLogicalHeight(const Length& height, UpdatePercentageHeightDescendants updateDescendants) const
{
Optional<LayoutUnit> availableHeight;
@@ -3118,7 +3118,8 @@
containingBlockChild = cb;
cb = cb->containingBlock();
}
- cb->addPercentHeightDescendant(const_cast<RenderBox&>(*this));
+ if (updateDescendants == UpdatePercentageHeightDescendants::Yes)
+ cb->addPercentHeightDescendant(const_cast<RenderBox&>(*this));
if (isHorizontal != cb->isHorizontalWritingMode())
availableHeight = containingBlockChild->containingBlockLogicalWidthForContent();
Modified: trunk/Source/WebCore/rendering/RenderBox.h (275872 => 275873)
--- trunk/Source/WebCore/rendering/RenderBox.h 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2021-04-13 09:14:23 UTC (rev 275873)
@@ -454,7 +454,8 @@
virtual LayoutUnit computeReplacedLogicalWidth(ShouldComputePreferred = ComputeActual) const;
virtual LayoutUnit computeReplacedLogicalHeight(Optional<LayoutUnit> estimatedUsedWidth = WTF::nullopt) const;
- Optional<LayoutUnit> computePercentageLogicalHeight(const Length& height) const;
+ enum class UpdatePercentageHeightDescendants { Yes , No };
+ Optional<LayoutUnit> computePercentageLogicalHeight(const Length& height, UpdatePercentageHeightDescendants = UpdatePercentageHeightDescendants::Yes) const;
LayoutUnit availableLogicalWidth() const { return contentLogicalWidth(); }
virtual LayoutUnit availableLogicalHeight(AvailableLogicalHeightType) const;
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (275872 => 275873)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-04-13 09:14:23 UTC (rev 275873)
@@ -832,7 +832,22 @@
else
child.setLocation(location.transposedPoint());
}
-
+
+bool RenderFlexibleBox::canComputePercentageFlexBasis(const RenderBox& child, const Length& flexBasis, UpdatePercentageHeightDescendants updateDescendants)
+{
+ if (!isColumnFlow() || m_hasDefiniteHeight == SizeDefiniteness::Definite)
+ return true;
+ if (m_hasDefiniteHeight == SizeDefiniteness::Indefinite)
+ return false;
+ bool definite = child.computePercentageLogicalHeight(flexBasis, updateDescendants).hasValue();
+ if (m_inLayout && (isHorizontalWritingMode() == child.isHorizontalWritingMode())) {
+ // We can reach this code even while we're not laying ourselves out, such
+ // as from mainSizeForPercentageResolution.
+ m_hasDefiniteHeight = definite ? SizeDefiniteness::Definite : SizeDefiniteness::Indefinite;
+ }
+ return definite;
+}
+
bool RenderFlexibleBox::childMainSizeIsDefinite(const RenderBox& child, const Length& flexBasis)
{
if (flexBasis.isAuto())
@@ -839,20 +854,8 @@
return false;
if (isColumnFlow() && flexBasis.isIntrinsic())
return false;
- if (flexBasis.isPercentOrCalculated()) {
- if (!isColumnFlow() || m_hasDefiniteHeight == SizeDefiniteness::Definite)
- return true;
- if (m_hasDefiniteHeight == SizeDefiniteness::Indefinite)
- return false;
- bool definite = child.computePercentageLogicalHeight(flexBasis).hasValue();
- // Do not cache the definite height state with orthogonal children as in that case the logical height
- // of the child is not in the same axis as the logical height of the flex container. Also do not cache it
- // outside the layout process (we can reach this code as from mainSizeForPercentageResolution()).
- if (m_inLayout && (isHorizontalWritingMode() == child.isHorizontalWritingMode()))
- m_hasDefiniteHeight = definite ? SizeDefiniteness::Definite : SizeDefiniteness::Indefinite;
-
- return definite;
- }
+ if (flexBasis.isPercentOrCalculated())
+ return canComputePercentageFlexBasis(child, flexBasis, UpdatePercentageHeightDescendants::No);
return true;
}
@@ -1244,7 +1247,8 @@
// This function implements section 9.8. Definite and Indefinite Sizes, case 2) of the flexbox spec.
// If the flex container has a definite main size the flex item post-flexing main size is also treated
// as definite. We make up a percentage to check whether we have a definite size.
- if (!childMainSizeIsDefinite(child, Length(0, LengthType::Percent)))
+ auto updateDescendants = m_inLayout ? UpdatePercentageHeightDescendants::Yes : UpdatePercentageHeightDescendants::No;
+ if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), updateDescendants))
return false;
return child.hasOverridingLogicalHeight();
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (275872 => 275873)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2021-04-13 09:05:44 UTC (rev 275872)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2021-04-13 09:14:23 UTC (rev 275873)
@@ -149,6 +149,7 @@
LayoutUnit computeInnerFlexBaseSizeForChild(RenderBox& child, LayoutUnit mainAxisBorderAndPadding);
void adjustAlignmentForChild(RenderBox& child, LayoutUnit);
ItemPosition alignmentForChild(const RenderBox& child) const;
+ bool canComputePercentageFlexBasis(const RenderBox& child, const Length& flexBasis, UpdatePercentageHeightDescendants);
bool childMainSizeIsDefinite(const RenderBox&, const Length& flexBasis);
bool childCrossSizeIsDefinite(const RenderBox&, const Length& flexBasis);
bool needToStretchChildLogicalHeight(const RenderBox& child) const;
