Auto merge of #16127 - jbendig:issue_14954, r=emilio

Add full parsing/serialization for mask-repeat and background-repeat

I implemented full parsing and serialization for the mask-repeat and background-repeat style properties. I think some more tests are required but I'm not what I'm missing. I'd appreciate some direction.

I also had to modify some layout code to get my changes to compile. As a result, background-repeat should work individually in both directions now too.

---
<!-- 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
- [ ] These changes fix #14954.

<!-- Either: -->
- [ ] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/16127)
<!-- Reviewable:end -->

components/layout/display_list_builder.rs

@@ -768,45 +768,45 @@ impl FragmentDisplayListBuilding for Fragment {
             let mut stretch_size = image_size;
 
             // Adjust origin and size based on background-repeat
-            match *get_cyclic(&background.background_repeat.0, index) {
+            let background_repeat = get_cyclic(&background.background_repeat.0, index);
-                background_repeat::single_value::T::no_repeat => {
+            match background_repeat.0 {
+                background_repeat::single_value::RepeatKeyword::NoRepeat => {
                     bounds.origin.x = anchor_origin_x;
-                    bounds.origin.y = anchor_origin_y;
                     bounds.size.width = image_size.width;
-                    bounds.size.height = image_size.height;
                 }
-                background_repeat::single_value::T::repeat_x => {
+                background_repeat::single_value::RepeatKeyword::Repeat => {
-                    bounds.origin.y = anchor_origin_y;
-                    bounds.size.height = image_size.height;
                     ImageFragmentInfo::tile_image(&mut bounds.origin.x,
                                                   &mut bounds.size.width,
                                                   anchor_origin_x,
                                                   image_size.width);
                 }
-                background_repeat::single_value::T::repeat_y => {
+                background_repeat::single_value::RepeatKeyword::Space => {
-                    bounds.origin.x = anchor_origin_x;
-                    bounds.size.width = image_size.width;
-                    ImageFragmentInfo::tile_image(&mut bounds.origin.y,
-                                                  &mut bounds.size.height,
-                                                  anchor_origin_y,
-                                                  image_size.height);
-                }
-                background_repeat::single_value::T::repeat => {
-                    ImageFragmentInfo::tile_image(&mut bounds.origin.x,
-                                                  &mut bounds.size.width,
-                                                  anchor_origin_x,
-                                                  image_size.width);
-                    ImageFragmentInfo::tile_image(&mut bounds.origin.y,
-                                                  &mut bounds.size.height,
-                                                  anchor_origin_y,
-                                                  image_size.height);
-                }
-                background_repeat::single_value::T::space => {
                     ImageFragmentInfo::tile_image_spaced(&mut bounds.origin.x,
                                                          &mut bounds.size.width,
                                                          &mut tile_spacing.width,
                                                          anchor_origin_x,
                                                          image_size.width);
+
+                }
+                background_repeat::single_value::RepeatKeyword::Round => {
+                    ImageFragmentInfo::tile_image_round(&mut bounds.origin.x,
+                                                        &mut bounds.size.width,
+                                                        anchor_origin_x,
+                                                        &mut stretch_size.width);
+                }
+            };
+            match background_repeat.1 {
+                background_repeat::single_value::RepeatKeyword::NoRepeat => {
+                    bounds.origin.y = anchor_origin_y;
+                    bounds.size.height = image_size.height;
+                }
+                background_repeat::single_value::RepeatKeyword::Repeat => {
+                    ImageFragmentInfo::tile_image(&mut bounds.origin.y,
+                                                  &mut bounds.size.height,
+                                                  anchor_origin_y,
+                                                  image_size.height);
+                }
+                background_repeat::single_value::RepeatKeyword::Space => {
                     ImageFragmentInfo::tile_image_spaced(&mut bounds.origin.y,
                                                          &mut bounds.size.height,
                                                          &mut tile_spacing.height,
@@ -814,11 +814,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                          image_size.height);
 
                 }
-                background_repeat::single_value::T::round => {
+                background_repeat::single_value::RepeatKeyword::Round => {
-                    ImageFragmentInfo::tile_image_round(&mut bounds.origin.x,
-                                                        &mut bounds.size.width,
-                                                        anchor_origin_x,
-                                                        &mut stretch_size.width);
                     ImageFragmentInfo::tile_image_round(&mut bounds.origin.y,
                                                         &mut bounds.size.height,
                                                         anchor_origin_y,

components/style/properties/gecko.mako.rs

@@ -2084,27 +2084,24 @@ fn static_assert() {
     %>
 
     <%self:simple_image_array_property name="repeat" shorthand="${shorthand}" field_name="mRepeat">
-        use properties::longhands::${shorthand}_repeat::single_value::computed_value::T;
+        use properties::longhands::${shorthand}_repeat::single_value::computed_value::RepeatKeyword;
         use gecko_bindings::structs::nsStyleImageLayers_Repeat;
         use gecko_bindings::structs::NS_STYLE_IMAGELAYER_REPEAT_REPEAT;
         use gecko_bindings::structs::NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT;
         use gecko_bindings::structs::NS_STYLE_IMAGELAYER_REPEAT_SPACE;
         use gecko_bindings::structs::NS_STYLE_IMAGELAYER_REPEAT_ROUND;
 
-        let (repeat_x, repeat_y) = match servo {
+        fn to_ns(repeat: RepeatKeyword) -> u32 {
-          T::repeat_x => (NS_STYLE_IMAGELAYER_REPEAT_REPEAT,
+            match repeat {
-                          NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT),
+                RepeatKeyword::Repeat => NS_STYLE_IMAGELAYER_REPEAT_REPEAT,
-          T::repeat_y => (NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT,
+                RepeatKeyword::Space => NS_STYLE_IMAGELAYER_REPEAT_SPACE,
-                          NS_STYLE_IMAGELAYER_REPEAT_REPEAT),
+                RepeatKeyword::Round => NS_STYLE_IMAGELAYER_REPEAT_ROUND,
-          T::repeat  => (NS_STYLE_IMAGELAYER_REPEAT_REPEAT,
+                RepeatKeyword::NoRepeat => NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT,
-                         NS_STYLE_IMAGELAYER_REPEAT_REPEAT),
+            }
-          T::space => (NS_STYLE_IMAGELAYER_REPEAT_SPACE,
+        }
-                       NS_STYLE_IMAGELAYER_REPEAT_SPACE),
+
-          T::round => (NS_STYLE_IMAGELAYER_REPEAT_ROUND,
+        let repeat_x = to_ns(servo.0);
-                       NS_STYLE_IMAGELAYER_REPEAT_ROUND),
+        let repeat_y = to_ns(servo.1);
-          T::no_repeat => (NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT,
-                           NS_STYLE_IMAGELAYER_REPEAT_NO_REPEAT),
-        };
         nsStyleImageLayers_Repeat {
               mXRepeat: repeat_x as u8,
               mYRepeat: repeat_y as u8,

components/style/properties/longhand/background.mako.rs

@@ -192,11 +192,117 @@ ${helpers.predefined_type("background-color", "CSSColor",
     }
 </%helpers:vector_longhand>
 
-${helpers.single_keyword("background-repeat",
+<%helpers:vector_longhand name="background-repeat" animatable="False"
-                         "repeat repeat-x repeat-y space round no-repeat",
+                          spec="https://drafts.csswg.org/css-backgrounds/#the-background-repeat">
-                         vector=True,
+    use std::fmt;
-                         spec="https://drafts.csswg.org/css-backgrounds/#the-background-repeat",
+    use style_traits::ToCss;
-                         animatable=False)}
+    use values::HasViewportPercentage;
+
+    define_css_keyword_enum!(RepeatKeyword:
+                             "repeat" => Repeat,
+                             "space" => Space,
+                             "round" => Round,
+                             "no-repeat" => NoRepeat);
+
+    #[derive(Debug, Clone, PartialEq)]
+    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+    pub enum SpecifiedValue {
+        RepeatX,
+        RepeatY,
+        Other(RepeatKeyword, Option<RepeatKeyword>),
+    }
+
+    pub mod computed_value {
+        pub use super::RepeatKeyword;
+
+        #[derive(Debug, Clone, PartialEq)]
+        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+        pub struct T(pub RepeatKeyword, pub RepeatKeyword);
+    }
+
+    no_viewport_percentage!(SpecifiedValue);
+
+    impl ToCss for computed_value::T {
+        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            match (self.0, self.1) {
+                (RepeatKeyword::Repeat, RepeatKeyword::NoRepeat) => dest.write_str("repeat-x"),
+                (RepeatKeyword::NoRepeat, RepeatKeyword::Repeat) => dest.write_str("repeat-y"),
+                (horizontal, vertical) => {
+                    try!(horizontal.to_css(dest));
+                    if horizontal != vertical {
+                        try!(dest.write_str(" "));
+                        try!(vertical.to_css(dest));
+                    }
+                    Ok(())
+                },
+            }
+        }
+    }
+    impl ToCss for SpecifiedValue {
+        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            match *self {
+                SpecifiedValue::RepeatX => dest.write_str("repeat-x"),
+                SpecifiedValue::RepeatY => dest.write_str("repeat-y"),
+                SpecifiedValue::Other(horizontal, vertical) => {
+                    try!(horizontal.to_css(dest));
+                    if let Some(vertical) = vertical {
+                        try!(dest.write_str(" "));
+                        try!(vertical.to_css(dest));
+                    }
+                    Ok(())
+                }
+            }
+        }
+    }
+
+    #[inline]
+    pub fn get_initial_value() -> computed_value::T {
+        computed_value::T(RepeatKeyword::Repeat, RepeatKeyword::Repeat)
+    }
+
+    #[inline]
+    pub fn get_initial_specified_value() -> SpecifiedValue {
+        SpecifiedValue::Other(RepeatKeyword::Repeat, None)
+    }
+
+    impl ToComputedValue for SpecifiedValue {
+        type ComputedValue = computed_value::T;
+
+        #[inline]
+        fn to_computed_value(&self, _context: &Context) -> computed_value::T {
+            match *self {
+                SpecifiedValue::RepeatX =>
+                    computed_value::T(RepeatKeyword::Repeat, RepeatKeyword::NoRepeat),
+                SpecifiedValue::RepeatY =>
+                    computed_value::T(RepeatKeyword::NoRepeat, RepeatKeyword::Repeat),
+                SpecifiedValue::Other(horizontal, vertical) =>
+                    computed_value::T(horizontal, vertical.unwrap_or(horizontal))
+            }
+        }
+
+        #[inline]
+        fn from_computed_value(computed: &computed_value::T) -> Self {
+            match (computed.0, computed.1) {
+                (RepeatKeyword::Repeat, RepeatKeyword::NoRepeat) => SpecifiedValue::RepeatX,
+                (RepeatKeyword::NoRepeat, RepeatKeyword::Repeat) => SpecifiedValue::RepeatY,
+                (horizontal, vertical) => SpecifiedValue::Other(horizontal, Some(vertical)),
+            }
+        }
+    }
+
+    pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
+        let ident = input.expect_ident()?;
+        match_ignore_ascii_case! { &ident,
+            "repeat-x" => Ok(SpecifiedValue::RepeatX),
+            "repeat-y" => Ok(SpecifiedValue::RepeatY),
+            _ => {
+                let horizontal = try!(RepeatKeyword::from_ident(&ident));
+                let vertical = input.try(RepeatKeyword::parse).ok();
+                Ok(SpecifiedValue::Other(horizontal, vertical))
+            }
+        }
+    }
+</%helpers:vector_longhand>
 
 ${helpers.single_keyword("background-attachment",
                          "scroll fixed" + (" local" if product == "gecko" else ""),

components/style/properties/longhand/svg.mako.rs

@@ -94,15 +94,24 @@ ${helpers.single_keyword("mask-mode",
                          animatable=False,
                          spec="https://drafts.fxtf.org/css-masking/#propdef-mask-mode")}
 
-// TODO implement all of repeat-style for background and mask
+<%helpers:vector_longhand name="mask-repeat" products="gecko" animatable="False" extra_prefixes="webkit"
-// https://drafts.csswg.org/css-backgrounds-3/#repeat-style
+                          spec="https://drafts.fxtf.org/css-masking/#propdef-mask-repeat">
-${helpers.single_keyword("mask-repeat",
+    pub use properties::longhands::background_repeat::single_value::parse;
-                         "repeat repeat-x repeat-y space round no-repeat",
+    pub use properties::longhands::background_repeat::single_value::SpecifiedValue;
-                         vector=True,
+    pub use properties::longhands::background_repeat::single_value::computed_value;
-                         products="gecko",
+    pub use properties::longhands::background_repeat::single_value::RepeatKeyword;
-                         extra_prefixes="webkit",
+    use properties::longhands::background_repeat::single_value;
-                         animatable=False,
+
-                         spec="https://drafts.fxtf.org/css-masking/#propdef-mask-repeat")}
+    #[inline]
+    pub fn get_initial_value() -> computed_value::T {
+        computed_value::T(RepeatKeyword::NoRepeat, RepeatKeyword::NoRepeat)
+    }
+
+    #[inline]
+    pub fn get_initial_specified_value() -> SpecifiedValue {
+        SpecifiedValue::Other(RepeatKeyword::NoRepeat, None)
+    }
+</%helpers:vector_longhand>
 
 <%helpers:vector_longhand name="mask-position-x" products="gecko" animatable="True" extra_prefixes="webkit"
                           spec="https://drafts.fxtf.org/css-masking/#propdef-mask-position">

tests/unit/style/parsing/mask.rs

@@ -121,3 +121,70 @@ fn mask_shorthand_should_parse_mode_everywhere() {
     let mut parser = Parser::new("alpha");
     assert!(mask::parse_value(&context, &mut parser).is_ok());
 }
+
+#[test]
+fn mask_repeat_should_parse_shorthand_correctly() {
+    use style::properties::longhands::mask_repeat::single_value::{RepeatKeyword, SpecifiedValue};
+
+    let repeat_x = parse_longhand!(mask_repeat, "repeat-x");
+    assert_eq!(repeat_x, mask_repeat::SpecifiedValue(vec![SpecifiedValue::RepeatX]));
+
+    let repeat_y = parse_longhand!(mask_repeat, "repeat-y");
+    assert_eq!(repeat_y, mask_repeat::SpecifiedValue(vec![SpecifiedValue::RepeatY]));
+
+    let repeat = parse_longhand!(mask_repeat, "repeat");
+    assert_eq!(repeat,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::Repeat, None)]));
+
+    let space = parse_longhand!(mask_repeat, "space");
+    assert_eq!(space,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::Space, None)]));
+
+    let round = parse_longhand!(mask_repeat, "round");
+    assert_eq!(round,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::Round, None)]));
+
+    let no_repeat = parse_longhand!(mask_repeat, "no-repeat");
+    assert_eq!(no_repeat,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::NoRepeat, None)]));
+}
+
+#[test]
+fn mask_repeat_should_parse_longhand_correctly() {
+    use style::properties::longhands::mask_repeat::single_value::{RepeatKeyword, SpecifiedValue};
+
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let reporter = CSSErrorReporterTest;
+    let context = ParserContext::new(Origin::Author, &url, &reporter);
+
+    // repeat-x is not available in longhand form.
+    let mut parser = Parser::new("repeat-x no-repeat");
+    assert!(mask_repeat::parse(&context, &mut parser).is_err());
+
+    let mut parser = Parser::new("no-repeat repeat-x");
+    assert!(mask_repeat::parse(&context, &mut parser).is_err());
+
+    // repeat-y is not available in longhand form.
+    let mut parser = Parser::new("repeat-y no-repeat");
+    assert!(mask_repeat::parse(&context, &mut parser).is_err());
+
+    let mut parser = Parser::new("no-repeat repeat-y");
+    assert!(mask_repeat::parse(&context, &mut parser).is_err());
+
+    // Longhand form supports two directions.
+    let no_repeat_and_round = parse_longhand!(mask_repeat, "no-repeat round");
+    assert_eq!(no_repeat_and_round,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::NoRepeat,
+                                                                      Some(RepeatKeyword::Round))]));
+
+    // Not three directions.
+    let mut parser = Parser::new("repeat no-repeat round");
+    assert!(mask_repeat::parse(&context, &mut parser).is_err());
+
+    // Multiple values with mixed shortform and longform should parse.
+    let multiple = parse_longhand!(mask_repeat, "repeat, no-repeat round");
+    assert_eq!(multiple,
+               mask_repeat::SpecifiedValue(vec![SpecifiedValue::Other(RepeatKeyword::Repeat, None),
+                                                SpecifiedValue::Other(RepeatKeyword::NoRepeat,
+                                                                      Some(RepeatKeyword::Round))]));
+}

