Auto merge of #16224 - servo:valid-fontface, r=upsuper

Make the parser accept @font-face rules without font-family or src.

Fix #16165.

Also, it turns out that the CSSFontFaceRule IDL specified in the css-fonts spec is not web-compatible. Instead browsers implement a .style attribute like in CSSStyleRule: https://github.com/w3c/csswg-drafts/issues/825

This in turn requires preserving data about which descriptors were set or not (distinguishing unset from set to a value that happens to be the initial value), so this commit also makes every field `Option<_>`.

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

7 files changed, 225 insertions, 177 deletions

diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 74c95499006..abc0a5e771a 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -355,20 +355,24 @@ fn add_font_face_rules(stylesheet: &Stylesheet,
                        outstanding_web_fonts_counter: &Arc<AtomicUsize>) {
     if opts::get().load_webfonts_synchronously {
         let (sender, receiver) = ipc::channel().unwrap();
-        stylesheet.effective_font_face_rules(&device, guard, |font_face| {
-            let effective_sources = font_face.effective_sources();
-            font_cache_thread.add_web_font(font_face.family.clone(),
-                                           effective_sources,
-                                           sender.clone());
-            receiver.recv().unwrap();
+        stylesheet.effective_font_face_rules(&device, guard, |rule| {
+            if let Some(font_face) = rule.font_face() {
+                let effective_sources = font_face.effective_sources();
+                font_cache_thread.add_web_font(font_face.family().clone(),
+                                               effective_sources,
+                                               sender.clone());
+                receiver.recv().unwrap();
+            }
         })
     } else {
-        stylesheet.effective_font_face_rules(&device, guard, |font_face| {
-            let effective_sources = font_face.effective_sources();
-            outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst);
-            font_cache_thread.add_web_font(font_face.family.clone(),
-                                          effective_sources,
-                                          (*font_cache_sender).clone());
+        stylesheet.effective_font_face_rules(&device, guard, |rule| {
+            if let Some(font_face) = rule.font_face() {
+                let effective_sources = font_face.effective_sources();
+                outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst);
+                font_cache_thread.add_web_font(font_face.family().clone(),
+                                              effective_sources,
+                                              (*font_cache_sender).clone());
+            }
         })
     }
 }
diff --git a/components/script/dom/webidls/CSSFontFaceRule.webidl b/components/script/dom/webidls/CSSFontFaceRule.webidl
index 3749a79a818..a0a1b8f950b 100644
--- a/components/script/dom/webidls/CSSFontFaceRule.webidl
+++ b/components/script/dom/webidls/CSSFontFaceRule.webidl
@@ -2,16 +2,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-// https://drafts.csswg.org/css-fonts/#cssfontfacerule
+// https://drafts.csswg.org/css-fonts/#cssfontfacerule is unfortunately not web-compatible:
+// https://github.com/w3c/csswg-drafts/issues/825
+
+// https://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSFontFaceRule ,
+// plus extended attributes matching CSSStyleRule
 [Exposed=Window]
 interface CSSFontFaceRule : CSSRule {
-  // attribute DOMString family;
-  // attribute DOMString src;
-  // attribute DOMString style;
-  // attribute DOMString weight;
-  // attribute DOMString stretch;
-  // attribute DOMString unicodeRange;
-  // attribute DOMString variant;
-  // attribute DOMString featureSettings;
+  // [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
 };
 
diff --git a/components/style/font_face.rs b/components/style/font_face.rs
index 2b441ceb0d7..94dcc61212d 100644
--- a/components/style/font_face.rs
+++ b/components/style/font_face.rs
@@ -12,11 +12,11 @@
 use computed_values::{font_style, font_weight, font_stretch};
 use computed_values::font_family::FamilyName;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
+#[cfg(feature = "gecko")] use gecko_bindings::structs::CSSFontFaceDescriptors;
 #[cfg(feature = "gecko")] use cssparser::UnicodeRange;
 use parser::{ParserContext, log_css_error, Parse};
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
-use std::iter;
 use style_traits::{ToCss, OneOrMoreCommaSeparated};
 use values::specified::url::SpecifiedUrl;
 
@@ -74,14 +74,12 @@ impl ToCss for UrlSource {
 /// Parse the block inside a `@font-face` rule.
 ///
 /// Note that the prelude parsing code lives in the `stylesheets` module.
-pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
-                             -> Result<FontFaceData, ()> {
-    let mut rule = FontFaceData::initial();
+pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser) -> FontFaceRuleData {
+    let mut rule = FontFaceRuleData::empty();
     {
         let parser = FontFaceRuleParser {
             context: context,
             rule: &mut rule,
-            missing: MissingDescriptors::new(),
         };
         let mut iter = DeclarationListParser::new(input, parser);
         while let Some(declaration) = iter.next() {
@@ -92,24 +90,27 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
                 log_css_error(iter.input, pos, &*message, context);
             }
         }
-        if iter.parser.missing.any() {
-            return Err(())
-        }
     }
-    Ok(rule)
+    rule
 }
 
+/// A @font-face rule that is known to have font-family and src declarations.
+#[cfg(feature = "servo")]
+pub struct FontFace<'a>(&'a FontFaceRuleData);
+
 /// A list of effective sources that we send over through IPC to the font cache.
+#[cfg(feature = "servo")]
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 pub struct EffectiveSources(Vec<Source>);
 
-impl FontFaceData {
+#[cfg(feature = "servo")]
+impl<'a> FontFace<'a> {
     /// Returns the list of effective sources for that font-face, that is the
     /// sources which don't list any format hint, or the ones which list at
     /// least "truetype" or "opentype".
     pub fn effective_sources(&self) -> EffectiveSources {
-        EffectiveSources(self.sources.iter().rev().filter(|source| {
+        EffectiveSources(self.sources().iter().rev().filter(|source| {
             if let Source::Url(ref url_source) = **source {
                 let hints = &url_source.format_hints;
                 // We support only opentype fonts and truetype is an alias for
@@ -125,7 +126,8 @@ impl FontFaceData {
     }
 }
 
-impl iter::Iterator for EffectiveSources {
+#[cfg(feature = "servo")]
+impl Iterator for EffectiveSources {
     type Item = Source;
     fn next(&mut self) -> Option<Source> {
         self.0.pop()
@@ -134,8 +136,7 @@ impl iter::Iterator for EffectiveSources {
 
 struct FontFaceRuleParser<'a, 'b: 'a> {
     context: &'a ParserContext<'b>,
-    rule: &'a mut FontFaceData,
-    missing: MissingDescriptors,
+    rule: &'a mut FontFaceRuleData,
 }
 
 /// Default methods reject all at rules.
@@ -172,82 +173,53 @@ impl Parse for Source {
     }
 }
 
-macro_rules! font_face_descriptors {
+macro_rules! font_face_descriptors_common {
     (
-        mandatory descriptors = [
-            $( #[$m_doc: meta] $m_name: tt $m_ident: ident: $m_ty: ty = $m_initial: expr, )*
-        ]
-        optional descriptors = [
-            $( #[$o_doc: meta] $o_name: tt $o_ident: ident: $o_ty: ty = $o_initial: expr, )*
-        ]
+        $( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty, )*
     ) => {
         /// Data inside a `@font-face` rule.
         ///
         /// https://drafts.csswg.org/css-fonts/#font-face-rule
         #[derive(Debug, PartialEq, Eq)]
-        pub struct FontFaceData {
-            $(
-                #[$m_doc]
-                pub $m_ident: $m_ty,
-            )*
-            $(
-                #[$o_doc]
-                pub $o_ident: $o_ty,
-            )*
-        }
-
-        struct MissingDescriptors {
+        pub struct FontFaceRuleData {
             $(
-                $m_ident: bool,
+                #[$doc]
+                pub $ident: Option<$ty>,
             )*
         }
 
-        impl MissingDescriptors {
-            fn new() -> Self {
-                MissingDescriptors {
+        impl FontFaceRuleData {
+            fn empty() -> Self {
+                FontFaceRuleData {
                     $(
-                        $m_ident: true,
+                        $ident: None,
                     )*
                 }
             }
 
-            fn any(&self) -> bool {
+            /// Convert to Gecko types
+            #[cfg(feature = "gecko")]
+            pub fn set_descriptors(&self, descriptors: &mut CSSFontFaceDescriptors) {
                 $(
-                    self.$m_ident
-                )||*
-            }
-        }
-
-        impl FontFaceData {
-            fn initial() -> Self {
-                FontFaceData {
-                    $(
-                        $m_ident: $m_initial,
-                    )*
-                    $(
-                        $o_ident: $o_initial,
-                    )*
-                }
+                    if let Some(ref value) = self.$ident {
+                        descriptors.$gecko_ident.set_from(value)
+                    }
+                )*
+                // Leave unset descriptors to eCSSUnit_Null,
+                // FontFaceSet::FindOrCreateUserFontEntryFromFontFace does the defaulting
+                // to initial values.
             }
         }
 
-        impl ToCssWithGuard for FontFaceData {
+        impl ToCssWithGuard for FontFaceRuleData {
             // Serialization of FontFaceRule is not specced.
             fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
             where W: fmt::Write {
                 dest.write_str("@font-face {\n")?;
                 $(
-                    dest.write_str(concat!("  ", $m_name, ": "))?;
-                    ToCss::to_css(&self.$m_ident, dest)?;
-                    dest.write_str(";\n")?;
-                )*
-                $(
-                    // Because of parse_font_face_block,
-                    // this condition is always true for "src" and "font-family".
-                    // But it can be false for other descriptors.
-                    if self.$o_ident != $o_initial {
-                        dest.write_str(concat!("  ", $o_name, ": "))?;
-                        ToCss::to_css(&self.$o_ident, dest)?;
+                    if let Some(ref value) = self.$ident {
+                        dest.write_str(concat!("  ", $name, ": "))?;
+                        ToCss::to_css(value, dest)?;
                         dest.write_str(";\n")?;
                     }
                 )*
@@ -261,13 +233,14 @@ macro_rules! font_face_descriptors {
             fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> {
                 match_ignore_ascii_case! { name,
                     $(
-                        $m_name => {
-                            self.rule.$m_ident = Parse::parse(self.context, input)?;
-                            self.missing.$m_ident = false
-                        },
-                    )*
-                    $(
-                        $o_name => self.rule.$o_ident = Parse::parse(self.context, input)?,
+                        $name => {
+                            // DeclarationParser also calls parse_entirely
+                            // so we’d normally not need to,
+                            // but in this case we do because we set the value as a side effect
+                            // rather than returning it.
+                            let value = input.parse_entirely(|i| Parse::parse(self.context, i))?;
+                            self.rule.$ident = Some(value)
+                        }
                     )*
                     _ => return Err(())
                 }
@@ -277,33 +250,85 @@ macro_rules! font_face_descriptors {
     }
 }
 
+macro_rules! font_face_descriptors {
+    (
+        mandatory descriptors = [
+            $( #[$m_doc: meta] $m_name: tt $m_ident: ident / $m_gecko_ident: ident: $m_ty: ty, )*
+        ]
+        optional descriptors = [
+            $( #[$o_doc: meta] $o_name: tt $o_ident: ident / $o_gecko_ident: ident: $o_ty: ty =
+                $o_initial: expr, )*
+        ]
+    ) => {
+        font_face_descriptors_common! {
+            $( #[$m_doc] $m_name $m_ident / $m_gecko_ident: $m_ty, )*
+            $( #[$o_doc] $o_name $o_ident / $o_gecko_ident: $o_ty, )*
+        }
+
+        impl FontFaceRuleData {
+            /// Per https://github.com/w3c/csswg-drafts/issues/1133 an @font-face rule
+            /// is valid as far as the CSS parser is concerned even if it doesn’t have
+            /// a font-family or src declaration.
+            ///
+            /// However both are required for the rule to represent an actual font face.
+            #[cfg(feature = "servo")]
+            pub fn font_face(&self) -> Option<FontFace> {
+                if $( self.$m_ident.is_some() )&&* {
+                    Some(FontFace(self))
+                } else {
+                    None
+                }
+            }
+        }
+
+        #[cfg(feature = "servo")]
+        impl<'a> FontFace<'a> {
+            $(
+                #[$m_doc]
+                pub fn $m_ident(&self) -> &$m_ty {
+                    self.0 .$m_ident.as_ref().unwrap()
+                }
+            )*
+            $(
+                #[$o_doc]
+                pub fn $o_ident(&self) -> $o_ty {
+                    if let Some(ref value) = self.0 .$o_ident {
+                        value.clone()
+                    } else {
+                        $o_initial
+                    }
+                }
+            )*
+        }
+    }
+}
+
 /// css-name rust_identifier: Type = initial_value,
 #[cfg(feature = "gecko")]
 font_face_descriptors! {
     mandatory descriptors = [
         /// The name of this font face
-        "font-family" family: FamilyName = FamilyName {
-            name: atom!(""),
-            quoted: true,
-        },
+        "font-family" family / mFamily: FamilyName,
 
         /// The alternative sources for this font face.
-        "src" sources: Vec<Source> = Vec::new(),
+        "src" sources / mSrc: Vec<Source>,
     ]
     optional descriptors = [
         /// The style of this font face
-        "font-style" style: font_style::T = font_style::T::normal,
+        "font-style" style / mStyle: font_style::T = font_style::T::normal,
 
         /// The weight of this font face
-        "font-weight" weight: font_weight::T = font_weight::T::Weight400 /* normal */,
+        "font-weight" weight / mWeight: font_weight::T = font_weight::T::Weight400 /* normal */,
 
         /// The stretch of this font face
-        "font-stretch" stretch: font_stretch::T = font_stretch::T::normal,
+        "font-stretch" stretch / mStretch: font_stretch::T = font_stretch::T::normal,
 
         /// The ranges of code points outside of which this font face should not be used.
-        "unicode-range" unicode_range: Vec<UnicodeRange> = vec![
+        "unicode-range" unicode_range / mUnicodeRange: Vec<UnicodeRange> = vec![
             UnicodeRange { start: 0, end: 0x10FFFF }
         ],
+
+        // FIXME: add font-feature-settings, font-language-override, and font-display
     ]
 }
 
@@ -311,13 +336,10 @@ font_face_descriptors! {
 font_face_descriptors! {
     mandatory descriptors = [
         /// The name of this font face
-        "font-family" family: FamilyName = FamilyName {
-            name: atom!(""),
-            quoted: true,
-        },
+        "font-family" family / mFamily: FamilyName,
 
         /// The alternative sources for this font face.
-        "src" sources: Vec<Source> = Vec::new(),
+        "src" sources / mSrc: Vec<Source>,
     ]
     optional descriptors = [
     ]
diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs
index 4b1a331ea21..548266c4c40 100644
--- a/components/style/gecko/rules.rs
+++ b/components/style/gecko/rules.rs
@@ -4,9 +4,13 @@
 
 //! Bindings for CSS Rule objects
 
-use font_face::{FontFaceData, Source};
+use computed_values::{font_style, font_weight, font_stretch};
+use computed_values::font_family::FamilyName;
+use cssparser::UnicodeRange;
+use font_face::{FontFaceRuleData, Source};
 use gecko_bindings::bindings;
-use gecko_bindings::structs::{self, CSSFontFaceDescriptors, nsCSSFontFaceRule};
+use gecko_bindings::structs::{self, nsCSSFontFaceRule, nsCSSValue};
+use gecko_bindings::sugar::ns_css_value::ToNsCssValue;
 use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr};
 use shared_lock::{ToCssWithGuard, SharedRwLockReadGuard};
 use std::fmt;
@@ -14,34 +18,46 @@ use std::fmt;
 /// A @font-face rule
 pub type FontFaceRule = RefPtr<nsCSSFontFaceRule>;
 
-fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
-                             data: FontFaceData) {
-    // font-family
-    descriptors.mFamily.set_string_from_atom(&data.family.name);
+impl ToNsCssValue for FamilyName {
+    fn convert(&self, nscssvalue: &mut nsCSSValue) {
+        nscssvalue.set_string_from_atom(&self.name)
+    }
+}
+
+impl ToNsCssValue for font_weight::T {
+    fn convert(&self, nscssvalue: &mut nsCSSValue) {
+        nscssvalue.set_integer(*self as i32)
+    }
+}
 
-    macro_rules! map_enum {
-        ($target:ident = ($data:ident: $prop:ident) {
-            $($servo:ident => $gecko:ident,)+
-        }) => {{
-            use computed_values::$prop::T;
-            descriptors.$target.set_enum(match data.$data {
-                $( T::$servo => structs::$gecko as i32, )+
-            })
-        }}
+macro_rules! map_enum {
+    (
+        $(
+            $prop:ident {
+                $($servo:ident => $gecko:ident,)+
+            }
+        )+
+    ) => {
+        $(
+            impl ToNsCssValue for $prop::T {
+                fn convert(&self, nscssvalue: &mut nsCSSValue) {
+                    nscssvalue.set_enum(match *self {
+                        $( $prop::T::$servo => structs::$gecko as i32, )+
+                    })
+                }
+            }
+        )+
     }
+}
 
-    // font-style
-    map_enum!(mStyle = (style: font_style) {
+map_enum! {
+    font_style {
         normal => NS_FONT_STYLE_NORMAL,
         italic => NS_FONT_STYLE_ITALIC,
         oblique => NS_FONT_STYLE_OBLIQUE,
-    });
-
-    // font-weight
-    descriptors.mWeight.set_integer(data.weight as i32);
+    }
 
-    // font-stretch
-    map_enum!(mStretch = (stretch: font_stretch) {
+    font_stretch {
         normal          => NS_FONT_STRETCH_NORMAL,
         ultra_condensed => NS_FONT_STRETCH_ULTRA_CONDENSED,
         extra_condensed => NS_FONT_STRETCH_EXTRA_CONDENSED,
@@ -51,60 +67,58 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
         expanded        => NS_FONT_STRETCH_EXPANDED,
         extra_expanded  => NS_FONT_STRETCH_EXTRA_EXPANDED,
         ultra_expanded  => NS_FONT_STRETCH_ULTRA_EXPANDED,
-    });
+    }
+}
 
-    // src
-    let src_len = data.sources.iter().fold(0, |acc, src| {
-        acc + match *src {
-            // Each format hint takes one position in the array of mSrc.
-            Source::Url(ref url) => url.format_hints.len() + 1,
-            Source::Local(_) => 1,
-        }
-    });
-    let mut target_srcs =
-        descriptors.mSrc.set_array(src_len as i32).as_mut_slice().iter_mut();
-    macro_rules! next { () => {
-        target_srcs.next().expect("Length of target_srcs should be enough")
-    } }
-    for src in data.sources.iter() {
-        match *src {
-            Source::Url(ref url) => {
-                next!().set_url(&url.url);
-                for hint in url.format_hints.iter() {
-                    next!().set_font_format(&hint);
-                }
+impl ToNsCssValue for Vec<Source> {
+    fn convert(&self, nscssvalue: &mut nsCSSValue) {
+        let src_len = self.iter().fold(0, |acc, src| {
+            acc + match *src {
+                // Each format hint takes one position in the array of mSrc.
+                Source::Url(ref url) => url.format_hints.len() + 1,
+                Source::Local(_) => 1,
             }
-            Source::Local(ref family) => {
-                next!().set_local_font(&family.name);
+        });
+        let mut target_srcs =
+            nscssvalue.set_array(src_len as i32).as_mut_slice().iter_mut();
+        macro_rules! next { () => {
+            target_srcs.next().expect("Length of target_srcs should be enough")
+        } }
+        for src in self.iter() {
+            match *src {
+                Source::Url(ref url) => {
+                    next!().set_url(&url.url);
+                    for hint in url.format_hints.iter() {
+                        next!().set_font_format(&hint);
+                    }
+                }
+                Source::Local(ref family) => {
+                    next!().set_local_font(&family.name);
+                }
             }
         }
+        debug_assert!(target_srcs.next().is_none(), "Should have filled all slots");
     }
-    debug_assert!(target_srcs.next().is_none(), "Should have filled all slots");
+}
 
-    // unicode-range
-    let target_ranges = descriptors.mUnicodeRange
-        .set_array((data.unicode_range.len() * 2) as i32)
-        .as_mut_slice().chunks_mut(2);
-    for (range, target) in data.unicode_range.iter().zip(target_ranges) {
-        target[0].set_integer(range.start as i32);
-        target[1].set_integer(range.end as i32);
+impl ToNsCssValue for Vec<UnicodeRange> {
+    fn convert(&self, nscssvalue: &mut nsCSSValue) {
+        let target_ranges = nscssvalue
+            .set_array((self.len() * 2) as i32)
+            .as_mut_slice().chunks_mut(2);
+        for (range, target) in self.iter().zip(target_ranges) {
+            target[0].set_integer(range.start as i32);
+            target[1].set_integer(range.end as i32);
+        }
     }
-
-    // The following three descriptors are not implemented yet.
-    // font-feature-settings
-    descriptors.mFontFeatureSettings.set_normal();
-    // font-language-override
-    descriptors.mFontLanguageOverride.set_normal();
-    // font-display
-    descriptors.mDisplay.set_enum(structs::NS_FONT_DISPLAY_AUTO as i32);
 }
 
-impl From<FontFaceData> for FontFaceRule {
-    fn from(data: FontFaceData) -> FontFaceRule {
+impl From<FontFaceRuleData> for FontFaceRule {
+    fn from(data: FontFaceRuleData) -> FontFaceRule {
         let mut result = unsafe {
             UniqueRefPtr::from_addrefed(bindings::Gecko_CSSFontFaceRule_Create())
         };
-        set_font_face_descriptors(&mut result.mDecl.mDescriptors, data);
+        data.set_descriptors(&mut result.mDecl.mDescriptors);
         result.get()
     }
 }
diff --git a/components/style/gecko_bindings/sugar/mod.rs b/components/style/gecko_bindings/sugar/mod.rs
index c531a2efa01..1c004c8fcc1 100644
--- a/components/style/gecko_bindings/sugar/mod.rs
+++ b/components/style/gecko_bindings/sugar/mod.rs
@@ -6,7 +6,7 @@
 
 mod ns_com_ptr;
 mod ns_css_shadow_array;
-mod ns_css_value;
+pub mod ns_css_value;
 mod ns_style_auto_array;
 pub mod ns_style_coord;
 mod ns_t_array;
diff --git a/components/style/gecko_bindings/sugar/ns_css_value.rs b/components/style/gecko_bindings/sugar/ns_css_value.rs
index ff6ad413d50..d22f1148549 100644
--- a/components/style/gecko_bindings/sugar/ns_css_value.rs
+++ b/components/style/gecko_bindings/sugar/ns_css_value.rs
@@ -163,6 +163,11 @@ impl nsCSSValue {
         unsafe { bindings::Gecko_CSSValue_SetArray(self, len) }
         unsafe { self.mValue.mArray.as_mut().as_mut() }.unwrap()
     }
+
+    /// Generic set from any value that implements the ToNsCssValue trait.
+    pub fn set_from<T: ToNsCssValue>(&mut self, value: &T) {
+        value.convert(self)
+    }
 }
 
 impl Drop for nsCSSValue {
@@ -210,3 +215,9 @@ impl IndexMut<usize> for nsCSSValue_Array {
         &mut self.as_mut_slice()[i]
     }
 }
+
+/// Generic conversion to nsCSSValue
+pub trait ToNsCssValue {
+    /// Convert
+    fn convert(&self, nscssvalue: &mut nsCSSValue);
+}
diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs
index ba1ffb5d698..9b75efb52cf 100644
--- a/components/style/stylesheets.rs
+++ b/components/style/stylesheets.rs
@@ -12,7 +12,7 @@ use cssparser::{AtRuleType, RuleListParser, SourcePosition, Token, parse_one_rul
 use cssparser::ToCss as ParserToCss;
 use error_reporting::ParseErrorReporter;
 #[cfg(feature = "servo")]
-use font_face::FontFaceData;
+use font_face::FontFaceRuleData;
 use font_face::parse_font_face_block;
 #[cfg(feature = "gecko")]
 pub use gecko::rules::FontFaceRule;
@@ -557,7 +557,7 @@ impl ToCssWithGuard for StyleRule {
 
 /// A @font-face rule
 #[cfg(feature = "servo")]
-pub type FontFaceRule = FontFaceData;
+pub type FontFaceRule = FontFaceRuleData;
 
 impl Stylesheet {
     /// Updates an empty stylesheet from a given string of text.
@@ -1012,7 +1012,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
         match prelude {
             AtRulePrelude::FontFace => {
                 Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap(
-                    parse_font_face_block(self.context, input)?.into()))))
+                    parse_font_face_block(self.context, input).into()))))
             }
             AtRulePrelude::Media(media_queries) => {
                 Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule {