Title: [275754] trunk
Revision
275754
Author
[email protected]
Date
2021-04-09 02:17:59 -0700 (Fri, 09 Apr 2021)

Log Message

[css-grid] Fix min/max widths of grid affected by ancestor
https://bugs.webkit.org/show_bug.cgi?id=222100

Patch by Ziran Sun <[email protected]> on 2021-04-09
LayoutTests/imported/w3c:

Reviewed by Javier Fernandez.

* web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt:
* web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt:
* web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt:

Source/WebCore:

Reviewed by Reviewed by Javier Fernandez.

It's a reland of r273435, which got reverted because it broke
imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html. This change
narrows down the fix specificallly to the case when logical-width is recomputed.
We need to recalculate min/max widths of child that depend on the ancestor.
Before update logical-width, for element that needs preferredWidth recalcution,
it is necessary to make sure that min/max widths are set dirty.

This change is an import of chromium CL at
https://chromium-review.googlesource.com/c/chromium/src/+/527640/
Only the parts that apply to this issue are imported.

Tests were already imported in WPT.

* rendering/RenderBlock.cpp:
(WebCore::shouldRecalculateMinMaxWidthsAffectedByAncestor):
(WebCore::RenderBlock::recomputeLogicalWidth):

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (275753 => 275754)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-09 09:17:59 UTC (rev 275754)
@@ -1,3 +1,14 @@
+2021-04-09  Ziran Sun  <[email protected]>
+
+        [css-grid] Fix min/max widths of grid affected by ancestor
+        https://bugs.webkit.org/show_bug.cgi?id=222100
+
+        Reviewed by Javier Fernandez.
+
+        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt:
+        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt:
+        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt:
+
 2021-04-08  Youenn Fablet  <[email protected]>
 
         Streams: new test failure for canceling the branches of an errored tee'd stream

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt (275753 => 275754)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt	2021-04-09 09:17:59 UTC (rev 275754)
@@ -1,30 +1,10 @@
 
-FAIL .grid 1 assert_equals:
-<div class="grid">
-  <div class="paddingLeft50Percent" data-expected-padding-left="50" data-expected-width="60" data-expected-height="10">X</div>
-  <div data-offset-x="0" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
-</div>
-width expected 60 but got 50
-FAIL .grid 2 assert_equals:
-<div class="grid">
-  <div class="paddingRight50Percent" data-expected-padding-right="50" data-expected-width="60" data-expected-height="10">X</div>
-  <div data-offset-x="0" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
-</div>
-width expected 60 but got 50
+PASS .grid 1
+PASS .grid 2
 PASS .grid 3
 PASS .grid 4
-FAIL .grid 5 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingLeft50Percent" data-expected-padding-left="50" data-expected-width="60" data-expected-height="10">X</div>
-  <div data-offset-x="400" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
-</div>
-width expected 60 but got 50
-FAIL .grid 6 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingRight50Percent" data-expected-padding-right="50" data-expected-width="60" data-expected-height="10">X</div>
-  <div data-offset-x="400" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
-</div>
-width expected 60 but got 50
+PASS .grid 5
+PASS .grid 6
 PASS .grid 7
 PASS .grid 8
 Direction LTR

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt (275753 => 275754)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt	2021-04-09 09:17:59 UTC (rev 275754)
@@ -1,32 +1,12 @@
 
 PASS .grid 1
 PASS .grid 2
-FAIL .grid 3 assert_equals:
-<div class="grid">
-  <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="10" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
-FAIL .grid 4 assert_equals:
-<div class="grid">
-  <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="10" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
+PASS .grid 3
+PASS .grid 4
 PASS .grid 5
 PASS .grid 6
-FAIL .grid 7 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="10" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
-FAIL .grid 8 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="10" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
+PASS .grid 7
+PASS .grid 8
 Direction LTR
 
 Item padding-left: 50%;

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt (275753 => 275754)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt	2021-04-09 09:17:59 UTC (rev 275754)
@@ -1,32 +1,12 @@
 
 PASS .grid 1
 PASS .grid 2
