[webkit-changes] [223955] trunk/Source/WebCore

Wed, 25 Oct 2017 09:39:07 -0700

Title: [223955] trunk/Source/WebCore
Revision
223955
Author
jfernan...@igalia.com
Date
2017-10-25 09:38:14 -0700 (Wed, 25 Oct 2017)

Log Message

[css-grid] Avoid clearing the overrideContainingBlockWidth if possible
https://bugs.webkit.org/show_bug.cgi?id=178260

Reviewed by Sergio Villar Senin.

Since the intrinsic width computation uses the same logic than the
track sizing algorithm we are clearing the overrideContainingBlockWidth
of some grid items that are required to laid out them properly.

It's very uncommon that any intrinsic size computation isn't performed
as part of a layout process. However, if it happens, once cleared the
overrideContainingBlockWidth it may lead to an incorrect layout of the
affected grid items.

This change is a defensive approach to avoid the issues caused by
such off-layout preferred size requests, which may imply recomputing
the grid container intrinsic size.

No new tests, because we are only removing some redundant logic.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
(WebCore::GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild const):
(WebCore::IndefiniteSizeStrategy::minLogicalWidthForChild const):
(WebCore::DefiniteSizeStrategy::minLogicalWidthForChild const):
* rendering/GridTrackSizingAlgorithm.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (223954 => 223955)


--- trunk/Source/WebCore/ChangeLog	2017-10-25 15:11:07 UTC (rev 223954)
+++ trunk/Source/WebCore/ChangeLog	2017-10-25 16:38:14 UTC (rev 223955)
@@ -1,3 +1,34 @@
+2017-10-25  Javier Fernandez  <jfernan...@igalia.com>
+
+        [css-grid] Avoid clearing the overrideContainingBlockWidth if possible
+        https://bugs.webkit.org/show_bug.cgi?id=178260
+
+        Reviewed by Sergio Villar Senin.
+
+        Since the intrinsic width computation uses the same logic than the
+        track sizing algorithm we are clearing the overrideContainingBlockWidth
+        of some grid items that are required to laid out them properly.
+
+        It's very uncommon that any intrinsic size computation isn't performed
+        as part of a layout process. However, if it happens, once cleared the
+        overrideContainingBlockWidth it may lead to an incorrect layout of the
+        affected grid items.
+
+        This change is a defensive approach to avoid the issues caused by
+        such off-layout preferred size requests, which may imply recomputing
+        the grid container intrinsic size.
+
+        No new tests, because we are only removing some redundant logic.
+
+        * rendering/GridTrackSizingAlgorithm.cpp:
+        (WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild const):
+        (WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild const):
+        (WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
+        (WebCore::GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild const):
+        (WebCore::IndefiniteSizeStrategy::minLogicalWidthForChild const):
+        (WebCore::DefiniteSizeStrategy::minLogicalWidthForChild const):
+        * rendering/GridTrackSizingAlgorithm.h:
+
 2017-10-25  Gustavo Noronha Silva  <gustavo.noro...@collabora.co.uk>
 
         Unreviewed follow up changing one more enum value as discussed in the bug

Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp (223954 => 223955)


--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2017-10-25 15:11:07 UTC (rev 223954)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2017-10-25 16:38:14 UTC (rev 223955)
@@ -742,11 +742,6 @@
 {
     GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(renderGrid(), child, ForColumns);
     if (direction() == childInlineDirection) {
-        // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
-        // what we are interested in here. Thus we need to set the override logical width to std::nullopt (no possible resolution).
-        if (shouldClearOverrideContainingBlockContentSizeForChild(child, ForColumns))
-            setOverrideContainingBlockContentSizeForChild(child, childInlineDirection, std::nullopt);
-
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         LayoutUnit marginLogicalWidth = child.needsLayout() ? computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child) : child.marginLogicalWidth();
@@ -762,11 +757,6 @@
 {
     GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(renderGrid(), child, ForColumns);
     if (direction() == childInlineDirection) {
-        // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
-        // what we are interested in here. Thus we need to set the inline-axis override size to -1 (no possible resolution).
-        if (shouldClearOverrideContainingBlockContentSizeForChild(child, ForColumns))
-            setOverrideContainingBlockContentSizeForChild(child, childInlineDirection, std::nullopt);
-
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         LayoutUnit marginLogicalWidth = child.needsLayout() ? computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child) : child.marginLogicalWidth();
@@ -789,19 +779,20 @@
     if (!childSize.isAuto() || (childMinSize.isAuto() && overflowIsVisible))
         return minContentForChild(child);
 
-    bool overrideSizeHasChanged = updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection);
-
+    LayoutUnit gridAreaSize = m_algorithm.gridAreaBreadthForChild(child, childInlineDirection);
     if (isRowAxis)
-        return minLogicalWidthForChild(child, childMinSize, childInlineDirection);
+        return minLogicalWidthForChild(child, childMinSize, gridAreaSize);
 
+    bool overrideSizeHasChanged = updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection, gridAreaSize);
     layoutGridItemForMinSizeComputation(child, overrideSizeHasChanged);
 
     return child.computeLogicalHeightUsing(MinSize, childMinSize, std::nullopt).value_or(0) + child.marginLogicalHeight() + child.scrollbarLogicalHeight();
 }
 
