Title: [285998] trunk
Revision
285998
Author
[email protected]
Date
2021-11-18 05:12:28 -0800 (Thu, 18 Nov 2021)

Log Message

Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
https://bugs.webkit.org/show_bug.cgi?id=228022

LayoutTests/imported/w3c:

Reviewed by Javier Fernandez.

* web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt:
* web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt:
* web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt:
* web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt:
* web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt:

Source/WebCore:

This is a re-land of r280078 and r282008. r280078 has caused a crash. Details on the crash test case is
at https://bugs.webkit.org/show_bug.cgi?id=232922. The reason for the crash is that, when recomputing the
intrinsic width we need to recompute the intrinsic width of the caller, which is the grid item, rather than
the grid in this case.

This patch addresses the original issue and the cause for the crash.

When computing the logical height using an intrinsic aspect ratio, RenderReplaced should use the
overridingLogicalWidth whenever defined just as how it does for flex items. This change is to
replace the use of intrinsic (non-stretched) logical width in current code with the overridingLogicalWidth.

The current code doesn't set overringLogicalWidth while the width changes while stretches along Row Axis.
This change adds the logic in.

Reviewed by Javier Fernandez.

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeReplacedLogicalHeight const):

LayoutTests:

Reviewed by Javier Fernandez.

* TestExpectations: unskip tests that are now passing.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (285997 => 285998)


--- trunk/LayoutTests/ChangeLog	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/ChangeLog	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,3 +1,12 @@
+2021-11-18  Ziran Sun  <[email protected]>
+
+        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
+        https://bugs.webkit.org/show_bug.cgi?id=228022
+
+        Reviewed by Javier Fernandez.
+
+        * TestExpectations: unskip tests that are now passing.
+
 2021-11-18  Youenn Fablet  <[email protected]>
 
         MediaPlayerAVFoundation should support rvfc

Modified: trunk/LayoutTests/TestExpectations (285997 => 285998)


--- trunk/LayoutTests/TestExpectations	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/TestExpectations	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1388,9 +1388,6 @@
 imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-015.html [ Pass Failure ]
 webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-align-001.html [ ImageOnlyFailure ]
 webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-justify-001.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-002.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html [ ImageOnlyFailure ]
 
 imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-dynamic-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-repeat-max-width-001.html [ ImageOnlyFailure ]
@@ -4326,8 +4323,6 @@
 webkit.org/b/212201 imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-limits-001.html [ Skip ]
 webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-1.html [ ImageOnlyFailure ]
 webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-2.html [ ImageOnlyFailure ]
-webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-3.html [ ImageOnlyFailure ]
-webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-4.html [ ImageOnlyFailure ]
 webkit.org/b/212246 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-align-cycles-001.html [ ImageOnlyFailure ]
 webkit.org/b/231021 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-inline-baseline.html [ ImageOnlyFailure ]
 webkit.org/b/212246 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-content-baseline-001.html [ ImageOnlyFailure ]

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,3 +1,16 @@
+2021-11-18  Ziran Sun  <[email protected]>
+
+        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
+        https://bugs.webkit.org/show_bug.cgi?id=228022
+
+        Reviewed by Javier Fernandez.
+
+        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt:
+        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt:
+        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt:
+        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt:
+        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt:
+
 2021-11-18  Arcady Goldmints-Orlov  <[email protected]>
 
         Update test expectations after r285346

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,7 +1,5 @@
 
 
-FAIL .before 1 assert_equals:
-<img id="item" src="" class=" before" data-expected-width="200" data-expected-height="200">
-height expected 200 but got 100
+PASS .before 1
 PASS .after 2
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,7 +1,5 @@
 
 
 PASS .before 1
-FAIL .after 2 assert_equals:
-<img id="item" src="" class=" before after" data-expected-width="200" data-expected-height="200">
-height expected 200 but got 100
+PASS .after 2
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,7 +1,5 @@
 
 
-FAIL .before 1 assert_equals:
-<img id="item" src="" class=" before" data-expected-width="200" data-expected-height="200">
-height expected 200 but got 100
+PASS .before 1
 PASS .after 2
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,7 +1,5 @@
 
 
 PASS .before 1
-FAIL .after 2 assert_equals:
-<img id="item" src="" class=" before after" data-expected-width="80" data-expected-height="80">
-height expected 80 but got 100
+PASS .after 2
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt (285997 => 285998)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,7 +1,5 @@
 
 
-FAIL .before 1 assert_equals:
-<img id="item" src="" class=" before" data-expected-width="80" data-expected-height="80">
-height expected 80 but got 100
+PASS .before 1
 PASS .after 2
 

Modified: trunk/LayoutTests/platform/win/TestExpectations (285997 => 285998)


--- trunk/LayoutTests/platform/win/TestExpectations	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/LayoutTests/platform/win/TestExpectations	2021-11-18 13:12:28 UTC (rev 285998)
@@ -3824,8 +3824,6 @@
 
 webkit.org/b/177216 fast/images/animated-image-mp4.html [ Skip ]
 
-webkit.org/b/228325 fast/images/exif-orientation-composited.html [ Pass ImageOnlyFailure ]
-
 webkit.org/b/177234 http/wpt/resource-timing/rt-cors.html [ Skip ]
 webkit.org/b/177234 http/wpt/resource-timing/rt-cors.worker.html [ Skip ]
 

Modified: trunk/Source/WebCore/ChangeLog (285997 => 285998)


--- trunk/Source/WebCore/ChangeLog	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/Source/WebCore/ChangeLog	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1,3 +1,29 @@
+2021-11-18  Ziran Sun  <[email protected]>
+
+        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
+        https://bugs.webkit.org/show_bug.cgi?id=228022
+
+        This is a re-land of r280078 and r282008. r280078 has caused a crash. Details on the crash test case is
+        at https://bugs.webkit.org/show_bug.cgi?id=232922. The reason for the crash is that, when recomputing the
+        intrinsic width we need to recompute the intrinsic width of the caller, which is the grid item, rather than
+        the grid in this case.
+        
+        This patch addresses the original issue and the cause for the crash.
+
+        When computing the logical height using an intrinsic aspect ratio, RenderReplaced should use the
+        overridingLogicalWidth whenever defined just as how it does for flex items. This change is to
+        replace the use of intrinsic (non-stretched) logical width in current code with the overridingLogicalWidth.
+
+        The current code doesn't set overringLogicalWidth while the width changes while stretches along Row Axis.
+        This change adds the logic in.
+
+        Reviewed by Javier Fernandez.
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
+        * rendering/RenderReplaced.cpp:
+        (WebCore::RenderReplaced::computeReplacedLogicalHeight const):
+
 2021-10-26  Carlos Garcia Campos  <[email protected]>
 
         [GTK][a11y] Add implementation of value interface when building with ATSPI

Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (285997 => 285998)


--- trunk/Source/WebCore/rendering/RenderGrid.cpp	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp	2021-11-18 13:12:28 UTC (rev 285998)
@@ -1183,12 +1183,15 @@
 void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child)
 {
     ASSERT(child.overridingContainingBlockContentLogicalHeight());
+    ASSERT(child.overridingContainingBlockContentLogicalWidth());
 
-    // We clear height override values because we will decide now whether it's allowed or
+    // We clear height and width override values because we will decide now whether it's allowed or
     // not, evaluating the conditions which might have changed since the old values were set.
     child.clearOverridingLogicalHeight();
+    child.clearOverridingLogicalWidth();
 
     GridTrackSizingDirection childBlockDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, ForRows);
+    GridTrackSizingDirection childInlineDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, ForColumns);
     bool blockFlowIsColumnAxis = childBlockDirection == ForRows;
     bool allowedToStretchChildBlockSize = blockFlowIsColumnAxis ? allowedToStretchChildAlongColumnAxis(child) : allowedToStretchChildAlongRowAxis(child);
     if (allowedToStretchChildBlockSize && !aspectRatioPrefersInline(child, blockFlowIsColumnAxis)) {
@@ -1204,7 +1207,13 @@
             child.setLogicalHeight(0_lu);
             child.setNeedsLayout(MarkOnlyThis);
         }
-    }
+    } else if (!allowedToStretchChildBlockSize && allowedToStretchChildAlongRowAxis(child)) {
+        LayoutUnit stretchedLogicalWidth = availableAlignmentSpaceForChildBeforeStretching(GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, childInlineDirection).value(), child);
+        LayoutUnit desiredLogicalWidth = child.constrainLogicalWidthInFragmentByMinMax(stretchedLogicalWidth, contentWidth(), *this, nullptr);
+        child.setOverridingLogicalWidth(desiredLogicalWidth);
+        if (desiredLogicalWidth != child.logicalWidth())
+            child.setNeedsLayout(MarkOnlyThis);
+    } 
 }
 
 // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.

Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (285997 => 285998)


--- trunk/Source/WebCore/rendering/RenderReplaced.cpp	2021-11-18 12:34:31 UTC (rev 285997)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp	2021-11-18 13:12:28 UTC (rev 285998)
@@ -623,7 +623,7 @@
     bool hasIntrinsicWidth = constrainedSize.hasIntrinsicWidth || constrainedSize.width() > 0;
 
     // See computeReplacedLogicalHeight() for a similar check for heights.
-    if (intrinsicRatio && isFlexItem() && hasOverridingLogicalWidth() && hasIntrinsicHeight && hasIntrinsicWidth)
+    if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalWidth() && hasIntrinsicHeight && hasIntrinsicWidth)
         return computeReplacedLogicalHeightRespectingMinMaxHeight(roundToInt(round(overridingContentLogicalWidth() / intrinsicRatio)));
 
     // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic height, then that intrinsic height is the used value of 'height'.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to