Don't rely on font hint for format detection. Handle parsing comma

separated format hints. Fix oversight in mac code dealing with
creating web fonts from memory.

7 files changed, 157 insertions, 117 deletions

diff --git a/src/components/gfx/font_cache_task.rs b/src/components/gfx/font_cache_task.rs
index dd1771881a6..bb071f38f82 100644
--- a/src/components/gfx/font_cache_task.rs
+++ b/src/components/gfx/font_cache_task.rs
@@ -42,6 +42,16 @@ impl FontFamily {
             }
         }
 
+        // If a request is made for a font family that exists,
+        // pick the first valid font in the family if we failed
+        // to find an exact match for the descriptor.
+        for template in self.templates.mut_iter() {
+            let maybe_template = template.get();
+            if maybe_template.is_some() {
+                return maybe_template;
+            }
+        }
+
         None
     }
 
@@ -60,7 +70,7 @@ impl FontFamily {
 /// Commands that the FontContext sends to the font cache task.
 pub enum Command {
     GetFontTemplate(String, FontTemplateDescriptor, Sender<Reply>),
-    AddWebFont(Url, String, Sender<()>),
+    AddWebFont(Vec<Url>, String, Sender<()>),
     Exit(Sender<()>),
 }
 
@@ -88,7 +98,6 @@ impl FontCache {
             match msg {
                 GetFontTemplate(family, descriptor, result) => {
                     let maybe_font_template = self.get_font_template(&family, &descriptor);
-
                     let font_template = match maybe_font_template {
                         Some(font_template) => font_template,
                         None => self.get_last_resort_template(&descriptor),
@@ -96,19 +105,21 @@ impl FontCache {
 
                     result.send(GetFontTemplateReply(font_template));
                 }
-                AddWebFont(url, family_name, result) => {
-                    let maybe_resource = load_whole_resource(&self.resource_task, url.clone());
-                    match maybe_resource {
-                        Ok((_, bytes)) => {
-                            if !self.web_families.contains_key(&family_name) {
-                                let family = FontFamily::new();
-                                self.web_families.insert(family_name.clone(), family);
+                AddWebFont(urls, family_name, result) => {
+                    for url in urls.iter() {
+                        let maybe_resource = load_whole_resource(&self.resource_task, url.clone());
+                        match maybe_resource {
+                            Ok((_, bytes)) => {
+                                if !self.web_families.contains_key(&family_name) {
+                                    let family = FontFamily::new();
+                                    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));
+                            },
+                            Err(msg) => {
+                                fail!(msg);
                             }
-                            let family = self.web_families.get_mut(&family_name);
-                            family.add_template(format!("{}", url).as_slice(), Some(bytes));
-                        },
-                        Err(msg) => {
-                            fail!(msg);
                         }
                     }
                     result.send(());
@@ -166,7 +177,7 @@ impl FontCache {
         }
     }
 
-    fn find_font_in_web_family<'a>(&'a mut self, family_name: &String, desc: &FontTemplateDescriptor) 
+    fn find_font_in_web_family<'a>(&'a mut self, family_name: &String, desc: &FontTemplateDescriptor)
                                 -> Option<Arc<FontTemplateData>> {
         if self.web_families.contains_key(family_name) {
             let family = self.web_families.get_mut(family_name);
@@ -253,9 +264,9 @@ impl FontCacheTask {
         }
     }
 
-    pub fn add_web_font(&mut self, url: Url, family: &str) {
+    pub fn add_web_font(&mut self, urls: Vec<Url>, family: &str) {
         let (response_chan, response_port) = channel();
-        self.chan.send(AddWebFont(url, family.to_string(), response_chan));
+        self.chan.send(AddWebFont(urls, family.to_string(), response_chan));
         response_port.recv();
     }
 
diff --git a/src/components/gfx/font_context.rs b/src/components/gfx/font_context.rs
index a9cccae61fb..0a1ef69e0ce 100644
--- a/src/components/gfx/font_context.rs
+++ b/src/components/gfx/font_context.rs
@@ -34,7 +34,7 @@ fn create_scaled_font(backend: BackendType, template: &Arc<FontTemplateData>, pt
 
 #[cfg(target_os="macos")]
 fn create_scaled_font(backend: BackendType, template: &Arc<FontTemplateData>, pt_size: f64) -> ScaledFont {
-    let cgfont = template.ctfont.copy_to_CGFont();
+    let cgfont = template.ctfont.get_ref().copy_to_CGFont();
     ScaledFont::new(backend, &cgfont, pt_size as AzFloat)
 }
 
diff --git a/src/components/gfx/font_template.rs b/src/components/gfx/font_template.rs
index c25a104a06b..602d70cff04 100644
--- a/src/components/gfx/font_template.rs
+++ b/src/components/gfx/font_template.rs
@@ -44,6 +44,7 @@ pub struct FontTemplate {
     descriptor: Option<FontTemplateDescriptor>,
     weak_ref: Option<Weak<FontTemplateData>>,
     strong_ref: Option<Arc<FontTemplateData>>,      // GWTODO: Add code path to unset the strong_ref for web fonts!
+    is_valid: bool,
 }
 
 /// Holds all of the template information for a font that
@@ -71,6 +72,7 @@ impl FontTemplate {
             descriptor: None,
             weak_ref: maybe_weak_ref,
             strong_ref: maybe_strong_ref,
+            is_valid: true,
         }
     }
 
@@ -95,23 +97,32 @@ impl FontTemplate {
                 }
             },
             None => {
-                let data = self.get_data();
-                let handle: Result<FontHandle, ()> = FontHandleMethods::new_from_template(fctx, data.clone(), None);
-                match handle {
-                    Ok(handle) => {
-                        let actual_desc = FontTemplateDescriptor::new(handle.boldness(),
-                                            handle.is_italic());
-                        let desc_match = actual_desc == *requested_desc;
-
-                        self.descriptor = Some(actual_desc);
-                        if desc_match {
-                            Some(data)
-                        } else {
-                            None
+                match self.is_valid {
+                    true => {
+                        let data = self.get_data();
+                        let handle: Result<FontHandle, ()> = FontHandleMethods::new_from_template(fctx, data.clone(), None);
+                        match handle {
+                            Ok(handle) => {
+                                let actual_desc = FontTemplateDescriptor::new(handle.boldness(),
+                                                    handle.is_italic());
+                                let desc_match = actual_desc == *requested_desc;
+
+                                self.descriptor = Some(actual_desc);
+                                self.is_valid = true;
+                                if desc_match {
+                                    Some(data)
+                                } else {
+                                    None
+                                }
+                            }
+                            Err(()) => {
+                                self.is_valid = false;
+                                debug!("Unable to create a font from template {}", self.identifier);
+                                None
+                            }
                         }
-                    }
-                    Err(()) => {
-                        debug!("Unable to create a font from template {}", self.identifier);
+                    },
+                    false => {
                         None
                     }
                 }
@@ -119,6 +130,14 @@ impl FontTemplate {
         }
     }
 
+    /// Get the data for creating a font.
+    pub fn get(&mut self) -> Option<Arc<FontTemplateData>> {
+        match self.is_valid {
+            true => Some(self.get_data()),
+            false => None
+        }
+    }
+
     /// Get the font template data. If any strong references still
     /// exist, it will return a clone, otherwise it will load the
     /// font data and store a weak reference to it internally.
diff --git a/src/components/gfx/platform/macos/font.rs b/src/components/gfx/platform/macos/font.rs
index 9ff32f0dee9..cefeda94cda 100644
--- a/src/components/gfx/platform/macos/font.rs
+++ b/src/components/gfx/platform/macos/font.rs
@@ -65,13 +65,17 @@ impl FontHandleMethods for FontHandle {
             Some(s) => s,
             None => 0.0
         };
-        let ct_result = core_text::font::new_from_name(template.identifier.as_slice(), size);
-        ct_result.and_then(|ctfont| {
-            Ok(FontHandle {
-                font_data: template.clone(),
-                ctfont: ctfont,
-            })
-        })
+        match template.ctfont {
+            Some(ref ctfont) => {
+                Ok(FontHandle {
+                    font_data: template.clone(),
+                    ctfont: ctfont.clone_with_font_size(size),
+                })
+            }
+            None => {
+                Err(())
+            }
+        }
     }
 
     fn get_template(&self) -> Arc<FontTemplateData> {
diff --git a/src/components/gfx/platform/macos/font_template.rs b/src/components/gfx/platform/macos/font_template.rs
index 44d4e3f8acd..8641d491523 100644
--- a/src/components/gfx/platform/macos/font_template.rs
+++ b/src/components/gfx/platform/macos/font_template.rs
@@ -12,7 +12,7 @@ use core_text;
 /// CTFont object is cached here for use by the
 /// render functions that create CGFont references.
 pub struct FontTemplateData {
-    pub ctfont: CTFont,
+    pub ctfont: Option<CTFont>,
     pub identifier: String,
 }
 
@@ -21,11 +21,14 @@ impl FontTemplateData {
         let ctfont = match font_data {
             Some(bytes) => {
                 let fontprov = CGDataProvider::from_buffer(bytes.as_slice());
-                let cgfont = CGFont::from_data_provider(fontprov);
-                core_text::font::new_from_CGFont(&cgfont, 0.0)
+                let cgfont_result = CGFont::from_data_provider(fontprov);
+                match cgfont_result {
+                    Ok(cgfont) => Some(core_text::font::new_from_CGFont(&cgfont, 0.0)),
+                    Err(_) => None
+                }
             },
             None => {
-                core_text::font::new_from_name(identifier.as_slice(), 0.0).unwrap()
+                Some(core_text::font::new_from_name(identifier.as_slice(), 0.0).unwrap())
             }
         };
 
diff --git a/src/components/layout/layout_task.rs b/src/components/layout/layout_task.rs
index 1c5e842553e..44cfde0225b 100644
--- a/src/components/layout/layout_task.rs
+++ b/src/components/layout/layout_task.rs
@@ -61,7 +61,6 @@ use std::ptr;
 use std::task::TaskBuilder;
 use style::{AuthorOrigin, Stylesheet, Stylist};
 use style::CSSFontFaceRule;
-use style::TtfFormat;
 use sync::{Arc, Mutex};
 use url::Url;
 
@@ -455,20 +454,16 @@ 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) => {
+                    let mut font_urls = vec!();
                     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());
-                                },
-                                _ => {}
-                            }                            
+                            font_urls.push(source.url.clone());
                         }
                     }
+                    self.font_cache_task.add_web_font(font_urls, font_face_rule.family.as_slice());
                 },
                 _ => {}
             }
diff --git a/src/components/style/font_face.rs b/src/components/style/font_face.rs
index df69c78b0b0..353808f3772 100644
--- a/src/components/style/font_face.rs
+++ b/src/components/style/font_face.rs
@@ -22,7 +22,7 @@ pub enum FontFaceFormat {
 
 pub struct FontFaceSource {
     pub url: Url,
-    pub format_hint: FontFaceFormat,
+    pub format_hints: Vec<FontFaceFormat>,
 }
 
 pub struct FontFaceSourceLine {
@@ -35,7 +35,6 @@ pub struct FontFaceRule {
 }
 
 pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_url: &Url) {
-
     let mut maybe_family = None;
     let mut source_lines = vec!();
 
@@ -79,84 +78,95 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
                         'outer: loop {
 
                             // url() or local() should be next
-                            let url = match iter.next() {
+                            let maybe_url = match iter.next() {
                                 Some(&URL(ref string_value)) => {
-                                    parse_url(string_value.as_slice(), Some(base_url.clone()))
+                                    Some(parse_url(string_value.as_slice(), Some(base_url.clone())))
+                                },
+                                Some(&Function(ref string_value, ref _values)) => {
+                                    match string_value.as_slice() {
+                                        "local" => {
+                                            log_css_error(location, "local font face is not supported yet - skipping");
+                                            None
+                                        },
+                                        _ => {
+                                            log_css_error(location, format!("Unexpected token {}", string_value).as_slice());
+                                            syntax_error = true;
+                                            break;
+                                        }
+                                    }
                                 },
                                 _ => {
-                                    log_css_error(location, "Unsupported declaration (local font face is not supported yet)");
+                                    log_css_error(location, "Unsupported declaration type");
                                     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 maybe_format_hint_string = one_component_value(
-                                                                            values.as_slice()).and_then(|c| {
-                                                match c {
-                                                    &String(ref s) => Some(s.as_slice().to_ascii_lower()),
-                                                    _ => None,
-                                                }
-                                            });
-                                            
-                                            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;
+
+                            match maybe_url {
+                                Some(url) => {
+                                    let mut source = FontFaceSource {
+                                        url: url,
+                                        format_hints: vec!(),
+                                    };
+
+                                    // optional format, or comma to start loop again
+                                    match next_token {
+                                        Some(&Function(ref string_value, ref values)) => {
+                                            match string_value.as_slice() {
+                                                "format" => {
+                                                    let mut format_iter = values.as_slice().skip_whitespace();
+
+                                                    loop {
+                                                        let fmt_token = format_iter.next();
+                                                        match fmt_token {
+                                                            Some(&String(ref format_hint)) => {
+                                                                let hint = match format_hint.as_slice() {
+                                                                    "embedded-opentype" => EotFormat,
+                                                                    "woff" => WoffFormat,
+                                                                    "truetype" | "opentype" => TtfFormat,
+                                                                    "svg" => SvgFormat,
+                                                                    _ => UnknownFormat,
+                                                                };
+                                                                source.format_hints.push(hint);
+                                                            },
+                                                            _ => {
+                                                                log_css_error(location, "Unexpected token");
+                                                                syntax_error = true;
+                                                                break 'outer;
+                                                            }
                                                         }
 
-                                                        let source = FontFaceSource {
-                                                            url: url.clone(),
-                                                            format_hint: hint,
-                                                        };
-                                                        sources.push(source);
+                                                        let comma_token = format_iter.next();
+                                                        match comma_token {
+                                                            Some(&Comma) => {},
+                                                            None => {
+                                                                break;
+                                                            }
+                                                            _ => {
+                                                                log_css_error(location, "Unexpected token");
+                                                                syntax_error = true;
+                                                                break 'outer;
+                                                            }
+                                                        }
                                                     }
                                                 },
-                                                None => {
-                                                    log_css_error(location, 
+                                                _ => {
+                                                    log_css_error(location,
                                                                     format!("Unsupported token {}", string_value).as_slice());
                                                     syntax_error = true;
-                                                    break;                                                   
+                                                    break;
                                                 }
                                             }
+                                            next_token = iter.next();
                                         },
-                                        _ => {
-                                            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);
-                                }
+                                },
+                                None => {},
                             }
 
                             // after url or optional format, comes comma or end
@@ -171,9 +181,7 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
                             }
                         }
 
-                        if !syntax_error {
-                            assert!(sources.len() > 0);
-
+                        if !syntax_error && sources.len() > 0 {
                             let source_line = FontFaceSourceLine {
                                 sources: sources
                             };
@@ -195,4 +203,4 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
         };
         parent_rules.push(CSSFontFaceRule(font_face_rule));
     }
-}
\ No newline at end of file
+}