servo: Merge #17776 - More grid serialization and parsing fixes (from canaltinova:disabled-mochitests); r=emilio
authorNazım Can Altınova <canaltinova@gmail.com>
Wed, 19 Jul 2017 23:54:33 -0700
changeset 418545 dc7a346e62fc7053e0519c324ab232898e184320
parent 418544 1ec985416f30eb25b17f36fdd6bcaeb171fb1cdb
child 418546 9c0436b452d6df7ab0eddf0fa1787e305630fc0b
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17776 - More grid serialization and parsing fixes (from canaltinova:disabled-mochitests); r=emilio These are the bugs I discovered while I was re-enabling the disabled grid mochitests. Now all grid mochitests are passing! Additionally I converted `Vec<CustomIdent>`'s into `Box<[CustomIdent]>`. We are storing so many line names in vectors. These should help a bit. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 20a3b0236d20e621591e751646758e833526084b
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/shorthand/position.mako.rs
servo/components/style/values/generics/grid.rs
servo/components/style/values/specified/grid.rs
servo/tests/unit/style/parsing/position.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -1366,20 +1366,21 @@ fn static_assert() {
 
     % for value in GRID_LINES:
     pub fn set_${value.name}(&mut self, v: longhands::${value.name}::computed_value::T) {
         use gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine};
 
         let ident = v.ident.as_ref().map_or(&[] as &[_], |ident| ident.0.as_slice());
         self.gecko.${value.gecko}.mLineName.assign(ident);
         self.gecko.${value.gecko}.mHasSpan = v.is_span;
-        self.gecko.${value.gecko}.mInteger = v.line_num.map(|i| {
+        if let Some(integer) = v.line_num {
             // clamping the integer between a range
-            cmp::max(nsStyleGridLine_kMinLine, cmp::min(i.value(), nsStyleGridLine_kMaxLine))
-        }).unwrap_or(0);
+            self.gecko.${value.gecko}.mInteger = cmp::max(nsStyleGridLine_kMinLine,
+                cmp::min(integer.value(), nsStyleGridLine_kMaxLine));
+        }
     }
 
     pub fn copy_${value.name}_from(&mut self, other: &Self) {
         self.gecko.${value.gecko}.mHasSpan = other.gecko.${value.gecko}.mHasSpan;
         self.gecko.${value.gecko}.mInteger = other.gecko.${value.gecko}.mInteger;
         self.gecko.${value.gecko}.mLineName.assign(&*other.gecko.${value.gecko}.mLineName);
     }
 
@@ -1515,41 +1516,42 @@ fn static_assert() {
                 unsafe {
                     bindings::Gecko_SetStyleGridTemplateArrayLengths(&mut ${self_grid}, 0);
                     bindings::Gecko_ResizeTArrayForStrings(
                         &mut ${self_grid}.mRepeatAutoLineNameListBefore, 0);
                     bindings::Gecko_ResizeTArrayForStrings(
                         &mut ${self_grid}.mRepeatAutoLineNameListAfter, 0);
                 }
             },
-            GridTemplateComponent::Subgrid(mut list) => {
+            GridTemplateComponent::Subgrid(list) => {
                 ${self_grid}.set_mIsSubgrid(true);
                 let names_length = match list.fill_idx {
                     Some(_) => list.names.len() - 1,
                     None => list.names.len(),
                 };
                 let num_values = cmp::min(names_length, max_lines + 1);
 
                 unsafe {
                     bindings::Gecko_SetStyleGridTemplateArrayLengths(&mut ${self_grid}, 0);
                     bindings::Gecko_SetGridTemplateLineNamesLength(&mut ${self_grid}, num_values as u32);
                     bindings::Gecko_ResizeTArrayForStrings(
                         &mut ${self_grid}.mRepeatAutoLineNameListBefore, 0);
                     bindings::Gecko_ResizeTArrayForStrings(
                         &mut ${self_grid}.mRepeatAutoLineNameListAfter, 0);
                 }
 
+                let mut names = list.names.into_vec();
                 if let Some(idx) = list.fill_idx {
                     ${self_grid}.set_mIsAutoFill(true);
                     ${self_grid}.mRepeatAutoIndex = idx as i16;
-                    set_line_names(&list.names.swap_remove(idx as usize),
+                    set_line_names(&names.swap_remove(idx as usize),
                                    &mut ${self_grid}.mRepeatAutoLineNameListBefore);
                 }
 
-                for (servo_names, gecko_names) in list.names.iter().zip(${self_grid}.mLineNameLists.iter_mut()) {
+                for (servo_names, gecko_names) in names.iter().zip(${self_grid}.mLineNameLists.iter_mut()) {
                     set_line_names(servo_names, gecko_names);
                 }
             },
         }
     }
 
     pub fn copy_grid_template_${kind}_from(&mut self, other: &Self) {
         unsafe {
--- a/servo/components/style/properties/shorthand/position.mako.rs
+++ b/servo/components/style/properties/shorthand/position.mako.rs
@@ -247,17 +247,16 @@
     use values::specified::{GridTemplateComponent, GenericGridTemplateComponent};
     use values::specified::grid::parse_line_names;
 
     /// Parsing for `<grid-template>` shorthand (also used by `grid` shorthand).
     pub fn parse_grid_template<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                        -> Result<(GridTemplateComponent,
                                                   GridTemplateComponent,
                                                   Either<TemplateAreas, None_>), ParseError<'i>> {
-
         // Other shorthand sub properties also parse `none` and `subgrid` keywords and this
         // shorthand should know after these keywords there is nothing to parse. Otherwise it
         // gets confused and rejects the sub properties that contains `none` or `subgrid`.
         <% keywords = {
             "none": "GenericGridTemplateComponent::None",
             "subgrid": "GenericGridTemplateComponent::Subgrid(LineNameList::default())"
         }
         %>
@@ -273,52 +272,53 @@
                     }
                 }
                 Err(())
             }) {
                 return Ok(x);
             }
         % endfor
 
-        let first_line_names = input.try(parse_line_names).unwrap_or(vec![]);
+        let first_line_names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice());
         if let Ok(s) = input.try(Parser::expect_string) {
             let mut strings = vec![];
             let mut values = vec![];
             let mut line_names = vec![];
-            let mut names = first_line_names;
+            let mut names = first_line_names.into_vec();
             let mut string = s.into_owned().into_boxed_str();
             loop {
-                line_names.push(names);
+                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);
-                names = input.try(parse_line_names).unwrap_or(vec![]);
+                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);
+                    names.extend(v.into_vec());
                 }
 
                 string = match input.try(Parser::expect_string) {
                     Ok(s) => s.into_owned().into_boxed_str(),
                     _ => {      // only the named area determines whether we should bail out
-                        line_names.push(names);
+                        line_names.push(names.into_boxed_slice());
                         break
                     },
                 };
             }
 
             if line_names.len() == values.len() {
-                line_names.push(vec![]);        // should be one longer than track sizes
+                // should be one longer than track sizes
+                line_names.push(vec![].into_boxed_slice());
             }
 
             let template_areas = TemplateAreas::from_vec(strings)
                 .map_err(|()| StyleParseError::UnspecifiedError)?;
             let template_rows = TrackList {
                 list_type: TrackListType::Normal,
                 values: values,
-                line_names: line_names,
+                line_names: line_names.into_boxed_slice(),
                 auto_repeat: None,
             };
 
             let template_cols = if input.try(|i| i.expect_delim('/')).is_ok() {
                 let value = GridTemplateComponent::parse_without_none(context, input)?;
                 if let GenericGridTemplateComponent::TrackList(ref list) = value {
                     if list.list_type != TrackListType::Explicit {
                         return Err(StyleParseError::UnspecifiedError.into())
@@ -367,21 +367,51 @@
                                       dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *template_areas {
             Either::Second(_none) => {
                 template_rows.to_css(dest)?;
                 dest.write_str(" / ")?;
                 template_columns.to_css(dest)
             },
             Either::First(ref areas) => {
+                // The length of template-area and template-rows values should be equal.
+                if areas.strings.len() != template_rows.track_list_len() {
+                    return Ok(());
+                }
+
                 let track_list = match *template_rows {
-                    GenericGridTemplateComponent::TrackList(ref list) => list,
-                    _ => unreachable!(),        // should exist!
+                    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() {
+                            return Ok(());
+                        }
+                        list
+                    },
+                    // Others template components shouldn't exist with normal shorthand values.
+                    // But if we need to serialize a group of longhand sub-properties for
+                    // the shorthand, we should be able to return empty string instead of crashing.
+                    _ => return Ok(()),
                 };
 
+                // We need to check some values that longhand accepts but shorthands don't.
+                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() => {
+                        return Ok(());
+                    },
+                    // Also the shorthands don't accept subgrids unlike longhand.
+                    // We should fail without an error here.
+                    GenericGridTemplateComponent::Subgrid(_) => {
+                        return Ok(());
+                    },
+                    _ => {},
+                }
+
                 let mut names_iter = track_list.line_names.iter();
                 for (((i, string), names), size) in areas.strings.iter().enumerate()
                                                                  .zip(&mut names_iter)
                                                                  .zip(track_list.values.iter()) {
                     if i > 0 {
                         dest.write_str(" ")?;
                     }
 
@@ -423,18 +453,18 @@
                                     grid-auto-flow"
                     spec="https://drafts.csswg.org/css-grid/#propdef-grid"
                     disable_when_testing="True"
                     products="gecko">
     use parser::Parse;
     use properties::longhands::{grid_auto_columns, grid_auto_rows, grid_auto_flow};
     use properties::longhands::grid_auto_flow::computed_value::{AutoFlow, T as SpecifiedAutoFlow};
     use values::{Either, None_};
-    use values::generics::grid::GridTemplateComponent;
-    use values::specified::{LengthOrPercentage, TrackSize};
+    use values::generics::grid::{GridTemplateComponent, TrackListType};
+    use values::specified::{GenericGridTemplateComponent, LengthOrPercentage, TrackSize};
 
     pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                -> Result<Longhands, ParseError<'i>> {
         let mut temp_rows = GridTemplateComponent::None;
         let mut temp_cols = GridTemplateComponent::None;
         let mut temp_areas = Either::Second(None_);
         let mut auto_rows = TrackSize::default();
         let mut auto_cols = TrackSize::default();
@@ -502,37 +532,71 @@
             *self.grid_auto_rows == TrackSize::default() &&
             *self.grid_auto_columns == TrackSize::default() &&
             *self.grid_auto_flow == grid_auto_flow::get_initial_value()
         }
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a> {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            // `grid` shorthand resets these properties. If they are not zero, that means they
+            // are changed by longhands and in that case we should fail serializing `grid`.
+            if *self.grid_row_gap != LengthOrPercentage::zero() ||
+               *self.grid_column_gap != LengthOrPercentage::zero() {
+                return Ok(());
+            }
+
+
             if *self.grid_template_areas != Either::Second(None_) ||
                (*self.grid_template_rows != GridTemplateComponent::None &&
                    *self.grid_template_columns != GridTemplateComponent::None) ||
                self.is_grid_template() {
                 return super::grid_template::serialize_grid_template(self.grid_template_rows,
                                                                      self.grid_template_columns,
                                                                      self.grid_template_areas, dest);
             }
 
             if self.grid_auto_flow.autoflow == AutoFlow::Column {
+                // It should fail to serialize if other branch of the if condition's values are set.
+                if *self.grid_auto_rows != TrackSize::default() ||
+                   *self.grid_template_columns != GridTemplateComponent::None {
+                    return Ok(());
+                }
+
+                // It should fail to serialize if template-rows value is not Explicit.
+                if let GenericGridTemplateComponent::TrackList(ref list) = *self.grid_template_rows {
+                    if list.list_type != TrackListType::Explicit {
+                        return Ok(());
+                    }
+                }
+
                 self.grid_template_rows.to_css(dest)?;
                 dest.write_str(" / auto-flow")?;
                 if self.grid_auto_flow.dense {
                     dest.write_str(" dense")?;
                 }
 
                 if !self.grid_auto_columns.is_default() {
                     dest.write_str(" ")?;
                     self.grid_auto_columns.to_css(dest)?;
                 }
             } else {
+                // It should fail to serialize if other branch of the if condition's values are set.
+                if *self.grid_auto_columns != TrackSize::default() ||
+                   *self.grid_template_rows != GridTemplateComponent::None {
+                    return Ok(());
+                }
+
+                // It should fail to serialize if template-column value is not Explicit.
+                if let GenericGridTemplateComponent::TrackList(ref list) = *self.grid_template_columns {
+                    if list.list_type != TrackListType::Explicit {
+                        return Ok(());
+                    }
+                }
+
                 dest.write_str("auto-flow")?;
                 if self.grid_auto_flow.dense {
                     dest.write_str(" dense")?;
                 }
 
                 if !self.grid_auto_rows.is_default() {
                     dest.write_str(" ")?;
                     self.grid_auto_rows.to_css(dest)?;
--- a/servo/components/style/values/generics/grid.rs
+++ b/servo/components/style/values/generics/grid.rs
@@ -114,19 +114,17 @@ impl Parse for GridLine {
             return Err(StyleParseError::UnspecifiedError.into())
         }
 
         if grid_line.is_span {
             if let Some(i) = grid_line.line_num {
                 if i.value() <= 0 {       // disallow negative integers for grid spans
                     return Err(StyleParseError::UnspecifiedError.into())
                 }
-            } else if grid_line.ident.is_some() {       // integer could be omitted
-                grid_line.line_num = Some(Integer::new(1));
-            } else {
+            } else if grid_line.ident.is_none() {       // integer could be omitted
                 return Err(StyleParseError::UnspecifiedError.into())
             }
         }
 
         Ok(grid_line)
     }
 }
 
@@ -200,30 +198,28 @@ impl<L: ToComputedValue> ToComputedValue
     }
 }
 
 /// A `<track-size>` type for explicit grid track sizing. Like `<track-breadth>`, this is
 /// generic only to avoid code bloat. It only takes `<length-percentage>`
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-track-size
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)]
+#[derive(Clone, Debug, HasViewportPercentage, PartialEq)]
 pub enum TrackSize<L> {
     /// A flexible `<track-breadth>`
     Breadth(TrackBreadth<L>),
     /// A `minmax` function for a range over an inflexible `<track-breadth>`
     /// and a flexible `<track-breadth>`
     ///
     /// https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-minmax
-    #[css(comma, function)]
     Minmax(TrackBreadth<L>, TrackBreadth<L>),
     /// A `fit-content` function.
     ///
     /// https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-fit-content
-    #[css(function)]
     FitContent(L),
 }
 
 impl<L> TrackSize<L> {
     /// Check whether this is a `<fixed-size>`
     ///
     /// https://drafts.csswg.org/css-grid/#typedef-fixed-size
     pub fn is_fixed(&self) -> bool {
@@ -256,16 +252,44 @@ impl<L> Default for TrackSize<L> {
 
 impl<L: PartialEq> TrackSize<L> {
     /// Returns true if current TrackSize is same as default.
     pub fn is_default(&self) -> bool {
         *self == TrackSize::default()
     }
 }
 
+impl<L: ToCss> ToCss for TrackSize<L> {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        match *self {
+            TrackSize::Breadth(ref breadth) => breadth.to_css(dest),
+            TrackSize::Minmax(ref min, ref max) => {
+                // According to gecko minmax(auto, <flex>) is equivalent to <flex>,
+                // and both are serialized as <flex>.
+                if let TrackBreadth::Keyword(TrackKeyword::Auto) = *min {
+                    if let TrackBreadth::Flex(_) = *max {
+                        return max.to_css(dest);
+                    }
+                }
+
+                dest.write_str("minmax(")?;
+                min.to_css(dest)?;
+                dest.write_str(", ")?;
+                max.to_css(dest)?;
+                dest.write_str(")")
+            },
+            TrackSize::FitContent(ref lop) => {
+                dest.write_str("fit-content(")?;
+                lop.to_css(dest)?;
+                dest.write_str(")")
+            },
+        }
+    }
+}
+
 impl<L: ToComputedValue> ToComputedValue for TrackSize<L> {
     type ComputedValue = TrackSize<L::ComputedValue>;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
         match *self {
             TrackSize::Breadth(ref b) => match *b {
                 // <flex> outside `minmax()` expands to `mimmax(auto, <flex>)`
@@ -325,18 +349,23 @@ pub enum RepeatCount {
     /// An `<auto-fill>` keyword allowed only for `<auto-repeat>`
     AutoFill,
     /// An `<auto-fit>` keyword allowed only for `<auto-repeat>`
     AutoFit,
 }
 
 impl Parse for RepeatCount {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        if let Ok(i) = input.try(|i| Integer::parse(context, i)) {
+        // Maximum number of repeat is 10000. The greater numbers should be clamped.
+        const MAX_LINE: i32 = 10000;
+        if let Ok(mut i) = input.try(|i| Integer::parse(context, i)) {
             if i.value() > 0 {
+                if i.value() > MAX_LINE {
+                    i = Integer::new(MAX_LINE);
+                }
                 Ok(RepeatCount::Number(i))
             } else {
                 Err(StyleParseError::UnspecifiedError.into())
             }
         } else {
             try_match_ident_ignore_ascii_case! { input.expect_ident()?,
                 "auto-fill" => Ok(RepeatCount::AutoFill),
                 "auto-fit" => Ok(RepeatCount::AutoFit),
@@ -357,17 +386,17 @@ no_viewport_percentage!(RepeatCount);
 pub struct TrackRepeat<L> {
     /// The number of times for the value to be repeated (could also be `auto-fit` or `auto-fill`)
     pub count: RepeatCount,
     /// `<line-names>` accompanying `<track_size>` values.
     ///
     /// If there's no `<line-names>`, then it's represented by an empty vector.
     /// For N `<track-size>` 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>`.
-    pub line_names: Vec<Vec<CustomIdent>>,
+    pub line_names: Box<[Box<[CustomIdent]>]>,
     /// `<track-size>` values.
     pub track_sizes: Vec<TrackSize<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
         // with `repeat` function. It should serialized with `N` repeated form.
@@ -418,30 +447,31 @@ impl<L: Clone> TrackRepeat<L> {
             let mut line_names = vec![];
             let mut track_sizes = vec![];
             let mut prev_names = vec![];
 
             for _ in 0..num.value() {
                 let mut names_iter = self.line_names.iter();
                 for (size, names) in self.track_sizes.iter().zip(&mut names_iter) {
                     prev_names.extend_from_slice(&names);
-                    line_names.push(mem::replace(&mut prev_names, vec![]));
+                    let vec = mem::replace(&mut prev_names, vec![]);
+                    line_names.push(vec.into_boxed_slice());
                     track_sizes.push(size.clone());
                 }
 
                 if let Some(names) = names_iter.next() {
                     prev_names.extend_from_slice(&names);
                 }
             }
 
-            line_names.push(prev_names);
+            line_names.push(prev_names.into_boxed_slice());
             TrackRepeat {
                 count: self.count,
                 track_sizes: track_sizes,
-                line_names: line_names,
+                line_names: line_names.into_boxed_slice(),
             }
 
         } else {    // if it's auto-fit/auto-fill, then it's left to the layout.
             TrackRepeat {
                 count: self.count,
                 track_sizes: self.track_sizes.clone(),
                 line_names: self.line_names.clone(),
             }
@@ -512,17 +542,17 @@ pub struct TrackList<T> {
     pub list_type: TrackListType,
     /// A vector of `<track-size>` values.
     pub values: Vec<TrackSize<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>`.
-    pub line_names: Vec<Vec<CustomIdent>>,
+    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>>,
 }
 
 impl<T: ToCss> ToCss for TrackList<T> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         let auto_idx = match self.list_type {
             TrackListType::Auto(i) => i as usize,
@@ -569,17 +599,17 @@ impl<T: ToCss> ToCss for TrackList<T> {
 /// The `<line-name-list>` for subgrids.
 ///
 /// `subgrid [ <line-names> | repeat(<positive-integer> | auto-fill, <line-names>+) ]+`
 /// Old spec: https://www.w3.org/TR/2015/WD-css-grid-1-20150917/#typedef-line-name-list
 #[derive(Clone, PartialEq, Debug, Default)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct LineNameList {
     /// The optional `<line-name-list>`
-    pub names: Vec<Vec<CustomIdent>>,
+    pub names: Box<[Box<[CustomIdent]>]>,
     /// Indicates the line name that requires `auto-fill`
     pub fill_idx: Option<u32>,
 }
 
 impl Parse for LineNameList {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         input.expect_ident_matching("subgrid")?;
         let mut line_names = vec![];
@@ -621,17 +651,17 @@ impl Parse for LineNameList {
             } else if let Ok(names) = input.try(parse_line_names) {
                 line_names.push(names);
             } else {
                 break
             }
         }
 
         Ok(LineNameList {
-            names: line_names,
+            names: line_names.into_boxed_slice(),
             fill_idx: fill_idx,
         })
     }
 }
 
 impl ToCss for LineNameList {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         dest.write_str("subgrid")?;
@@ -672,8 +702,18 @@ no_viewport_percentage!(LineNameList);
 pub enum GridTemplateComponent<L> {
     /// `none` value.
     None,
     /// The grid `<track-list>`
     TrackList(TrackList<L>),
     /// A `subgrid <line-name-list>?`
     Subgrid(LineNameList),
 }
+
+impl<L> GridTemplateComponent<L> {
+    /// Returns length of the <track-list>s <track-size>
+    pub fn track_list_len(&self) -> usize {
+        match *self {
+            GridTemplateComponent::TrackList(ref tracklist) => tracklist.values.len(),
+            _ => 0,
+        }
+    }
+}
--- a/servo/components/style/values/specified/grid.rs
+++ b/servo/components/style/values/specified/grid.rs
@@ -76,26 +76,26 @@ impl Parse for TrackSize<LengthOrPercent
         let lop = input.parse_nested_block(|i| LengthOrPercentage::parse_non_negative(context, i))?;
         Ok(TrackSize::FitContent(lop))
     }
 }
 
 /// Parse the grid line names into a vector of owned strings.
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-line-names
-pub fn parse_line_names<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Vec<CustomIdent>, ParseError<'i>> {
+pub fn parse_line_names<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Box<[CustomIdent]>, ParseError<'i>> {
     input.expect_square_bracket_block()?;
     input.parse_nested_block(|input| {
         let mut values = vec![];
         while let Ok(ident) = input.try(|i| i.expect_ident()) {
             let ident = CustomIdent::from_ident(ident, &["span"])?;
             values.push(ident);
         }
 
-        Ok(values)
+        Ok(values.into_boxed_slice())
     })
 }
 
 /// The type of `repeat` function (only used in parsing).
 ///
 /// https://drafts.csswg.org/css-grid/#typedef-track-repeat
 #[derive(Clone, Copy, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@@ -123,17 +123,17 @@ impl TrackRepeat<LengthOrPercentage> {
                     RepeatType::Fixed
                 };
 
                 let mut names = vec![];
                 let mut values = vec![];
                 let mut current_names;
 
                 loop {
-                    current_names = input.try(parse_line_names).unwrap_or(vec![]);
+                    current_names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice());
                     if let Ok(track_size) = input.try(|i| TrackSize::parse(context, i)) {
                         if !track_size.is_fixed() {
                             if is_auto {
                                 // should be <fixed-size> for <auto-repeat>
                                 return Err(StyleParseError::UnspecifiedError.into())
                             }
 
                             if repeat_type == RepeatType::Fixed {
@@ -145,34 +145,34 @@ impl TrackRepeat<LengthOrPercentage> {
                         names.push(current_names);
                         if is_auto {
                             // FIXME: In the older version of the spec
                             // (https://www.w3.org/TR/2015/WD-css-grid-1-20150917/#typedef-auto-repeat),
                             // if the repeat type is `<auto-repeat>` we shouldn't try to parse more than
                             // one `TrackSize`. But in current version of the spec, this is deprecated
                             // but we are adding this for gecko parity. We should remove this when
                             // gecko implements new spec.
-                            names.push(input.try(parse_line_names).unwrap_or(vec![]));
+                            names.push(input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()));
                             break
                         }
                     } else {
                         if values.is_empty() {
                             // expecting at least one <track-size>
                             return Err(StyleParseError::UnspecifiedError.into())
                         }
 
                         names.push(current_names);      // final `<line-names>`
                         break       // no more <track-size>, breaking
                     }
                 }
 
                 let repeat = TrackRepeat {
                     count: count,
                     track_sizes: values,
-                    line_names: names,
+                    line_names: names.into_boxed_slice(),
                 };
 
                 Ok((repeat, repeat_type))
             })
         })
     }
 }
 
@@ -182,16 +182,18 @@ impl HasViewportPercentage for TrackRepe
         self.track_sizes.iter().any(|ref v| v.has_viewport_percentage())
     }
 }
 
 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![];
@@ -200,27 +202,28 @@ impl Parse for TrackList<LengthOrPercent
 
         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;
         // 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.append(&mut input.try(parse_line_names).unwrap_or(vec![]));
+            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)) {
                 if !track_size.is_fixed() {
                     atleast_one_not_fixed = true;
                     if auto_repeat.is_some() {
                         // <auto-track-list> only accepts <fixed-size> and <fixed-repeat>
                         return Err(StyleParseError::UnspecifiedError.into())
                     }
                 }
 
-                names.push(mem::replace(&mut current_names, vec![]));
+                let vec = mem::replace(&mut current_names, vec![]);
+                names.push(vec.into_boxed_slice());
                 values.push(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()
                 }
 
                 match type_ {
                     RepeatType::Normal => {
@@ -232,48 +235,50 @@ impl Parse for TrackList<LengthOrPercent
                     RepeatType::Auto => {
                         if auto_repeat.is_some() || atleast_one_not_fixed {
                             // We've either seen <auto-repeat> earlier, or there's at least one non-fixed value
                             return Err(StyleParseError::UnspecifiedError.into())
                         }
 
                         list_type = TrackListType::Auto(values.len() as u16);
                         auto_repeat = Some(repeat);
-                        names.push(mem::replace(&mut current_names, vec![]));
+                        let vec = mem::replace(&mut current_names, vec![]);
+                        names.push(vec.into_boxed_slice());
                         continue
                     },
                     RepeatType::Fixed => (),
                 }
 
                 // If the repeat count is numeric, we axpand and merge the values.
                 let mut repeat = repeat.expand();
-                let mut repeat_names_iter = repeat.line_names.drain(..);
+                let mut repeat_names_iter = repeat.line_names.iter();
                 for (size, repeat_names) in repeat.track_sizes.drain(..).zip(&mut repeat_names_iter) {
                     current_names.extend_from_slice(&repeat_names);
-                    names.push(mem::replace(&mut current_names, vec![]));
+                    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);
                 }
             } else {
                 if values.is_empty() && auto_repeat.is_none() {
                     return Err(StyleParseError::UnspecifiedError.into())
                 }
 
-                names.push(current_names);
+                names.push(current_names.into_boxed_slice());
                 break
             }
         }
 
         Ok(TrackList {
             list_type: list_type,
             values: values,
-            line_names: names,
+            line_names: names.into_boxed_slice(),
             auto_repeat: auto_repeat,
         })
     }
 }
 
 impl HasViewportPercentage for TrackList<LengthOrPercentage> {
     #[inline]
     fn has_viewport_percentage(&self) -> bool {
--- a/servo/tests/unit/style/parsing/position.rs
+++ b/servo/tests/unit/style/parsing/position.rs
@@ -232,17 +232,17 @@ fn test_computed_grid_template_rows_colu
         "[a b c] auto [e] auto [d b c] auto [e] auto [d]");
 
     assert_computed_serialization(grid_template_rows::parse,
         "[a] 50px [b] 10% [b c d] repeat(2, [e] 40px [f]) [g] repeat(auto-fill, [h i] 20px [j]) [k] 10px [l]",
         "[a] 50px [b] 10% [b c d e] 40px [f e] 40px [f g] repeat(auto-fill, [h i] 20px [j]) [k] 10px [l]");
 
     assert_computed_serialization(grid_template_rows::parse,
         "10px repeat(2, 1fr auto minmax(200px, 1fr))",
-        "10px minmax(auto, 1fr) auto minmax(200px, 1fr) minmax(auto, 1fr) auto minmax(200px, 1fr)");
+        "10px 1fr auto minmax(200px, 1fr) 1fr auto minmax(200px, 1fr)");
 
     assert_computed_serialization(grid_template_rows::parse,
         "subgrid [a] [] repeat(auto-fill, [])", "subgrid [a] [] repeat(auto-fill, [])");
 
     assert_computed_serialization(grid_template_rows::parse,
         "subgrid [a] [b] repeat(2, [c d] [] [e]) [] repeat(auto-fill, [])",
         "subgrid [a] [b] [c d] [] [e] [c d] [] [e] [] repeat(auto-fill, [])");
 }