style: Cleanup generic font-family handling.

To be more similar between Rust and C++. This introduces GenericFontFamily and
exposes that plus FontFamilyNameSyntax to C++, using that where appropriate
instead of plain uint8_t as we were doing.

As a follow-up, as discussed on IRC with Jonathan, we can remove the -moz-fixed
family, and turn it just into an alias of Monospace.

The only non-trivial change is the MatchType changes, but they're ok I think.
The code already assumed at most one CSS generic, and the struct still takes 8
bits. I've verified that the relevant tests are passing (though try is closed).

Differential Revision: https://phabricator.services.mozilla.com/D24272

components/style/font_metrics.rs

@@ -43,7 +43,7 @@ pub trait FontMetricsProvider {
     }
 
     /// Get default size of a given language and generic family.
-    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au;
+    fn get_size(&self, font_name: &Atom, font_family: crate::values::computed::font::GenericFontFamily) -> Au;
 
     /// Construct from a shared style context
     fn create_from(context: &SharedStyleContext) -> Self

components/style/gecko/wrapper.rs

@@ -69,6 +69,7 @@ use crate::selector_parser::{AttrValue, HorizontalDirection, Lang};
 use crate::shared_lock::Locked;
 use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use crate::stylist::CascadeData;
+use crate::values::computed::font::GenericFontFamily;
 use crate::values::specified::length::FontBaseSize;
 use crate::CaseSensitivityExt;
 use app_units::Au;
@@ -1041,7 +1042,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
         GeckoFontMetricsProvider::new()
     }
 
