Address review comments.

4 files changed, 126 insertions, 41 deletions

diff --git a/src/components/gfx/font_cache_task.rs b/src/components/gfx/font_cache_task.rs
index 1dd71f9f1f8..dd1771881a6 100644
--- a/src/components/gfx/font_cache_task.rs
+++ b/src/components/gfx/font_cache_task.rs
@@ -105,7 +105,7 @@ impl FontCache {
                                 self.web_families.insert(family_name.clone(), family);
                             }
                             let family = self.web_families.get_mut(&family_name);
-                            family.add_template(format!("{}", url).as_slice(), Some(bytes));                           
+                            family.add_template(format!("{}", url).as_slice(), Some(bytes));
                         },
                         Err(msg) => {
                             fail!(msg);
diff --git a/src/components/layout/layout_task.rs b/src/components/layout/layout_task.rs
index eb7cfade084..1c5e842553e 100644
--- a/src/components/layout/layout_task.rs
+++ b/src/components/layout/layout_task.rs
@@ -455,20 +455,23 @@ impl LayoutTask {
         // Find all font-face rules and notify the font cache of them.
         // GWTODO: Need to handle unloading web fonts (when we handle unloading stylesheets!)
         // GWTODO: Need to handle font-face nested within media rules.
+        // GWTODO: Don't rely on format hint here. Should determine file format from data.
         for rule in sheet.rules.iter() {
             match rule {
                 &CSSFontFaceRule(ref font_face_rule) => {
-                    for source in font_face_rule.sources.iter() {
-                        match source.format {
-                            TtfFormat => {
-                                self.font_cache_task.add_web_font(source.url.clone(), font_face_rule.family.as_slice());
-                            },
-                            _ => {}
+                    for source_line in font_face_rule.source_lines.iter() {
+                        for source in source_line.sources.iter() {
+                            match source.format_hint {
+                                TtfFormat => {
+                                    self.font_cache_task.add_web_font(source.url.clone(), font_face_rule.family.as_slice());
+                                },
+                                _ => {}
+                            }                            
                         }
                     }
                 },
                 _ => {}
-            }            
+            }
         }
 
         self.stylist.add_stylesheet(sheet, AuthorOrigin);
diff --git a/src/components/style/font_face.rs b/src/components/style/font_face.rs
index 495abc19333..df69c78b0b0 100644
--- a/src/components/style/font_face.rs
+++ b/src/components/style/font_face.rs
@@ -22,19 +22,27 @@ pub enum FontFaceFormat {
 
 pub struct FontFaceSource {
     pub url: Url,
-    pub format: FontFaceFormat,
+    pub format_hint: FontFaceFormat,
+}
+
+pub struct FontFaceSourceLine {
+    pub sources: Vec<FontFaceSource>
 }
 
 pub struct FontFaceRule {
     pub family: String,
-    pub sources: Vec<FontFaceSource>,
+    pub source_lines: Vec<FontFaceSourceLine>,
 }
 
 pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_url: &Url) {
-    let mut font_face_rule = FontFaceRule {
-        family: "".to_string(),
-        sources: vec!()
-    };
+
+    let mut maybe_family = None;
+    let mut source_lines = vec!();
+
+    if rule.prelude.as_slice().skip_whitespace().next().is_some() {
+        log_css_error(rule.location, "@font-face prelude contains unexpected characters");
+        return;
+    }
 
     let block = match rule.block {
         Some(block) => block,
@@ -53,9 +61,10 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
                 let name_lower = name.as_slice().to_ascii_lower();
                 match name_lower.as_slice() {
                     "font-family" => {
+                        // FIXME(#2802): Share code with the font-family parser.
                         match one_component_value(value.as_slice()) {
                             Some(&String(ref string_value)) => {
-                                font_face_rule.family = string_value.clone();
+                                maybe_family = Some(string_value.clone());
                             },
                             _ => {
                                 log_css_error(location, format!("Unsupported font-family string {:s}", name).as_slice());
@@ -63,46 +72,113 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
                         }
                     },
                     "src" => {
-                        for component_value in value.as_slice().skip_whitespace() {
-                            match component_value {
-                                &URL(ref string_value) => {
-                                    let font_url = parse_url(string_value.as_slice(), Some(base_url.clone()));
-                                    let src = FontFaceSource { url: font_url, format: UnknownFormat };
-                                    font_face_rule.sources.push(src);
+                        let mut iter = value.as_slice().skip_whitespace();
+                        let mut sources = vec!();
+                        let mut syntax_error = false;
+
+                        'outer: loop {
+
+                            // url() or local() should be next
+                            let url = match iter.next() {
+                                Some(&URL(ref string_value)) => {
+                                    parse_url(string_value.as_slice(), Some(base_url.clone()))
                                 },
-                                &Function(ref string_value, ref values) => {
+                                _ => {
+                                    log_css_error(location, "Unsupported declaration (local font face is not supported yet)");
+                                    syntax_error = true;
+                                    break;
+                                }
+                            };
+
+                            // optional format, or comma to start loop again
+                            let mut next_token = iter.next();
+                            match next_token {
+                                Some(&Function(ref string_value, ref values)) => {
                                     match string_value.as_slice() {
                                         "format" => {
-                                            let format = one_component_value(values.as_slice()).and_then(|c| {
+                                            let maybe_format_hint_string = one_component_value(
+                                                                            values.as_slice()).and_then(|c| {
                                                 match c {
-                                                    &String(ref format_string) => Some(format_string.as_slice().to_ascii_lower()),
+                                                    &String(ref s) => Some(s.as_slice().to_ascii_lower()),
                                                     _ => None,
                                                 }
                                             });
-                                            match font_face_rule.sources.mut_last() {
-                                                Some(source) => {
-                                                    source.format = match format.unwrap_or("".to_string()).as_slice() {
-                                                        "embedded-opentype" => EotFormat,
-                                                        "woff" => WoffFormat,
-                                                        "truetype" | "opentype" => TtfFormat,
-                                                        "svg" => SvgFormat,
-                                                        _ => UnknownFormat,
+                                            
+                                            match maybe_format_hint_string {
+                                                Some(ref format_hint_string) => {
+                                                    let format_hints: Vec<&str> = format_hint_string.as_slice().split(',').collect();
+
+                                                    for format_hint in format_hints.iter() {
+                                                        let format_hint = format_hint.trim();
+                                                        
+                                                        let hint = match format_hint.as_slice() {
+                                                            "embedded-opentype" => EotFormat,
+                                                            "woff" => WoffFormat,
+                                                            "truetype" | "opentype" => TtfFormat,
+                                                            "svg" => SvgFormat,
+                                                            _ => UnknownFormat,
+                                                        };
+
+                                                        if hint == UnknownFormat {
+                                                            log_css_error(location, 
+                                                                    format!("Unknown font format {}", format_hint).as_slice());
+                                                            syntax_error = true;
+                                                            break 'outer;
+                                                        }
+
+                                                        let source = FontFaceSource {
+                                                            url: url.clone(),
+                                                            format_hint: hint,
+                                                        };
+                                                        sources.push(source);
                                                     }
+                                                },
+                                                None => {
+                                                    log_css_error(location, 
+                                                                    format!("Unsupported token {}", string_value).as_slice());
+                                                    syntax_error = true;
+                                                    break;                                                   
                                                 }
-                                                None => {}
-                                            };
-                                        },
-                                        "local" => {
-                                            log_css_error(location, "local font face not supported yet!");
+                                            }
                                         },
                                         _ => {
-                                            log_css_error(location, format!("Unsupported declaration {}", string_value).as_slice());
+                                            log_css_error(location, 
+                                                            format!("Unsupported token {}", string_value).as_slice());
+                                            syntax_error = true;
+                                            break;
                                         }
                                     }
+                                    next_token = iter.next();
                                 },
-                                _ => {}
+                                _ => {
+                                    let source = FontFaceSource {
+                                        url: url,
+                                        format_hint: UnknownFormat,
+                                    };
+                                    sources.push(source);
+                                }
+                            }
+
+                            // after url or optional format, comes comma or end
+                            match next_token {
+                                Some(&Comma) => {},
+                                None => break,
+                                _ => {
+                                    log_css_error(location, "Unexpected token type");
+                                    syntax_error = true;
+                                    break;
+                                }
                             }
                         }
+
+                        if !syntax_error {
+                            assert!(sources.len() > 0);
+
+                            let source_line = FontFaceSourceLine {
+                                sources: sources
+                            };
+                            source_lines.push(source_line);
+                        }
                     },
                     _ => {
                         log_css_error(location, format!("Unsupported declaration {:s}", name).as_slice());
@@ -112,5 +188,11 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
         }
     }
 
-    parent_rules.push(CSSFontFaceRule(font_face_rule));
+    if maybe_family.is_some() && source_lines.len() > 0 {
+        let font_face_rule = FontFaceRule {
+            family: maybe_family.unwrap(),
+            source_lines: source_lines,
+        };
+        parent_rules.push(CSSFontFaceRule(font_face_rule));
+    }
 }
\ No newline at end of file
diff --git a/src/components/style/style.rs b/src/components/style/style.rs
index b34e5559dbb..adabe63b449 100644
--- a/src/components/style/style.rs
+++ b/src/components/style/style.rs
@@ -47,7 +47,7 @@ pub use selectors::{NamespaceConstraint, Selector, CompoundSelector, SimpleSelec
 pub use selectors::{parse_selector_list};
 pub use namespaces::NamespaceMap;
 pub use media_queries::{MediaRule, MediaQueryList, MediaQuery, Device, MediaType, MediaQueryType};
-pub use font_face::{FontFaceFormat, FontFaceRule, FontFaceSource, TtfFormat};
+pub use font_face::{FontFaceFormat, FontFaceRule, FontFaceSource,FontFaceSourceLine, TtfFormat};
 
 mod stylesheets;
 mod errors;