The problem is that we're using a vector of indexes pointing to the vector with the list of affected tracks. Those indexes become easily incorrect because the first thing we do in distributeSpaceToTracks() is to sort the vector with the affected tracks by growth potential.
Created attachment 246644 [details] Patch
Ping reviewers.
Comment on attachment 246644 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246644&action=review > Source/WebCore/rendering/RenderGrid.cpp:82 > + LayoutUnit m_plannedSize { 0 }; Public data members are an anti-pattern we should strive to avoid. Is there some way to do this without having it be public? Even an accessor function that returns a reference is considered slightly better. > Source/WebCore/rendering/RenderGrid.cpp:685 > + Vector<GridTrack*>* tracksToGrowBeyondGrowthLimits = sizingData.growBeyondGrowthLimitsTracks.isEmpty() ? &sizingData.filteredTracks : &sizingData.growBeyondGrowthLimitsTracks; Seems we should use auto* or auto here instead of Vector<GridTrack*>*. > Source/WebCore/rendering/RenderGrid.h:58 > + typedef void (GridTrack::* AccumulatorGrowFunction)(LayoutUnit); I don’t see anything in this patch that requires that this type be public. What did I miss?
Comment on attachment 246644 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246644&action=review >> Source/WebCore/rendering/RenderGrid.cpp:82 >> + LayoutUnit m_plannedSize { 0 }; > > Public data members are an anti-pattern we should strive to avoid. Is there some way to do this without having it be public? Even an accessor function that returns a reference is considered slightly better. I know but the code with getters/setters was full of boilerplate. I thought about returning a reference but didn't consider it was specially better. >> Source/WebCore/rendering/RenderGrid.cpp:685 >> + Vector<GridTrack*>* tracksToGrowBeyondGrowthLimits = sizingData.growBeyondGrowthLimitsTracks.isEmpty() ? &sizingData.filteredTracks : &sizingData.growBeyondGrowthLimitsTracks; > > Seems we should use auto* or auto here instead of Vector<GridTrack*>*. Yep. >> Source/WebCore/rendering/RenderGrid.h:58 >> + typedef void (GridTrack::* AccumulatorGrowFunction)(LayoutUnit); > > I don’t see anything in this patch that requires that this type be public. What did I miss? You're right. Somehow I slipped it in.
Committed r180623: <http://trac.webkit.org/changeset/180623>