Based on the spec [1][2], it seems the repeat() notation is resolved in resolved values, instead of computed track list. The current Gecko expands repeat() (with numeric repeat count) in ToComputedValue [3]. This is incorrect, and will have impact on interpolation between repeat()s.

[1] https://drafts.csswg.org/css-grid/#computed-track-list
[2] https://drafts.csswg.org/css-grid/#resolved-track-list
[3] https://searchfox.org/mozilla-central/rev/b29663c6c9c61b0bf29e8add490cbd6bad293a67/servo/components/style/values/specified/grid.rs#327

BTW, there is a discussion about interpolating repeat(): https://github.com/w3c/csswg-drafts/issues/3503

The spec issue, https://github.com/w3c/csswg-drafts/issues/3503, is resolved by a more restrictive rule for now:

Define repeat interpolation using the more restrictive rule in https://github.com//issues/3503#issuecomment-453674839

It could be loosen, but for now we should follow the restrictive rule: require that the first arg is a number, and identical between start/end; different values, or keyword values, both go discrete

That means we have to fix this bug to match the updated spec and fix the wpts [1]/[2], which uses a loosen way if the first arg is a number (because Gecko uses an incorrect computed value of repeat()).

[1] https://searchfox.org/mozilla-central/rev/39265dd58298c40bed029617ad408bf806cce761/testing/web-platform/tests/css/css-grid/animation/grid-template-columns-interpolation.html#133-144
[2] https://searchfox.org/mozilla-central/rev/39265dd58298c40bed029617ad408bf806cce761/testing/web-platform/tests/css/css-grid/animation/grid-template-rows-interpolation.html#133-144

It seems the biggest problem is: nsStyleCoord doesn't support general function type (i.e. only calc() is acceptable), and we use m{Max|Min}TrackSizingFunctions to represent all the different types of <track-size>, but not <track-repeat> (i.e. repeat() notation). In order to make sure we keep repeat() notation in the computed value, we have to

1. Do not expand repeat() in ToComputedValue
2. Redesign nsStyleGridTemplate so it could represent <track-size>|<track-repeat>_
3. Update the conversion code in gekco.mako.rs, and then update all the layout code to use the new type. (And maybe need to resolve it by adding an utility function)

Looks like we have a lot of things to do.

(In reply to Boris Chiou [:boris] from comment #1)

fix the wpts [1]/[2], which uses a loosen way if the first arg is a number (because Gecko uses an incorrect computed value of repeat()).

[1] https://searchfox.org/mozilla-central/rev/39265dd58298c40bed029617ad408bf806cce761/testing/web-platform/tests/css/css-grid/animation/grid-template-columns-interpolation.html#133-144
[2] https://searchfox.org/mozilla-central/rev/39265dd58298c40bed029617ad408bf806cce761/testing/web-platform/tests/css/css-grid/animation/grid-template-rows-interpolation.html#133-144

Updated spec: https://drafts.csswg.org/css-grid/#repeat-interpolation

I have a wip patch that removes nsStyleGridTemplate and does this while at it. Still needs a bunch of work, but if you're not actively working on this mind if I take it?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

I have a wip patch that removes nsStyleGridTemplate and does this while at it. Still needs a bunch of work, but if you're not actively working on this mind if I take it?

Sure. Please feel free to take this. (It’s better to fix this together with using cbindgen.)

Cool, thanks!

While we're at it we may as well do this.

Depends on D36349

Comment on attachment 9074825 [details]
Bug 1519958 - Drive by, remove unused CSS keywords. r=#style,boris

Revision D36350 was moved to bug 1562269. Setting attachment 9074825 [details] to obsolete.

Ouch, landed with the wrong bug # :(

https://hg.mozilla.org/mozilla-central/rev/3669b147ccc6

I'm really sorry for the size of the patch. I tried to do this in two steps
but it was a lot of work and pretty ugly.

This patch makes us use cbindgen for grid-template-{rows,columns}, in order to:

• Make us preserve repeat() at computed-value time. This is per spec since
  interpolation needs to know about repeat(). Except for subgrid, which did the
  repeat expansion at parse-time and was a bit more annoying (plus it doesn't
  really animate yet so we don't need it to comply with the spec).

• Tweaks the WPT tests for interpolation to adopt the resolution at:
  https://github.com/w3c/csswg-drafts/issues/3503.

Trade-off here, as this patch stands, is that this change makes us use less
long-living memory, since we expand repeat() during layout, but at the cost of a
bit of CPU time during layout (conditional on the property applying though,
which wasn't the case before). It should be very easy to store a cached version
of the template, should this be too hot (I expect it isn't), or to change the
representation in other ways to optimize grid layout code if it's worth it.

Another trade-off: I've used SmallPointerArray to handle line-name merging,
pointing to the individual arrays in the style data, rather than actually
heap-allocating the merged lists. This would also be pretty easy to change
should we measure and see that it's not worth it.

This patch also opens the gate to potentially improving memory usage in some
other ways, by reference-counting line-name lists for example, though I don't
have data that suggests it is worth it.

In general, this patch makes much easier to tweak the internal representation of
the grid style data structures. Overall, I think it's a win, the amount of magic
going on in that mako code was a bit huge; it took a bit to wrap my head around
it.

This patch comments out the style struct size assertions. They will be
uncommented in a follow-up patch which contains some improvements for this type,
which are worth getting reviewed separately.

Also, this patch doesn't remove as much code as I would've hoped for because of
I tried not to change most of the dom/grid code for inspector, but I think a
fair bit of the nsGridContainerFrame.cpp code that collects information for it
can be simplified / de-copy-pasted to some extent. But that was a pre-existing
problem and this patch is already quite massive.

This re-enables the assertion which was disabled on the previous patch by doing
a bit of boxing around.

Depends on D36598

https://hg.mozilla.org/mozilla-central/rev/a1eb75e785de
https://hg.mozilla.org/mozilla-central/rev/786ba5f970ac
https://hg.mozilla.org/mozilla-central/rev/6e82b588cd77