tests/unit/style/properties/serialization.rs

@@ -827,7 +827,7 @@ mod shorthand_serialization {
                 )
             );
 
-            let repeat = single_vec_keyword_value!(repeat, repeat_x);
+            let repeat = single_vec_keyword_value!(repeat, RepeatX);
             let origin = single_vec_keyword_value!(origin, padding_box);
             let clip = single_vec_keyword_value!(clip, border_box);
             let composite = single_vec_keyword_value!(composite, subtract);
@@ -883,7 +883,7 @@ mod shorthand_serialization {
                 )
             );
 
-            let repeat = single_vec_keyword_value!(repeat, repeat_x);
+            let repeat = single_vec_keyword_value!(repeat, RepeatX);
             let origin = single_vec_keyword_value!(origin, padding_box);
             let clip = single_vec_keyword_value!(clip, padding_box);
             let composite = single_vec_keyword_value!(composite, subtract);

tests/wpt/metadata-css/css-backgrounds-3_dev/html4/background-size-027.htm.ini

@@ -1,3 +0,0 @@
-[background-size-027.htm]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata-css/css-backgrounds-3_dev/html4/background-size-031.htm.ini

@@ -1,3 +0,0 @@
-[background-size-031.htm]
-  type: reftest
-  expected: FAIL