Auto merge of #18206 - ferjm:bug1382369.grid.repeat.function, r=wafflespeanut

stylo: store specified value of grid layout repeat() function

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1382369](https://bugzilla.mozilla.org/show_bug.cgi?id=1382369)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18206)
<!-- Reviewable:end -->

components/style/gecko/conversions.rs

@@ -20,7 +20,7 @@ use stylesheets::{Origin, RulesMutateError};
 use values::computed::{Angle, CalcLengthOrPercentage, Gradient, Image};
 use values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto, Percentage};
 use values::generics::box_::VerticalAlign;
-use values::generics::grid::TrackSize;
+use values::generics::grid::{TrackListValue, TrackSize};
 use values::generics::image::{CompatMode, Image as GenericImage, GradientItem};
 use values::generics::rect::Rect;
 use values::specified::url::SpecifiedUrl;
@@ -896,6 +896,23 @@ impl TrackSize<LengthOrPercentage> {
     }
 }
 
+impl TrackListValue<LengthOrPercentage> {
+    /// Return TrackSize from given two nsStyleCoord
+    pub fn from_gecko_style_coords<T: CoordData>(gecko_min: &T, gecko_max: &T) -> Self {
+        TrackListValue::TrackSize(TrackSize::from_gecko_style_coords(gecko_min, gecko_max))
+    }
+
+    /// Save TrackSize to given gecko fields.
+    pub fn to_gecko_style_coords<T: CoordDataMut>(&self, gecko_min: &mut T, gecko_max: &mut T) {
+        use values::generics::grid::TrackListValue;
+
+        match *self {
+            TrackListValue::TrackSize(ref size) => size.to_gecko_style_coords(gecko_min, gecko_max),
+            _ => unreachable!("Should only transform from track-size computed values"),
+        }
+    }
+}
+
 impl<T> Rect<T> where T: GeckoStyleCoordConvertible {
     /// Convert this generic Rect to given Gecko fields.
     pub fn to_gecko_rect(&self, sides: &mut ::gecko_bindings::structs::nsStyleSides) {

components/style/properties/gecko.mako.rs

@@ -1895,7 +1895,7 @@ fn static_assert() {
         use nsstring::nsStringRepr;
         use values::CustomIdent;
         use values::generics::grid::{GridTemplateComponent, LineNameList, RepeatCount};
-        use values::generics::grid::{TrackList, TrackListType, TrackRepeat, TrackSize};
+        use values::generics::grid::{TrackList, TrackListType, TrackListValue, TrackRepeat, TrackSize};
 
         let value = match unsafe { ${self_grid}.mPtr.as_ref() } {
             None => return GridTemplateComponent::None,
@@ -1965,7 +1965,7 @@ fn static_assert() {
 
                     auto_repeat = Some(TrackRepeat{count, line_names, track_sizes});
                 } else {
-                    values.push(track_size);
+                    values.push(TrackListValue::TrackSize(track_size));
                 }
             }
 

components/style/properties/shorthand/position.mako.rs

@@ -243,7 +243,8 @@
     use parser::Parse;
     use properties::longhands::grid_template_areas::TemplateAreas;
     use values::{Either, None_};
-    use values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType, concat_serialize_idents};
+    use values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType};
+    use values::generics::grid::{TrackListValue, concat_serialize_idents};
     use values::specified::{GridTemplateComponent, GenericGridTemplateComponent};
     use values::specified::grid::parse_line_names;
 
@@ -287,7 +288,7 @@
                 line_names.push(names.into_boxed_slice());
                 strings.push(string);
                 let size = input.try(|i| TrackSize::parse(context, i)).unwrap_or_default();
-                values.push(size);
+                values.push(TrackListValue::TrackSize(size));
                 names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()).into_vec();
                 if let Ok(v) = input.try(parse_line_names) {
                     names.extend(v.into_vec());
@@ -380,7 +381,11 @@
                     GenericGridTemplateComponent::TrackList(ref list) => {
                         // We should fail if there is a `repeat` function. `grid` and
                         // `grid-template` shorthands doesn't accept that. Only longhand accepts.
-                        if list.auto_repeat.is_some() {
+                        if list.auto_repeat.is_some() ||
+                           list.values.iter().any(|v| match *v {
+                               TrackListValue::TrackRepeat(_) => true,
+                               _ => false,
+                           }) {
                             return Ok(());
                         }
                         list
@@ -395,8 +400,14 @@
                 match *template_columns {
                     // We should fail if there is a `repeat` function. `grid` and
                     // `grid-template` shorthands doesn't accept that. Only longhand accepts that.
-                    GenericGridTemplateComponent::TrackList(ref list) if list.auto_repeat.is_some() => {
+                    GenericGridTemplateComponent::TrackList(ref list) => {
-                        return Ok(());
+                        if list.auto_repeat.is_some() ||
+                           list.values.iter().any(|v| match *v {
+                               TrackListValue::TrackRepeat(_) => true,
+                               _ => false,
+                           }) {
+                            return Ok(());
+                        }
                     },
                     // Also the shorthands don't accept subgrids unlike longhand.
                     // We should fail without an error here.
@@ -407,9 +418,9 @@
                 }
 
                 let mut names_iter = track_list.line_names.iter();
-                for (((i, string), names), size) in areas.strings.iter().enumerate()
+                for (((i, string), names), value) in areas.strings.iter().enumerate()
-                                                                 .zip(&mut names_iter)
+                                                                  .zip(&mut names_iter)
-                                                                 .zip(track_list.values.iter()) {
+                                                                  .zip(track_list.values.iter()) {
                     if i > 0 {
                         dest.write_str(" ")?;
                     }
@@ -420,7 +431,7 @@
 
                     string.to_css(dest)?;
                     dest.write_str(" ")?;
-                    size.to_css(dest)?;
+                    value.to_css(dest)?;
                 }
 
                 if let Some(names) = names_iter.next() {

components/style/values/generics/grid.rs

@@ -394,45 +394,27 @@ pub struct TrackRepeat<L> {
 
 impl<L: ToCss> ToCss for TrackRepeat<L> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        // If repeat count is an integer instead of a keyword, it should'n serialized
+        dest.write_str("repeat(")?;
-        // with `repeat` function. It should serialized with `N` repeated form.
+        self.count.to_css(dest)?;
-        let repeat_count = match self.count {
+        dest.write_str(", ")?;
-            RepeatCount::Number(integer) => integer.value(),
-            _ => {
-                dest.write_str("repeat(")?;
-                self.count.to_css(dest)?;
-                dest.write_str(", ")?;
-                1
-            },
-        };
 
-        for i in 0..repeat_count {
+        let mut line_names_iter = self.line_names.iter();
-            if i != 0 {
+        for (i, (ref size, ref names)) in self.track_sizes.iter()
+                                              .zip(&mut line_names_iter).enumerate() {
+            if i > 0 {
                 dest.write_str(" ")?;
             }
 
-            let mut line_names_iter = self.line_names.iter();
+            concat_serialize_idents("[", "] ", names, " ", dest)?;
-            for (i, (ref size, ref names)) in self.track_sizes.iter()
+            size.to_css(dest)?;
-                                                  .zip(&mut line_names_iter).enumerate() {
-                if i > 0 {
-                    dest.write_str(" ")?;
-                }
-
-                concat_serialize_idents("[", "] ", names, " ", dest)?;
-                size.to_css(dest)?;
-            }
-
-            if let Some(line_names_last) = line_names_iter.next() {
-                concat_serialize_idents(" [", "]", line_names_last, " ", dest)?;
-            }
         }
 
-        match self.count {
+        if let Some(line_names_last) = line_names_iter.next() {
-            RepeatCount::AutoFill | RepeatCount::AutoFit => {
+            concat_serialize_idents(" [", "]", line_names_last, " ", dest)?;
-                dest.write_str(")")?;
-            },
-            _ => {},
         }
+
+        dest.write_str(")")?;
+
         Ok(())
     }
 }
@@ -475,6 +457,16 @@ impl<L: Clone> TrackRepeat<L> {
     }
 }
 
+/// Track list values. Can be <track-size> or <track-repeat>
+#[derive(Clone, Debug, PartialEq, ToComputedValue, ToCss)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+pub enum TrackListValue<T> {
+    /// A <track-size> value.
+    TrackSize(TrackSize<T>),
+    /// A <track-repeat> value.
+    TrackRepeat(TrackRepeat<T>),
+}
+
 /// The type of a `<track-list>` as determined during parsing.
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-track-list
@@ -504,21 +496,20 @@ impl ComputedValueAsSpecified for TrackListType {}
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-track-list
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Clone, Debug, PartialEq, ToComputedValue)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct TrackList<T> {
     /// The type of this `<track-list>` (auto, explicit or general).
     ///
     /// In order to avoid parsing the same value multiple times, this does a single traversal
     /// and arrives at the type of value it has parsed (or bails out gracefully with an error).
     pub list_type: TrackListType,
-    /// A vector of `<track-size>` values.
+    /// A vector of `<track-size> | <track-repeat>` values.
-    pub values: Vec<TrackSize<T>>,
+    pub values: Vec<TrackListValue<T>>,
     /// `<line-names>` accompanying `<track-size> | <track-repeat>` values.
     ///
     /// If there's no `<line-names>`, then it's represented by an empty vector.
     /// For N values, there will be N+1 `<line-names>`, and so this vector's
     /// length is always one value more than that of the `<track-size>`.
-    #[compute(clone)]
     pub line_names: Box<[Box<[CustomIdent]>]>,
     /// `<auto-repeat>` value. There can only be one `<auto-repeat>` in a TrackList.
     pub auto_repeat: Option<TrackRepeat<T>>,

components/style/values/specified/grid.rs

@@ -11,8 +11,9 @@ use std::ascii::AsciiExt;
 use std::mem;
 use style_traits::{ParseError, StyleParseError};
 use values::{CSSFloat, CustomIdent};
+use values::computed::{self, Context, ToComputedValue};
 use values::generics::grid::{GridTemplateComponent, RepeatCount, TrackBreadth, TrackKeyword, TrackRepeat};
-use values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType};
+use values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType, TrackListValue};
 use values::specified::LengthOrPercentage;
 
 /// Parse a single flexible length.
@@ -107,7 +108,7 @@ impl TrackRepeat<LengthOrPercentage> {
                 let is_auto = count == RepeatCount::AutoFit || count == RepeatCount::AutoFill;
                 let mut repeat_type = if is_auto {
                     RepeatType::Auto
-                } else {    // <fixed-size> is a subset of <track_size>, so it should work for both
+                } else {    // <fixed-size> is a subset of <track-size>, so it should work for both
                     RepeatType::Fixed
                 };
 
@@ -166,17 +167,6 @@ impl TrackRepeat<LengthOrPercentage> {
 
 impl Parse for TrackList<LengthOrPercentage> {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        // Merge the line names while parsing values. The resulting values will
-        // all be bunch of `<track-size>` and one <auto-repeat>.
-        // FIXME: We need to decide which way is better for repeat function in
-        // https://bugzilla.mozilla.org/show_bug.cgi?id=1382369.
-        //
-        // For example,
-        // `[a b] 100px [c d] repeat(1, 30px [g]) [h]` will be merged as `[a b] 100px [c d] 30px [g h]`
-        //  whereas, `[a b] repeat(2, [c] 50px [d]) [e f] repeat(auto-fill, [g] 12px) 10px [h]` will be merged as
-        // `[a b c] 50px [d c] 50px [d e f] repeat(auto-fill, [g] 12px) 10px [h]`, with the `<auto-repeat>` value
-        // set in the `auto_repeat` field, and the `idx` in TrackListType::Auto pointing to the values after
-        // `<auto-repeat>` (in this case, `10px [h]`).
         let mut current_names = vec![];
         let mut names = vec![];
         let mut values = vec![];
@@ -184,9 +174,14 @@ impl Parse for TrackList<LengthOrPercentage> {
         let mut list_type = TrackListType::Explicit;    // assume it's the simplest case
         // holds <auto-repeat> value. It can only be only one in a TrackList.
         let mut auto_repeat = None;
+        // if there is any <auto-repeat> the list will be of type TrackListType::Auto(idx)
+        // where idx points to the position of the <auto-repeat> in the track list. If there
+        // is any repeat before <auto-repeat>, we need to take the number of repetitions into
+        // account to set the position of <auto-repeat> so it remains the same while computing
+        // values.
+        let mut auto_offset = 0;
         // assume that everything is <fixed-size>. This flag is useful when we encounter <auto-repeat>
         let mut atleast_one_not_fixed = false;
-
         loop {
             current_names.extend_from_slice(&mut input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()));
             if let Ok(track_size) = input.try(|i| TrackSize::parse(context, i)) {
@@ -200,7 +195,7 @@ impl Parse for TrackList<LengthOrPercentage> {
 
                 let vec = mem::replace(&mut current_names, vec![]);
                 names.push(vec.into_boxed_slice());
-                values.push(track_size);
+                values.push(TrackListValue::TrackSize(track_size));
             } else if let Ok((repeat, type_)) = input.try(|i| TrackRepeat::parse_with_repeat_type(context, i)) {
                 if list_type == TrackListType::Explicit {
                     list_type = TrackListType::Normal;      // <explicit-track-list> doesn't contain repeat()
@@ -219,28 +214,21 @@ impl Parse for TrackList<LengthOrPercentage> {
                             return Err(StyleParseError::UnspecifiedError.into())
                         }
 
-                        list_type = TrackListType::Auto(values.len() as u16);
+                        list_type = TrackListType::Auto(values.len() as u16 + auto_offset);
                         auto_repeat = Some(repeat);
                         let vec = mem::replace(&mut current_names, vec![]);
                         names.push(vec.into_boxed_slice());
-                        continue
+                        continue;
                     },
                     RepeatType::Fixed => (),
                 }
 
-                // If the repeat count is numeric, we axpand and merge the values.
+                let vec = mem::replace(&mut current_names, vec![]);
-                let mut repeat = repeat.expand();
+                names.push(vec.into_boxed_slice());
-                let mut repeat_names_iter = repeat.line_names.iter();
+                if let RepeatCount::Number(num) = repeat.count {
-                for (size, repeat_names) in repeat.track_sizes.drain(..).zip(&mut repeat_names_iter) {
+                    auto_offset += (num.value() - 1) as u16;
-                    current_names.extend_from_slice(&repeat_names);
-                    let vec = mem::replace(&mut current_names, vec![]);
-                    names.push(vec.into_boxed_slice());
-                    values.push(size);
-                }
-
-                if let Some(names) = repeat_names_iter.next() {
-                    current_names.extend_from_slice(&names);
                 }
+                values.push(TrackListValue::TrackRepeat(repeat));
             } else {
                 if values.is_empty() && auto_repeat.is_none() {
                     return Err(StyleParseError::UnspecifiedError.into())
@@ -260,6 +248,79 @@ impl Parse for TrackList<LengthOrPercentage> {
     }
 }
 
+impl ToComputedValue for TrackList<LengthOrPercentage> {
+    type ComputedValue = TrackList<computed::LengthOrPercentage>;
+
+    #[inline]
+    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
+        // Merge the line names while computing values. The resulting values will
+        // all be bunch of `<track-size>` and one <auto-repeat>.
+        //
+        // For example,
+        // `[a b] 100px [c d] repeat(1, 30px [g]) [h]` will be merged as `[a b] 100px [c d] 30px [g h]`
+        //  whereas, `[a b] repeat(2, [c] 50px [d]) [e f] repeat(auto-fill, [g] 12px) 10px [h]` will be merged as
+        // `[a b c] 50px [d c] 50px [d e f] repeat(auto-fill, [g] 12px) 10px [h]`, with the `<auto-repeat>` value
+        // set in the `auto_repeat` field, and the `idx` in TrackListType::Auto pointing to the values after
+        // `<auto-repeat>` (in this case, `10px [h]`).
+        let mut prev_names = vec![];
+        let mut line_names = Vec::with_capacity(self.line_names.len() + 1);
+        let mut values = Vec::with_capacity(self.values.len() + 1);
+        for (pos, names) in self.line_names.iter().enumerate() {
+            prev_names.extend_from_slice(&names);
+            if pos >= self.values.len() {
+                let vec = mem::replace(&mut prev_names, vec![]);
+                line_names.push(vec.into_boxed_slice());
+                continue;
+            }
+
+            match self.values[pos] {
+                TrackListValue::TrackSize(ref size) => {
+                    let vec = mem::replace(&mut prev_names, vec![]);
+                    line_names.push(vec.into_boxed_slice());
+                    values.push(TrackListValue::TrackSize(size.to_computed_value(context)));
+                },
+                TrackListValue::TrackRepeat(ref repeat) => {
+                    // If the repeat count is numeric, we expand and merge the values.
+                    let mut repeat = repeat.expand();
+                    let mut repeat_names_iter = repeat.line_names.iter();
+                    for (size, repeat_names) in repeat.track_sizes.drain(..).zip(&mut repeat_names_iter) {
+                        prev_names.extend_from_slice(&repeat_names);
+                        let vec = mem::replace(&mut prev_names, vec![]);
+                        line_names.push(vec.into_boxed_slice());
+                        values.push(TrackListValue::TrackSize(size.to_computed_value(context)));
+                    }
+
+                    if let Some(names) = repeat_names_iter.next() {
+                        prev_names.extend_from_slice(&names);
+                    }
+                },
+            }
+        }
+
+        TrackList {
+            list_type: self.list_type.to_computed_value(context),
+            values: values,
+            line_names: line_names.into_boxed_slice(),
+            auto_repeat: self.auto_repeat.clone().map(|repeat| repeat.to_computed_value(context)),
+        }
+    }
+
+    #[inline]
+    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
+        let mut values = Vec::with_capacity(computed.values.len() + 1);
+        for value in computed.values.iter().map(ToComputedValue::from_computed_value) {
+            values.push(value);
+        }
+
+        TrackList {
+            list_type: computed.list_type,
+            values: values,
+            line_names: computed.line_names.clone(),
+            auto_repeat: computed.auto_repeat.clone().map(|ref repeat| TrackRepeat::from_computed_value(repeat)),
+        }
+    }
+}
+
 impl Parse for GridTemplateComponent<LengthOrPercentage> {
     // FIXME: Derive Parse (probably with None_)
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {