Bug 1341507 part 4 - Add auto-fill length field to line name lists returned from Servo. r=mats,emilio
authorEmily McDonough <emcdonough@mozilla.com>
Thu, 19 Mar 2020 22:11:43 +0000
changeset 519764 8c1c830a9b51b4dc9b9d2c8ca0cecb26e13eefdf
parent 519763 7f70bf534b1fd7905360a6a8c7a87c5db5746f1a
child 519765 3d7db672d69eda56b391778b7160e9138a92d119
push id37232
push usercsabou@mozilla.com
push dateFri, 20 Mar 2020 09:53:53 +0000
treeherdermozilla-central@32d6a3f1f83c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, emilio
bugs1341507
milestone76.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
Bug 1341507 part 4 - Add auto-fill length field to line name lists returned from Servo. r=mats,emilio Rename fill_idx to fill_start, to indicate it is not a single value but a range. Also change a numeric_limits<>::max() involving the fill_start to use decltype() to ensure its type matches that of the auto-generated structure's field, while we're touching that code. The test to ensure only a single repeat value is allowed will be removed by a later commit. Differential Revision: https://phabricator.services.mozilla.com/D60929
layout/generic/nsGridContainerFrame.cpp
servo/components/style/values/generics/grid.rs
--- a/layout/generic/nsGridContainerFrame.cpp
+++ b/layout/generic/nsGridContainerFrame.cpp
@@ -1357,22 +1357,25 @@ class MOZ_STACK_CLASS nsGridContainerFra
         mRange(aRange),
         mIsSameDirection(aIsSameDirection),
         mHasRepeatAuto(aTracks.mHasRepeatAuto) {
     if (MOZ_UNLIKELY(aRange)) {  // subgrid case
       mClampMinLine = 1;
       mClampMaxLine = 1 + aRange->Extent();
       mRepeatAutoEnd = mRepeatAutoStart;
       const auto& styleSubgrid = aTracks.mTemplate.AsSubgrid();
+      const auto fillStart = styleSubgrid->fill_start;
+      // Use decltype so we do not rely on the exact type that was exposed from
+      // rust code.
       mHasRepeatAuto =
-          styleSubgrid->fill_idx != std::numeric_limits<size_t>::max();
+          fillStart != std::numeric_limits<decltype(fillStart)>::max();
       if (mHasRepeatAuto) {
         const auto& lineNameLists = styleSubgrid->names;
         int32_t extraAutoFillLineCount = mClampMaxLine - lineNameLists.Length();
-        mRepeatAutoStart = styleSubgrid->fill_idx;
+        mRepeatAutoStart = fillStart;
         mRepeatAutoEnd =
             mRepeatAutoStart + std::max(0, extraAutoFillLineCount + 1);
       }
     } else {
       mClampMinLine = kMinLine;
       mClampMaxLine = kMaxLine;
       if (mHasRepeatAuto) {
         mTrackAutoRepeatLineNames =
--- a/servo/components/style/values/generics/grid.rs
+++ b/servo/components/style/values/generics/grid.rs
@@ -650,113 +650,118 @@ impl<L: ToCss, I: ToCss> ToCss for Track
     ToComputedValue,
     ToResolvedValue,
     ToShmem,
 )]
 #[repr(C)]
 pub struct LineNameList {
     /// The optional `<line-name-list>`
     pub names: crate::OwnedSlice<crate::OwnedSlice<CustomIdent>>,
-    /// Indicates the line name that requires `auto-fill`, if in bounds.
-    pub fill_idx: usize,
+    /// Indicates the starting line names that requires `auto-fill`, if in bounds.
+    pub fill_start: usize,
+    /// Indicates the number of line names in the auto-fill
+    pub fill_len: usize,
 }
 
 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![];
-        let mut fill_idx = None;
+        let mut fill_data = None;
 
         loop {
             let repeat_parse_result = input.try(|input| {
                 input.expect_function_matching("repeat")?;
                 input.parse_nested_block(|input| {
                     let count = RepeatCount::parse(context, input)?;
                     input.expect_comma()?;
                     let mut names_list = vec![];
                     names_list.push(parse_line_names(input)?); // there should be at least one
                     while let Ok(names) = input.try(parse_line_names) {
                         names_list.push(names);
                     }
                     Ok((names_list, count))
                 })
             });
-
-            if let Ok((mut names_list, count)) = repeat_parse_result {
+            if let Ok((names_list, count)) = repeat_parse_result {
                 match count {
                     // FIXME(emilio): we shouldn't expand repeat() at
                     // parse time for subgrid. (bug 1583429)
                     RepeatCount::Number(num) => line_names.extend(
                         names_list
                             .iter()
                             .cloned()
                             .cycle()
                             .take(num.value() as usize * names_list.len()),
                     ),
-                    RepeatCount::AutoFill if fill_idx.is_none() => {
+                    RepeatCount::AutoFill if fill_data.is_none() => {
                         // `repeat(autof-fill, ..)` should have just one line name.
                         // FIXME(bug 1341507) the above comment is wrong per:
                         // https://drafts.csswg.org/css-grid-2/#typedef-name-repeat
                         if names_list.len() != 1 {
                             return Err(
                                 input.new_custom_error(StyleParseErrorKind::UnspecifiedError)
                             );
                         }
-                        let names = names_list.pop().unwrap();
-
-                        line_names.push(names);
-                        fill_idx = Some(line_names.len() - 1);
+                        let fill_idx = line_names.len();
+                        let fill_len = names_list.len();
+                        fill_data = Some((fill_idx, fill_len));
+                        line_names.extend(names_list.into_iter());
                     },
                     _ => return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)),
                 }
             } else if let Ok(names) = input.try(parse_line_names) {
                 line_names.push(names);
             } else {
                 break;
             }
         }
 
         if line_names.len() > MAX_GRID_LINE as usize {
             line_names.truncate(MAX_GRID_LINE as usize);
         }
 
+        let (fill_start, fill_len) = fill_data.unwrap_or((usize::MAX, 0));
+
         Ok(LineNameList {
             names: line_names.into(),
-            fill_idx: fill_idx.unwrap_or(usize::MAX),
+            fill_start: fill_start,
+            fill_len: fill_len,
         })
     }
 }
 
 impl ToCss for LineNameList {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
         dest.write_str("subgrid")?;
-        let fill_idx = self.fill_idx;
+        let fill_start = self.fill_start;
+        let fill_len = self.fill_len;
         for (i, names) in self.names.iter().enumerate() {
-            if i == fill_idx {
+            if i == fill_start {
                 dest.write_str(" repeat(auto-fill,")?;
             }
 
             dest.write_str(" [")?;
 
             if let Some((ref first, rest)) = names.split_first() {
                 first.to_css(dest)?;
                 for name in rest {
                     dest.write_str(" ")?;
                     name.to_css(dest)?;
                 }
             }
 
             dest.write_str("]")?;
-            if i == fill_idx {
+            if i == fill_start + fill_len - 1 {
                 dest.write_str(")")?;
             }
         }
 
         Ok(())
     }
 }