Skip to content

Commit

Permalink
Cherry-pick 252432.1044@safari-7614-branch (22cbd76). rdar://104559684
Browse files Browse the repository at this point in the history
    Invalidate grid placement when style changes to subgrid
    rdar://104559684

    Reviewed by Jonathan Bedard and Matt Woodrow.

    Before this change, we didn't invalidate parent and child placement
    info, leading to a OOB read into the parent tracks information when
    copying that to the child. This change fixes that.

    * LayoutTests/fast/css-grid-layout/grid-stylechange-crash-expected.txt: Added.
    * LayoutTests/fast/css-grid-layout/grid-stylechange-crash.html: Added.
    * Source/WebCore/rendering/RenderGrid.cpp:
    (WebCore::RenderGrid::styleDidChange):
    (WebCore::RenderGrid::subgridDidChange const):
    (WebCore::RenderGrid::dirtyGrid):
    * Source/WebCore/rendering/RenderGrid.h:
    * Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:
    (WebCore::GridTrackSizingAlgorithm::copyUsedTrackSizesForSubgrid):

    Canonical link: https://commits.webkit.org/252432.1044@safari-7614-branch
  • Loading branch information
chirags27 authored and aperezdc committed Apr 3, 2023
1 parent 6d228e9 commit 2cba805
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 4 deletions.
@@ -0,0 +1,2 @@
PASS if no crash.

17 changes: 17 additions & 0 deletions LayoutTests/fast/css-grid-layout/grid-stylechange-crash.html
@@ -0,0 +1,17 @@
<script>
function test() {
if(window.testRunner)
testRunner.dumpAsText()
document.body.offsetTop;
x.style.gridTemplateRows = 'subgrid';
};
</script>
<body onload="test()">
<div style="display:grid">
<div id="x" style="display:grid">
PASS if no crash.
<div>
</div>
</div>
</div>
</body>
1 change: 1 addition & 0 deletions Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp
Expand Up @@ -1615,6 +1615,7 @@ bool GridTrackSizingAlgorithm::copyUsedTrackSizesForSubgrid()
GridSpan span = outer->gridSpanForChild(*m_renderGrid, direction);
Vector<GridTrack>& allTracks = tracks(m_direction);
int numTracks = allTracks.size();
RELEASE_ASSERT((parentTracks.size() - 1) >= (numTracks - 1 + span.startLine()));
for (int i = 0; i < numTracks; i++)
allTracks[i] = parentTracks[i + span.startLine()];

Expand Down
21 changes: 18 additions & 3 deletions Source/WebCore/rendering/RenderGrid.cpp
Expand Up @@ -106,9 +106,16 @@ void RenderGrid::styleDidChange(StyleDifference diff, const RenderStyle* oldStyl
}
}

auto subgridChanged = subgridDidChange(*oldStyle);
if (explicitGridDidResize(*oldStyle) || namedGridLinesDefinitionDidChange(*oldStyle) || implicitGridLinesDefinitionDidChange(*oldStyle) || oldStyle->gridAutoFlow() != style().gridAutoFlow()
|| (style().gridAutoRepeatColumns().size() || style().gridAutoRepeatRows().size()))
dirtyGrid();
|| (style().gridAutoRepeatColumns().size() || style().gridAutoRepeatRows().size()) || subgridChanged)
dirtyGrid(subgridChanged);
}

bool RenderGrid::subgridDidChange(const RenderStyle& oldStyle) const
{
return oldStyle.gridSubgridRows() != style().gridSubgridRows()
|| oldStyle.gridSubgridColumns() != style().gridSubgridColumns();
}

bool RenderGrid::explicitGridDidResize(const RenderStyle& oldStyle) const
Expand Down Expand Up @@ -1190,12 +1197,20 @@ GridTrackSizingDirection RenderGrid::autoPlacementMinorAxisDirection() const
return (autoPlacementMajorAxisDirection() == ForColumns) ? ForRows : ForColumns;
}

void RenderGrid::dirtyGrid()
void RenderGrid::dirtyGrid(bool subgridChanged)
{
if (currentGrid().needsItemsPlacement())
return;

currentGrid().setNeedsItemsPlacement(true);

auto currentChild = this;
while (currentChild && (subgridChanged || currentChild->isSubgridRows() || currentChild->isSubgridColumns())) {
currentChild = dynamicDowncast<RenderGrid>(currentChild->parent());
if (currentChild)
currentChild->currentGrid().setNeedsItemsPlacement(true);
subgridChanged = false;
}
}

Vector<LayoutUnit> RenderGrid::trackSizesForComputedStyle(GridTrackSizingDirection direction) const
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/rendering/RenderGrid.h
Expand Up @@ -62,7 +62,7 @@ class RenderGrid final : public RenderBlock {
bool avoidsFloats() const override { return true; }
bool canDropAnonymousBlockChild() const override { return false; }

void dirtyGrid();
void dirtyGrid(bool subgridChanged = false);
Vector<LayoutUnit> trackSizesForComputedStyle(GridTrackSizingDirection) const;

const Vector<LayoutUnit>& columnPositions() const { return m_columnPositions; }
Expand Down Expand Up @@ -144,6 +144,7 @@ class RenderGrid final : public RenderBlock {
bool selfAlignmentChangedToStretch(GridAxis, const RenderStyle& oldStyle, const RenderStyle& newStyle, const RenderBox&) const;
bool selfAlignmentChangedFromStretch(GridAxis, const RenderStyle& oldStyle, const RenderStyle& newStyle, const RenderBox&) const;

bool subgridDidChange(const RenderStyle& oldStyle) const;
bool explicitGridDidResize(const RenderStyle&) const;
bool namedGridLinesDefinitionDidChange(const RenderStyle&) const;
bool implicitGridLinesDefinitionDidChange(const RenderStyle&) const;
Expand Down

0 comments on commit 2cba805

Please sign in to comment.