-bool GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild(RenderBox& child, GridTrackSizingDirection direction) const
+bool GridTrackSizingAlgorithmStrategy::updateOverrideContainingBlockContentSizeForChild(RenderBox& child, GridTrackSizingDirection direction, std::optional<LayoutUnit> overrideSize) const
 {
-    LayoutUnit overrideSize = m_algorithm.gridAreaBreadthForChild(child, direction);
+    if (!overrideSize)
+        overrideSize = m_algorithm.gridAreaBreadthForChild(child, direction);
     if (hasOverrideContainingBlockContentSizeForChild(child, direction) && overrideContainingBlockContentSizeForChild(child, direction) == overrideSize)
         return false;
 
@@ -815,7 +806,7 @@
         : GridTrackSizingAlgorithmStrategy(algorithm) { }
 
 private:
-    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const override;
+    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const override;
     void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const override;
     void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
     double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
@@ -822,9 +813,9 @@
     bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
 };
 
-LayoutUnit IndefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, GridTrackSizingDirection childInlineDirection) const
+LayoutUnit IndefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
 {
-    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, overrideContainingBlockContentSizeForChild(child, childInlineDirection).value_or(0), *renderGrid(), nullptr) + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
+    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, availableSize, *renderGrid(), nullptr) + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
 }
 
 void IndefiniteSizeStrategy::layoutGridItemForMinSizeComputation(RenderBox& child, bool overrideSizeHasChanged) const
@@ -912,7 +903,7 @@
         : GridTrackSizingAlgorithmStrategy(algorithm) { }
 
 private:
-    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const override;
+    LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const override;
     void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const override;
     void maximizeTracks(Vector<GridTrack>&, std::optional<LayoutUnit>& freeSpace) override;
     double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
@@ -919,11 +910,11 @@
     bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
 };
 
-LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, GridTrackSizingDirection childInlineDirection) const
+LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
 {
     LayoutUnit marginLogicalWidth =
-        computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child);
-    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, overrideContainingBlockContentSizeForChild(child, childInlineDirection).value_or(0), *renderGrid(), nullptr) + marginLogicalWidth;
+        computeMarginLogicalSizeForChild(flowAwareDirectionForChild(renderGrid(), child, ForColumns), *renderGrid(), child);
+    return child.computeLogicalWidthInFragmentUsing(MinSize, childMinSize, availableSize, *renderGrid(), nullptr) + marginLogicalWidth;
 }
 
 void DefiniteSizeStrategy::maximizeTracks(Vector<GridTrack>& tracks, std::optional<LayoutUnit>& freeSpace)

Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h (223954 => 223955)


--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h	2017-10-25 15:11:07 UTC (rev 223954)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h	2017-10-25 16:38:14 UTC (rev 223955)
@@ -228,11 +228,11 @@
     GridTrackSizingAlgorithmStrategy(GridTrackSizingAlgorithm& algorithm)
         : m_algorithm(algorithm) { }
 
-    virtual LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, GridTrackSizingDirection) const = 0;
+    virtual LayoutUnit minLogicalWidthForChild(RenderBox&, Length childMinSize, LayoutUnit availableSize) const = 0;
     virtual void layoutGridItemForMinSizeComputation(RenderBox&, bool overrideSizeHasChanged) const = 0;
 
     LayoutUnit logicalHeightForChild(RenderBox&) const;
-    bool updateOverrideContainingBlockContentSizeForChild(RenderBox&, GridTrackSizingDirection) const;
+    bool updateOverrideContainingBlockContentSizeForChild(RenderBox&, GridTrackSizingDirection, std::optional<LayoutUnit> = std::nullopt) const;
 
     // GridTrackSizingAlgorithm accessors for subclasses.
     LayoutUnit computeTrackBasedSize() const { return m_algorithm.computeTrackBasedSize(); }

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to