-FAIL .grid 3 assert_equals:
-<div class="grid">
-  <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="0" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
-FAIL .grid 4 assert_equals:
-<div class="grid">
-  <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="0" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
+PASS .grid 3
+PASS .grid 4
 PASS .grid 5
 PASS .grid 6
-FAIL .grid 7 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="0" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
-FAIL .grid 8 assert_equals:
-<div class="grid directionRTL">
-  <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
-  <div data-offset-x="0" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
-</div>
-height expected 60 but got 50
+PASS .grid 7
+PASS .grid 8
 Direction LTR
 
 Item padding-left: 50%;

Modified: trunk/Source/WebCore/ChangeLog (275753 => 275754)


--- trunk/Source/WebCore/ChangeLog	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/Source/WebCore/ChangeLog	2021-04-09 09:17:59 UTC (rev 275754)
@@ -1,3 +1,27 @@
+2021-04-09  Ziran Sun  <[email protected]>
+
+        [css-grid] Fix min/max widths of grid affected by ancestor
+        https://bugs.webkit.org/show_bug.cgi?id=222100
+
+        Reviewed by Reviewed by Javier Fernandez.
+
+        It's a reland of r273435, which got reverted because it broke 
+        imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html. This change
+        narrows down the fix specificallly to the case when logical-width is recomputed.
+        We need to recalculate min/max widths of child that depend on the ancestor.
+        Before update logical-width, for element that needs preferredWidth recalcution,
+        it is necessary to make sure that min/max widths are set dirty.
+
+        This change is an import of chromium CL at 
+        https://chromium-review.googlesource.com/c/chromium/src/+/527640/
+        Only the parts that apply to this issue are imported.
+
+        Tests were already imported in WPT.       
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::shouldRecalculateMinMaxWidthsAffectedByAncestor):
+        (WebCore::RenderBlock::recomputeLogicalWidth):
+
 2021-04-09  Yusuke Suzuki  <[email protected]>
 
         ServiceWorker should save module scripts

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (275753 => 275754)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2021-04-09 09:17:42 UTC (rev 275753)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2021-04-09 09:17:59 UTC (rev 275754)
@@ -631,10 +631,31 @@
         fragmentedFlow->logicalWidthChangedInFragmentsForBlock(this, relayoutChildren);
 }
 
+static bool shouldRecalculateMinMaxWidthsAffectedByAncestor(const RenderBox* box)
+{
+    // If the preferred widths are already dirty at this point (during layout), it actually means that we never need to calculate them, since that should
+    // have been carried out by an ancestor that's sized based on preferred widths (a shrink-to-fit container, for instance). In such cases the
+    // object will be left as dirty indefinitely, and it would just be a waste of time to calculate the preferred withs when nobody needs them.
+    if (box->preferredLogicalWidthsDirty())
+        return false;
+    // If our containing block also has min/max widths that are affected by the ancestry, we have already dealt with this object as well. Avoid
+    // unnecessary work and O(n^2) time complexity.
+    if (const RenderBox* cb = box->containingBlock()) {
+        if (cb->needsPreferredWidthsRecalculation() && !cb->preferredLogicalWidthsDirty())
+            return false;
+    }
+    return true;
+}
+
 bool RenderBlock::recomputeLogicalWidth()
 {
     LayoutUnit oldWidth = logicalWidth();
-    
+
+    // Laying out this object means that its containing block is also being laid out. This object is special, in that its min/max widths depend on
+    // the ancestry (min/max width calculation should ideally be strictly bottom-up, but that's not always the case), so since the containing
+    // block size may have changed, we need to recalculate the min/max widths of this object, and every child that has the same issue, recursively.
+    if (needsPreferredWidthsRecalculation() && shouldRecalculateMinMaxWidthsAffectedByAncestor(this))
+        setPreferredLogicalWidthsDirty(true, MarkOnlyThis);
     updateLogicalWidth();
     
     bool hasBorderOrPaddingLogicalWidthChanged = this->hasBorderOrPaddingLogicalWidthChanged();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to