Bug 112361

Summary: [CSS Grid Layout] computedUsedBreadthOfGridTracks doesn't fill all the grid tracks
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ojan, tony, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 60731    

Description Julien Chaffraix 2013-03-14 10:32:05 PDT
The following code can be found in computedUsedBreadthOfGridTracks:

const Vector<GridTrackSize>& trackStyles = (direction == ForColumns) ? style()->gridColumns() : style()->gridRows();
[...]
Vector<GridTrack>& tracks = (direction == ForColumns) ? columnTracks : rowTracks;
for (size_t i = 0; i < trackStyles.size(); ++i) {
    GridTrack& track = tracks[i];
    const Length& minTrackBreadth = trackStyles[i].minTrackBreadth();
    const Length& maxTrackBreadth = trackStyles[i].maxTrackBreadth();
    [...]
}

While this will never read past |tracks| end, it's wrong as implicit rows / columns can make tracks.size() > trackStyles.size(). This means that implicit rows / columns are not set up properly prior to running the algorithm.

The loop should actually be:

for (size_t i = 0; i < tracks.size(); ++i) {
    GridTrack& track = tracks[i];
    const GridTrackSize& trackSize = gridTrackSize(i, direction);
    const Length& minTrackBreadth = trackSize.minTrackBreadth();
    const Length& maxTrackBreadth = trackSize.maxTrackBreadth();
    [...]

As a side note, we should investigate all the call sites iterating over the style's gridRows() / gridColumns() as they are probably wrong.
Comment 1 Julien Chaffraix 2013-03-14 13:30:36 PDT
I am not sure this bug is visible now. That's because we allow a grid track's usedbreadth to go above its maxbreadth if there is some max-content max sizing function, which matches the specification. This is used e.g. to handle the case where the max sizing function is smaller than the min sizing function.

Once we have implemented the default implicit sizing (bug 103333), it should be trivial to come up with a test case.
Comment 2 Julien Chaffraix 2013-03-21 14:18:42 PDT
This was fixed when implementing bug 103333 as it was required to pass the tests. The only call site using the style information is RenderGrid::maximumIndexInDirection which is legit as it estimates the grid size based on the style information.