Auto merge of #19964 - emilio:font-shorthand, r=nox

style: Don't serialize default values of the font shorthand.

This makes us consistent with the old style system and Blink / WebKit.

Not adding a test because shorthands serialization is a mess... :(

Anyway, nothing like fixing bugs by removing code.

Bug: 1436031

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

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

@@ -143,27 +143,14 @@
             None
         }
     % endif
-    enum SerializeFor {
-        Normal,
-    % if product == "gecko":
-        Canvas,
-    % endif
-    }
 
-    impl<'a> LonghandsToSerialize<'a> {
+    impl<'a> ToCss for LonghandsToSerialize<'a> {
-        fn to_css_for<W>(
+        fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-            &self,
-            serialize_for: SerializeFor,
-            dest: &mut CssWriter<W>,
-        ) -> fmt::Result
-        where
-            W: Write,
-        {
             % if product == "gecko":
                 match self.check_system() {
                     CheckSystemResult::AllSystem(sys) => return sys.to_css(dest),
                     CheckSystemResult::SomeSystem => return Ok(()),
-                    CheckSystemResult::None => ()
+                    CheckSystemResult::None => {}
                 }
             % endif
 
@@ -178,14 +165,7 @@
             // In case of serialization for canvas font, we need to drop
             // initial values of properties other than size and family.
             % for name in "style variant_caps weight stretch".split():
-                let needs_this_property = match serialize_for {
+                if self.font_${name} != &font_${name}::get_initial_specified_value() {
-                    SerializeFor::Normal => true,
-                % if product == "gecko":
-                    SerializeFor::Canvas =>
-                        self.font_${name} != &font_${name}::get_initial_specified_value(),
-                % endif
-                };
-                if needs_this_property {
                     self.font_${name}.to_css(dest)?;
                     dest.write_str(" ")?;
                 }
@@ -203,45 +183,35 @@
 
             Ok(())
         }
-
-        % if product == "gecko":
-            /// Check if some or all members are system fonts
-            fn check_system(&self) -> CheckSystemResult {
-                let mut sys = None;
-                let mut all = true;
-
-                % for prop in SYSTEM_FONT_LONGHANDS:
-                    if let Some(s) = self.${prop}.get_system() {
-                        debug_assert!(sys.is_none() || s == sys.unwrap());
-                        sys = Some(s);
-                    } else {
-                        all = false;
-                    }
-                % endfor
-                if self.line_height != &LineHeight::normal() {
-                    all = false
-                }
-                if all {
-                    CheckSystemResult::AllSystem(sys.unwrap())
-                } else if sys.is_some() {
-                    CheckSystemResult::SomeSystem
-                } else {
-                    CheckSystemResult::None
-                }
-            }
-
-            /// Serialize the shorthand value for canvas font attribute.
-            pub fn to_css_for_canvas<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-                self.to_css_for(SerializeFor::Canvas, dest)
-            }
-        % endif
     }
 
-    // This may be a bit off, unsure, possibly needs changes
+    impl<'a> LonghandsToSerialize<'a> {
-    impl<'a> ToCss for LonghandsToSerialize<'a>  {
+        % if product == "gecko":
-        fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
+        /// Check if some or all members are system fonts
-            self.to_css_for(SerializeFor::Normal, dest)
+        fn check_system(&self) -> CheckSystemResult {
+            let mut sys = None;
+            let mut all = true;
+
+            % for prop in SYSTEM_FONT_LONGHANDS:
+                if let Some(s) = self.${prop}.get_system() {
+                    debug_assert!(sys.is_none() || s == sys.unwrap());
+                    sys = Some(s);
+                } else {
+                    all = false;
+                }
+            % endfor
+            if self.line_height != &LineHeight::normal() {
+                all = false
+            }
+            if all {
+                CheckSystemResult::AllSystem(sys.unwrap())
+            } else if sys.is_some() {
+                CheckSystemResult::SomeSystem
+            } else {
+                CheckSystemResult::None
+            }
         }
+        % endif
     }
 </%helpers:shorthand>
 

ports/geckolib/glue.rs

@@ -2670,12 +2670,11 @@ pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue(
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_SerializeFontValueForCanvas(
+pub unsafe extern "C" fn Servo_SerializeFontValueForCanvas(
     declarations: RawServoDeclarationBlockBorrowed,
     buffer: *mut nsAString,
 ) {
     use style::properties::shorthands::font;
-
     read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
         let longhands = match font::LonghandsToSerialize::from_iter(decls.declarations().iter()) {
             Ok(l) => l,
@@ -2685,12 +2684,8 @@ pub extern "C" fn Servo_SerializeFontValueForCanvas(
             }
         };
 
-        let mut string = String::new();
+        let rv = longhands.to_css(&mut CssWriter::new(&mut *buffer));
-        let rv = longhands.to_css_for_canvas(&mut CssWriter::new(&mut string));
         debug_assert!(rv.is_ok());
-
-        let buffer = unsafe { buffer.as_mut().unwrap() };
-        buffer.assign_utf8(&string);
     })
 }