-    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au {
+    fn get_size(&self, font_name: &Atom, font_family: GenericFontFamily) -> Au {
         let mut cache = self.font_size_cache.borrow_mut();
         if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) {
             return sizes.1.size_for_generic(font_family);
@@ -1100,16 +1101,17 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
 }
 
 impl structs::FontSizePrefs {
-    fn size_for_generic(&self, font_family: u8) -> Au {
+    fn size_for_generic(&self, font_family: GenericFontFamily) -> Au {
         Au(match font_family {
-            structs::kPresContext_DefaultVariableFont_ID => self.mDefaultVariableSize,
+            GenericFontFamily::None => self.mDefaultVariableSize,
-            structs::kPresContext_DefaultFixedFont_ID => self.mDefaultFixedSize,
+            GenericFontFamily::Serif => self.mDefaultSerifSize,
-            structs::kGenericFont_serif => self.mDefaultSerifSize,
+            GenericFontFamily::SansSerif => self.mDefaultSansSerifSize,
-            structs::kGenericFont_sans_serif => self.mDefaultSansSerifSize,
+            GenericFontFamily::Monospace => self.mDefaultMonospaceSize,
-            structs::kGenericFont_monospace => self.mDefaultMonospaceSize,
+            GenericFontFamily::Cursive => self.mDefaultCursiveSize,
-            structs::kGenericFont_cursive => self.mDefaultCursiveSize,
+            GenericFontFamily::Fantasy => self.mDefaultFantasySize,
-            structs::kGenericFont_fantasy => self.mDefaultFantasySize,
+            GenericFontFamily::MozEmoji => {
-            _ => unreachable!("Unknown generic ID"),
+                unreachable!("Should never get here, since this doesn't (yet) appear on font family")
+            },
         })
     }
 }

components/style/properties/cascade.rs

@@ -683,6 +683,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
     #[cfg(feature = "gecko")]
     fn recompute_default_font_family_type_if_needed(&mut self) {
         use crate::gecko_bindings::{bindings, structs};
+        use crate::values::computed::font::GenericFontFamily;
 
         if !self.seen.contains(LonghandId::XLang) &&
            !self.seen.contains(LonghandId::FontFamily) {
@@ -697,7 +698,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
             // System fonts are all right, and should have the default font type
             // set to none already, so bail out early.
             if font.mFont.systemFont {
-                debug_assert_eq!(font.mFont.fontlist.mDefaultFontType, structs::FontFamilyType::eFamily_none);
+                debug_assert_eq!(font.mFont.fontlist.mDefaultFontType, GenericFontFamily::None);
                 return;
             }
 
@@ -717,11 +718,11 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 !use_document_fonts &&
                 matches!(
                     font.mGenericID,
-                    structs::kGenericFont_NONE |
+                    GenericFontFamily::None |
-                    structs::kGenericFont_fantasy |
+                    GenericFontFamily::Fantasy |
-                    structs::kGenericFont_cursive
+                    GenericFontFamily::Cursive
                 ) &&
-                default_font_type != structs::FontFamilyType::eFamily_none;
+                default_font_type != GenericFontFamily::None;
 
             if !prioritize_user_fonts && default_font_type == font.mFont.fontlist.mDefaultFontType {
                 // Nothing to do.

components/style/properties/gecko.mako.rs

@@ -1984,20 +1984,20 @@ fn static_assert() {
     <% impl_font_settings("font_variation_settings", "gfxFontVariation", "VariationValue", "f32", "f32") %>
 
     pub fn set_font_family(&mut self, v: longhands::font_family::computed_value::T) {
-        use crate::gecko_bindings::structs::FontFamilyType;
+        use crate::values::computed::font::GenericFontFamily;
 
         let is_system_font = v.is_system_font;
         self.gecko.mFont.systemFont = is_system_font;
         self.gecko.mGenericID = if is_system_font {
-            structs::kGenericFont_NONE
+            GenericFontFamily::None
         } else {
-            v.families.single_generic().unwrap_or(structs::kGenericFont_NONE)
+            v.families.single_generic().unwrap_or(GenericFontFamily::None)
         };
         self.gecko.mFont.fontlist.mFontlist.mBasePtr.set_move(
             v.families.shared_font_list().clone()
         );
         // Fixed-up if needed in Cascade::fixup_font_stuff.
-        self.gecko.mFont.fontlist.mDefaultFontType = FontFamilyType::eFamily_none;
+        self.gecko.mFont.fontlist.mDefaultFontType = GenericFontFamily::None;
     }
 
     pub fn copy_font_family_from(&mut self, other: &Self) {
@@ -2011,33 +2011,13 @@ fn static_assert() {
     }
 
     pub fn clone_font_family(&self) -> longhands::font_family::computed_value::T {
-        use crate::gecko_bindings::structs::FontFamilyType;
         use crate::values::computed::font::{FontFamily, SingleFontFamily, FontFamilyList};
 
         let fontlist = &self.gecko.mFont.fontlist;
         let shared_fontlist = unsafe { fontlist.mFontlist.mBasePtr.to_safe() };
 
         let families = if shared_fontlist.mNames.is_empty() {
-            let default = fontlist.mDefaultFontType;
+            let default = SingleFontFamily::Generic(fontlist.mDefaultFontType);
-            let default = match default {
-                FontFamilyType::eFamily_serif => {
-                    SingleFontFamily::Generic(atom!("serif"))
-                }
-                _ => {
-                    // This can break with some combinations of user prefs, see
-                    // bug 1442195 for example. It doesn't really matter in this
-                    // case...
-                    //
-                    // FIXME(emilio): Probably should be storing the whole
-                    // default font name instead though.
-                    debug_assert_eq!(
-                        default,
-                        FontFamilyType::eFamily_sans_serif,
-                        "Default generic should be serif or sans-serif"
-                    );
-                    SingleFontFamily::Generic(atom!("sans-serif"))
-                }
-            };
             FontFamilyList::new(Box::new([default]))
         } else {
             FontFamilyList::SharedFontList(shared_fontlist)

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

@@ -326,7 +326,7 @@ ${helpers.predefined_type(
 
         use app_units::Au;
         use cssparser::{Parser, ToCss};
-        use crate::gecko_bindings::structs::FontFamilyType;
+        use crate::values::computed::font::GenericFontFamily;
         use crate::properties::longhands;
         use std::fmt;
         use std::hash::{Hash, Hasher};
@@ -466,7 +466,7 @@ ${helpers.predefined_type(
                 pub ${name}: longhands::${name}::computed_value::T,
             % endfor
             pub system_font: SystemFont,
-            pub default_font_type: FontFamilyType,
+            pub default_font_type: GenericFontFamily,
         }
 
         impl SystemFont {

components/style/values/computed/font.rs

@@ -157,10 +157,6 @@ impl FontSize {
     }
 }
 
-/// XXXManishearth it might be better to
-/// animate this as computed, however this complicates
-/// clamping and might not be the right thing to do.
-/// We should figure it out.
 impl ToAnimatedValue for FontSize {
     type AnimatedValue = NonNegativeLength;
 
@@ -178,8 +174,8 @@ impl ToAnimatedValue for FontSize {
     }
 }
 
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
-#[cfg_attr(feature = "servo", derive(MallocSizeOf))]
+#[cfg_attr(feature = "servo", derive(Hash, MallocSizeOf))]
 /// Specifies a prioritized list of font family names or generic family names.
 pub struct FontFamily {
     /// The actual list of family names.
@@ -193,7 +189,7 @@ impl FontFamily {
     /// Get default font family as `serif` which is a generic font-family
     pub fn serif() -> Self {
         FontFamily {
-            families: FontFamilyList::new(Box::new([SingleFontFamily::Generic(atom!("serif"))])),
+            families: FontFamilyList::new(Box::new([SingleFontFamily::Generic(GenericFontFamily::Serif)])),
             is_system_font: false,
         }
     }
@@ -232,7 +228,7 @@ pub struct FamilyName {
     /// Name of the font family
     pub name: Atom,
     /// Syntax of the font family
-    pub syntax: FamilyNameSyntax,
+    pub syntax: FontFamilyNameSyntax,
 }
 
 impl ToCss for FamilyName {
@@ -241,12 +237,12 @@ impl ToCss for FamilyName {
         W: fmt::Write,
     {
         match self.syntax {
-            FamilyNameSyntax::Quoted => {
+            FontFamilyNameSyntax::Quoted => {
                 dest.write_char('"')?;
                 write!(CssStringWriter::new(dest), "{}", self.name)?;
                 dest.write_char('"')
             },
-            FamilyNameSyntax::Identifiers => {
+            FontFamilyNameSyntax::Identifiers => {
                 let mut first = true;
                 for ident in self.name.to_string().split(' ') {
                     if first {
@@ -268,11 +264,12 @@ impl ToCss for FamilyName {
     }
 }
 
-#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
+#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 /// Font family names must either be given quoted as strings,
 /// or unquoted as a sequence of one or more identifiers.
-pub enum FamilyNameSyntax {
+#[repr(u8)]
+pub enum FontFamilyNameSyntax {
     /// The family name was specified in a quoted form, e.g. "Font Name"
     /// or 'Font Name'.
     Quoted,
@@ -282,85 +279,79 @@ pub enum FamilyNameSyntax {
     Identifiers,
 }
 
-#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToCss)]
-#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
+#[cfg_attr(feature = "servo", derive(Deserialize, Serialize, Hash))]
 /// A set of faces that vary in weight, width or slope.
 pub enum SingleFontFamily {
     /// The name of a font family of choice.
     FamilyName(FamilyName),
     /// Generic family name.
-    Generic(Atom),
+    Generic(GenericFontFamily),
+}
+
+/// A generic font-family name.
+///
+/// The order here is important, if you change it make sure that
+/// `gfxPlatformFontList.h`s ranged array and `gfxFontFamilyList`'s
+/// sSingleGenerics are updated as well.
+#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, Parse, ToCss)]
+#[repr(u8)]
+#[allow(missing_docs)]
+pub enum GenericFontFamily {
+    /// No generic family specified, only for internal usage.
+    #[css(skip)]
+    None,
+    Serif,
+    SansSerif,
+    Monospace,
+    Cursive,
+    Fantasy,
+    /// This is basically monospace, but with different font prefs. We should
+    /// consider removing it in favor of just using the monospace.* prefs, since
+    /// in practice we don't override the monospace prefs at all.
+    #[cfg(feature = "gecko")]
+    MozFixed,
+    /// An internal value for emoji font selection.
+    #[css(skip)]
+    #[cfg(feature = "gecko")]
+    MozEmoji,
+}
+
+// TODO(emilio): Derive this when we make -moz-fixed a parse alias of Monospace.
+impl ToCss for GenericFontFamily {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        dest.write_str(match *self {
+            GenericFontFamily::MozEmoji |
+            GenericFontFamily::Default => return Ok(()),
+            GenericFontFamily::MozFixed |
+            GenericFontFamily::Monospace => "monospace",
+            GenericFontFamily::Serif => "serif",
+            GenericFontFamily::SansSerif => "sans-serif",
+            GenericFontFamily::Cursive => "cursive",
+            GenericFontFamily::Fantasy => "fantasy",
+        })
+    }
 }
 
 impl SingleFontFamily {
-    #[inline]
+    /// Parse a font-family value.
-    /// Get font family name as Atom
-    pub fn atom(&self) -> &Atom {
-        match *self {
-            SingleFontFamily::FamilyName(ref family_name) => &family_name.name,
-            SingleFontFamily::Generic(ref name) => name,
-        }
-    }
-
-    #[inline]
-    #[cfg(not(feature = "gecko"))] // Gecko can't borrow atoms as UTF-8.
-    /// Get font family name
-    pub fn name(&self) -> &str {
-        self.atom()
-    }
-
-    #[cfg(not(feature = "gecko"))] // Gecko can't borrow atoms as UTF-8.
-    /// Get the corresponding font-family with Atom
-    pub fn from_atom(input: Atom) -> SingleFontFamily {
-        match input {
-            atom!("serif") |
-            atom!("sans-serif") |
-            atom!("cursive") |
-            atom!("fantasy") |
-            atom!("monospace") => return SingleFontFamily::Generic(input),
-            _ => {},
-        }
-        match_ignore_ascii_case! { &input,
-            "serif" => return SingleFontFamily::Generic(atom!("serif")),
-            "sans-serif" => return SingleFontFamily::Generic(atom!("sans-serif")),
-            "cursive" => return SingleFontFamily::Generic(atom!("cursive")),
-            "fantasy" => return SingleFontFamily::Generic(atom!("fantasy")),
-            "monospace" => return SingleFontFamily::Generic(atom!("monospace")),
-            _ => {}
-        }
-
-        // We don't know if it's quoted or not. So we set it to
-        // quoted by default.
-        SingleFontFamily::FamilyName(FamilyName {
-            name: input,
-            syntax: FamilyNameSyntax::Quoted,
-        })
-    }
-
-    /// Parse a font-family value
     pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         if let Ok(value) = input.try(|i| i.expect_string_cloned()) {
             return Ok(SingleFontFamily::FamilyName(FamilyName {
                 name: Atom::from(&*value),
-                syntax: FamilyNameSyntax::Quoted,
+                syntax: FontFamilyNameSyntax::Quoted,
             }));
         }
-        let first_ident = input.expect_ident()?.clone();
 
-        // FIXME(bholley): The fast thing to do here would be to look up the
+        let first_ident = input.expect_ident_cloned()?;
-        // string (as lowercase) in the static atoms table. We don't have an
+        if let Ok(generic) = GenericFontFamily::from_ident(&first_ident) {
-        // API to do that yet though, so we do the simple thing for now.
+            return Ok(SingleFontFamily::Generic(generic));
-        let mut css_wide_keyword = false;
+        }
-        match_ignore_ascii_case! { &first_ident,
-            "serif" => return Ok(SingleFontFamily::Generic(atom!("serif"))),
-            "sans-serif" => return Ok(SingleFontFamily::Generic(atom!("sans-serif"))),
-            "cursive" => return Ok(SingleFontFamily::Generic(atom!("cursive"))),
-            "fantasy" => return Ok(SingleFontFamily::Generic(atom!("fantasy"))),
-            "monospace" => return Ok(SingleFontFamily::Generic(atom!("monospace"))),
-
-            #[cfg(feature = "gecko")]
-            "-moz-fixed" => return Ok(SingleFontFamily::Generic(atom!("-moz-fixed"))),
 
+        let reserved = match_ignore_ascii_case! { &first_ident,
             // https://drafts.csswg.org/css-fonts/#propdef-font-family
             // "Font family names that happen to be the same as a keyword value
             //  (`inherit`, `serif`, `sans-serif`, `monospace`, `fantasy`, and `cursive`)
@@ -368,18 +359,15 @@ impl SingleFontFamily {
             //  The keywords ‘initial’ and ‘default’ are reserved for future use
             //  and must also be quoted when used as font names.
             //  UAs must not consider these keywords as matching the <family-name> type."
-            "inherit" => css_wide_keyword = true,
+            "inherit" | "initial" | "unset" | "revert" | "default" => true,
-            "initial" => css_wide_keyword = true,
+            _ => false,
-            "unset" => css_wide_keyword = true,
+        };
-            "default" => css_wide_keyword = true,
-            _ => {}
-        }
 
         let mut value = first_ident.as_ref().to_owned();
 
         // These keywords are not allowed by themselves.
         // The only way this value can be valid with with another keyword.
-        if css_wide_keyword {
+        if reserved {
             let ident = input.expect_ident()?;
             value.push(' ');
             value.push_str(&ident);
@@ -393,9 +381,9 @@ impl SingleFontFamily {
             // `font-family: \ a\ \ b\ \ c\ ;`, it is tricky to serialize them
             // as identifiers correctly. Just mark them quoted so we don't need
             // to worry about them in serialization code.
-            FamilyNameSyntax::Quoted
+            FontFamilyNameSyntax::Quoted
         } else {
-            FamilyNameSyntax::Identifiers
+            FontFamilyNameSyntax::Identifiers
         };
         Ok(SingleFontFamily::FamilyName(FamilyName {
             name: Atom::from(value),
@@ -403,94 +391,18 @@ impl SingleFontFamily {
         }))
     }
 
-    #[cfg(feature = "gecko")]
-    /// Return the generic ID for a given generic font name
-    pub fn generic(name: &Atom) -> (structs::FontFamilyType, u8) {
-        use crate::gecko_bindings::structs::FontFamilyType;
-        if *name == atom!("serif") {
-            (FontFamilyType::eFamily_serif, structs::kGenericFont_serif)
-        } else if *name == atom!("sans-serif") {
-            (
-                FontFamilyType::eFamily_sans_serif,
-                structs::kGenericFont_sans_serif,
-            )
-        } else if *name == atom!("cursive") {
-            (
-                FontFamilyType::eFamily_cursive,
-                structs::kGenericFont_cursive,
-            )
-        } else if *name == atom!("fantasy") {
-            (
-                FontFamilyType::eFamily_fantasy,
-                structs::kGenericFont_fantasy,
-            )
-        } else if *name == atom!("monospace") {
-            (
-                FontFamilyType::eFamily_monospace,
-                structs::kGenericFont_monospace,
-            )
-        } else if *name == atom!("-moz-fixed") {
-            (
-                FontFamilyType::eFamily_moz_fixed,
-                structs::kGenericFont_moz_fixed,
-            )
-        } else {
-            panic!("Unknown generic {}", name);
-        }
-    }
-
     #[cfg(feature = "gecko")]
     /// Get the corresponding font-family with family name
     fn from_font_family_name(family: &structs::FontFamilyName) -> SingleFontFamily {
-        use crate::gecko_bindings::structs::FontFamilyType;
+        if family.mName.mRawPtr.is_null() {
-
+            debug_assert_ne!(family.mGeneric, GenericFontFamily::None);
-        match family.mType {
+            return SingleFontFamily::Generic(family.mGeneric);
-            FontFamilyType::eFamily_sans_serif => SingleFontFamily::Generic(atom!("sans-serif")),
+        }
-            FontFamilyType::eFamily_serif => SingleFontFamily::Generic(atom!("serif")),
-            FontFamilyType::eFamily_monospace => SingleFontFamily::Generic(atom!("monospace")),
-            FontFamilyType::eFamily_cursive => SingleFontFamily::Generic(atom!("cursive")),
-            FontFamilyType::eFamily_fantasy => SingleFontFamily::Generic(atom!("fantasy")),
-            FontFamilyType::eFamily_moz_fixed => SingleFontFamily::Generic(atom!("-moz-fixed")),
-            FontFamilyType::eFamily_named => {
         let name = unsafe { Atom::from_raw(family.mName.mRawPtr) };
         SingleFontFamily::FamilyName(FamilyName {
             name,
-                    syntax: FamilyNameSyntax::Identifiers,
+            syntax: family.mSyntax,
         })
-            },
-            FontFamilyType::eFamily_named_quoted => {
-                let name = unsafe { Atom::from_raw(family.mName.mRawPtr) };
-                SingleFontFamily::FamilyName(FamilyName {
-                    name,
-                    syntax: FamilyNameSyntax::Quoted,
-                })
-            },
-            _ => panic!("Found unexpected font FontFamilyType"),
-        }
-    }
-}
-
-impl ToCss for SingleFontFamily {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: fmt::Write,
-    {
-        match *self {
-            SingleFontFamily::FamilyName(ref name) => name.to_css(dest),
-
-            // All generic values accepted by the parser are known to not require escaping.
-            SingleFontFamily::Generic(ref name) => {
-                #[cfg(feature = "gecko")]
-                {
-                    // We should treat -moz-fixed as monospace
-                    if name == &atom!("-moz-fixed") {
-                        return dest.write_str("monospace");
-                    }
-                }
-
-                write!(dest, "{}", name)
-            },
-        }
     }
 }
 
@@ -506,7 +418,7 @@ pub enum FontFamilyList {
     /// A strong reference to a Gecko SharedFontList object.
     SharedFontList(RefPtr<structs::SharedFontList>),
     /// A font-family generic ID.
-    Generic(structs::FontFamilyType),
+    Generic(GenericFontFamily),
 }
 
 #[cfg(feature = "gecko")]
@@ -514,38 +426,19 @@ impl ToShmem for FontFamilyList {
     fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
         // In practice, the only SharedFontList objects we create from shared
         // style sheets are ones with a single generic entry.
-        ManuallyDrop::new(match self {
+        ManuallyDrop::new(match *self {
-            FontFamilyList::SharedFontList(r) => {
+            FontFamilyList::SharedFontList(ref r) => {
                 assert!(
                     r.mNames.len() == 1 && r.mNames[0].mName.mRawPtr.is_null(),
                     "ToShmem failed for FontFamilyList: cannot handle non-generic families",
                 );
-                FontFamilyList::Generic(r.mNames[0].mType)
+                FontFamilyList::Generic(r.mNames[0].mGeneric)
             },
-            FontFamilyList::Generic(t) => FontFamilyList::Generic(*t),
+            FontFamilyList::Generic(t) => FontFamilyList::Generic(t),
         })
     }
 }
 
-#[cfg(feature = "gecko")]
-impl Hash for FontFamilyList {
-    fn hash<H>(&self, state: &mut H)
-    where
-        H: Hasher,
-    {
-        use crate::string_cache::WeakAtom;
-
-        for name in self.shared_font_list().mNames.iter() {
-            name.mType.hash(state);
-            if !name.mName.mRawPtr.is_null() {
-                unsafe {
-                    WeakAtom::new(name.mName.mRawPtr).hash(state);
-                }
-            }
-        }
-    }
-}
-
 #[cfg(feature = "gecko")]
 impl PartialEq for FontFamilyList {
     fn eq(&self, other: &FontFamilyList) -> bool {
@@ -556,7 +449,8 @@ impl PartialEq for FontFamilyList {
             return false;
         }
         for (a, b) in self_list.mNames.iter().zip(other_list.mNames.iter()) {
-            if a.mType != b.mType || a.mName.mRawPtr != b.mName.mRawPtr {
+            if a.mSyntax != b.mSyntax || a.mName.mRawPtr != b.mName.mRawPtr ||
+                a.mGeneric != b.mGeneric {
                 return false;
             }
         }
@@ -588,19 +482,17 @@ impl FontFamilyList {
         for family in families.iter() {
             match *family {
                 SingleFontFamily::FamilyName(ref f) => {
-                    let quoted = matches!(f.syntax, FamilyNameSyntax::Quoted);
                     unsafe {
                         bindings::Gecko_nsTArray_FontFamilyName_AppendNamed(
                             names,
                             f.name.as_ptr(),
-                            quoted,
+                            f.syntax,
                         );
                     }
                 },
-                SingleFontFamily::Generic(ref name) => {
+                SingleFontFamily::Generic(family) => {
-                    let (family_type, _generic) = SingleFontFamily::generic(name);
                     unsafe {
-                        bindings::Gecko_nsTArray_FontFamilyName_AppendGeneric(names, family_type);
+                        bindings::Gecko_nsTArray_FontFamilyName_AppendGeneric(names, family);
                     }
                 },
             }
@@ -625,12 +517,11 @@ impl FontFamilyList {
     }
 
     /// Return the generic ID if it is a single generic font
-    #[cfg(feature = "gecko")]
+    pub fn single_generic(&self) -> Option<GenericFontFamily> {
-    pub fn single_generic(&self) -> Option<u8> {
         let mut iter = self.iter();
-        if let Some(SingleFontFamily::Generic(ref name)) = iter.next() {
+        if let Some(SingleFontFamily::Generic(f)) = iter.next() {
             if iter.next().is_none() {
-                return Some(SingleFontFamily::generic(name).1);
+                return Some(f);
             }
         }
         None
@@ -639,13 +530,12 @@ impl FontFamilyList {
     /// Return a reference to the Gecko SharedFontList.
     #[cfg(feature = "gecko")]
     pub fn shared_font_list(&self) -> &RefPtr<structs::SharedFontList> {
-        match self {
+        match *self {
-            FontFamilyList::SharedFontList(r) => r,
+            FontFamilyList::SharedFontList(ref r) => r,
             FontFamilyList::Generic(t) => {
                 unsafe {
                     // TODO(heycam): Should really add StaticRefPtr sugar.
-                    let index =
+                    let index = t as usize;
-                        (*t as usize) - (structs::FontFamilyType::eFamily_generic_first as usize);
                     mem::transmute::<
                         &structs::StaticRefPtr<structs::SharedFontList>,
                         &RefPtr<structs::SharedFontList>,

components/style/values/specified/font.rs

@@ -515,7 +515,8 @@ impl From<LengthPercentage> for FontSize {
 }
 
 /// Specifies a prioritized list of font family names or generic family names.
-#[derive(Clone, Debug, Eq, Hash, PartialEq, ToCss, ToShmem)]
+#[derive(Clone, Debug, Eq, PartialEq, ToCss, ToShmem)]
+#[cfg_attr(feature = "servo", derive(Hash))]
 pub enum FontFamily {
     /// List of `font-family`
     #[css(comma)]