Skip to content
Permalink
Browse files
[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:


Canonical link: https://commits.webkit.org/194951@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223955 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
javifernandez committed Oct 25, 2017
1 parent d90dfa3 commit f9ad71d790869ec683cb4816be4b9a3b15d4cd2d
Showing 3 changed files with 46 additions and 24 deletions.
@@ -1,3 +1,34 @@
2017-10-25 Javier Fernandez <jfernandez@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.noronha@collabora.co.uk>

Unreviewed follow up changing one more enum value as discussed in the bug
@@ -742,11 +742,6 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::minContentForChild(RenderBox& child
{
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 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::maxContentForChild(RenderBox& child
{
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 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::minSizeForChild(RenderBox& child) c
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,16 +806,16 @@ class IndefiniteSizeStrategy final : public GridTrackSizingAlgorithmStrategy {
: 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;
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,18 +903,18 @@ class DefiniteSizeStrategy final : public GridTrackSizingAlgorithmStrategy {
: 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;
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)
@@ -228,11 +228,11 @@ class GridTrackSizingAlgorithmStrategy {
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(); }

0 comments on commit f9ad71d

Please sign in to comment.