Auto merge of #8420 - craftytrickster:8371/generic-font-family, r=glennw

Enabled use of FontFamily enum type

https://github.com/servo/servo/issues/8371

In addition to replacing loose strings with the FontFamily enum in `font_cache_task.rs`, I also centralized the add_generic_font calls into one single function. If centralizing into one function is not desired or if anything else needs to be changed, please let me know.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8420)
<!-- Reviewable:end -->

components/gfx/font_cache_task.rs

@@ -19,18 +19,19 @@ use std::sync::mpsc::channel;
 use std::sync::{Arc, Mutex};
 use string_cache::Atom;
 use style::font_face::Source;
+use style::properties::longhands::font_family::computed_value::FontFamily;
 use url::Url;
 use util::str::LowercaseString;
 use util::task::spawn_named;
 
 /// A list of font templates that make up a given font family.
-struct FontFamily {
+struct FontTemplates {
     templates: Vec<FontTemplate>,
 }
 
-impl FontFamily {
-    fn new() -> FontFamily {
-        FontFamily {
+impl FontTemplates {
+    fn new() -> FontTemplates {
+        FontTemplates {
             templates: vec!(),
         }
     }
@@ -79,10 +80,10 @@ impl FontFamily {
 /// Commands that the FontContext sends to the font cache task.
 #[derive(Deserialize, Serialize)]
 pub enum Command {
-    GetFontTemplate(String, FontTemplateDescriptor, IpcSender<Reply>),
+    GetFontTemplate(FontFamily, FontTemplateDescriptor, IpcSender<Reply>),
     GetLastResortFontTemplate(FontTemplateDescriptor, IpcSender<Reply>),
-    AddWebFont(Atom, Source, IpcSender<()>),
-    AddDownloadedWebFont(LowercaseString, Url, Vec<u8>, IpcSender<()>),
+    AddWebFont(FontFamily, Source, IpcSender<()>),
+    AddDownloadedWebFont(FontFamily, Url, Vec<u8>, IpcSender<()>),
     Exit(IpcSender<()>),
 }
 
@@ -97,21 +98,38 @@ pub enum Reply {
 struct FontCache {
     port: IpcReceiver<Command>,
     channel_to_self: IpcSender<Command>,
-    generic_fonts: HashMap<LowercaseString, LowercaseString>,
-    local_families: HashMap<LowercaseString, FontFamily>,
-    web_families: HashMap<LowercaseString, FontFamily>,
+    generic_fonts: HashMap<FontFamily, LowercaseString>,
+    local_families: HashMap<LowercaseString, FontTemplates>,
+    web_families: HashMap<LowercaseString, FontTemplates>,
     font_context: FontContextHandle,
     resource_task: ResourceTask,
 }
 
-fn add_generic_font(generic_fonts: &mut HashMap<LowercaseString, LowercaseString>,
-                    generic_name: &str, mapped_name: &str) {
-    let opt_system_default = system_default_family(generic_name);
-    let family_name = match opt_system_default {
-        Some(system_default) => LowercaseString::new(&system_default),
-        None => LowercaseString::new(mapped_name),
-    };
-    generic_fonts.insert(LowercaseString::new(generic_name), family_name);
+fn populate_generic_fonts() -> HashMap<FontFamily, LowercaseString> {
+    let mut generic_fonts = HashMap::with_capacity(5);
+
+    append_map(&mut generic_fonts, FontFamily::Serif, "Times New Roman");
+    append_map(&mut generic_fonts, FontFamily::SansSerif, "Arial");
+    append_map(&mut generic_fonts, FontFamily::Cursive, "Apple Chancery");
+    append_map(&mut generic_fonts, FontFamily::Fantasy, "Papyrus");
+    append_map(&mut generic_fonts, FontFamily::Monospace, "Menlo");
+
+    fn append_map(generic_fonts: &mut HashMap<FontFamily, LowercaseString>,
+                  font_family: FontFamily,
+                  mapped_name: &str) {
+        let family_name = {
+            let opt_system_default = system_default_family(font_family.name());
+            match opt_system_default {
+                Some(system_default) => LowercaseString::new(&system_default),
+                None => LowercaseString::new(mapped_name)
+            }
+        };
+
+        generic_fonts.insert(font_family, family_name);
+    }
+
+
+    generic_fonts
 }
 
 impl FontCache {
@@ -121,7 +139,6 @@ impl FontCache {
 
             match msg {
                 Command::GetFontTemplate(family, descriptor, result) => {
-                    let family = LowercaseString::new(&family);
                     let maybe_font_template = self.find_font_template(&family, &descriptor);
                     result.send(Reply::GetFontTemplateReply(maybe_font_template)).unwrap();
                 }
@@ -129,11 +146,11 @@ impl FontCache {
                     let font_template = self.last_resort_font_template(&descriptor);
                     result.send(Reply::GetFontTemplateReply(Some(font_template))).unwrap();
                 }
-                Command::AddWebFont(family_name, src, result) => {
-                    let family_name = LowercaseString::new(&family_name);
+                Command::AddWebFont(family, src, result) => {
+                    let family_name = LowercaseString::new(family.name());
                     if !self.web_families.contains_key(&family_name) {
-                        let family = FontFamily::new();
-                        self.web_families.insert(family_name.clone(), family);
+                        let templates = FontTemplates::new();
+                        self.web_families.insert(family_name, templates);
                     }
 
                     match src {
@@ -162,7 +179,7 @@ impl FontCache {
                                         let mut bytes = bytes.lock().unwrap();
                                         let bytes = mem::replace(&mut *bytes, Vec::new());
                                         let command =
-                                            Command::AddDownloadedWebFont(family_name.clone(),
+                                            Command::AddDownloadedWebFont(family.clone(),
                                                                           url.clone(),
                                                                           bytes,
                                                                           result.clone());
@@ -171,18 +188,21 @@ impl FontCache {
                                 }
                             });
                         }
-                        Source::Local(ref local_family_name) => {
-                            let family = &mut self.web_families.get_mut(&family_name).unwrap();
-                            for_each_variation(&local_family_name, |path| {
-                                family.add_template(Atom::from(&*path), None);
+                        Source::Local(ref family) => {
+                            let family_name = LowercaseString::new(family.name());
+                            let templates = &mut self.web_families.get_mut(&family_name).unwrap();
+                            for_each_variation(&family_name, |path| {
+                                templates.add_template(Atom::from(&*path), None);
                             });
                             result.send(()).unwrap();
                         }
                     }
                 }
-                Command::AddDownloadedWebFont(family_name, url, bytes, result) => {
-                    let family = &mut self.web_families.get_mut(&family_name).unwrap();
-                    family.add_template(Atom::from(&*url.to_string()), Some(bytes));
+                Command::AddDownloadedWebFont(family, url, bytes, result) => {
+                    let family_name = LowercaseString::new(family.name());
+
+                    let templates = &mut self.web_families.get_mut(&family_name).unwrap();
+                    templates.add_template(Atom::from(&*url.to_string()), Some(bytes));
                     drop(result.send(()));
                 }
                 Command::Exit(result) => {
@@ -198,15 +218,15 @@ impl FontCache {
         for_each_available_family(|family_name| {
             let family_name = LowercaseString::new(&family_name);
             if !self.local_families.contains_key(&family_name) {
-                let family = FontFamily::new();
-                self.local_families.insert(family_name, family);
+                let templates = FontTemplates::new();
+                self.local_families.insert(family_name, templates);
             }
         });
     }
 
-    fn transform_family(&self, family: &LowercaseString) -> LowercaseString {
+    fn transform_family(&self, family: &FontFamily) -> LowercaseString {
         match self.generic_fonts.get(family) {
-            None => family.clone(),
+            None => LowercaseString::new(family.name()),
             Some(mapped_family) => (*mapped_family).clone()
         }
     }
@@ -235,22 +255,26 @@ impl FontCache {
         }
     }
 
-    fn find_font_in_web_family<'a>(&'a mut self, family_name: &LowercaseString, desc: &FontTemplateDescriptor)
+    fn find_font_in_web_family<'a>(&'a mut self, family: &FontFamily, desc: &FontTemplateDescriptor)
                                 -> Option<Arc<FontTemplateData>> {
-        if self.web_families.contains_key(family_name) {
-            let family = self.web_families.get_mut(family_name).unwrap();
-            let maybe_font = family.find_font_for_style(desc, &self.font_context);
+        let family_name = LowercaseString::new(family.name());
+
+        if self.web_families.contains_key(&family_name) {
+            let templates = self.web_families.get_mut(&family_name).unwrap();
+            let maybe_font = templates.find_font_for_style(desc, &self.font_context);
             maybe_font
         } else {
             None
         }
     }
 
-    fn find_font_template(&mut self, family: &LowercaseString, desc: &FontTemplateDescriptor)
+    fn find_font_template(&mut self, family: &FontFamily, desc: &FontTemplateDescriptor)
                             -> Option<Arc<FontTemplateData>> {
-        let transformed_family_name = self.transform_family(family);
-        self.find_font_in_web_family(&transformed_family_name, desc)
-            .or_else(|| self.find_font_in_local_family(&transformed_family_name, desc))
+        self.find_font_in_web_family(family, desc)
+            .or_else(|| {
+                let transformed_family = self.transform_family(family);
+                self.find_font_in_local_family(&transformed_family, desc)
+            })
     }
 
     fn last_resort_font_template(&mut self, desc: &FontTemplateDescriptor)
@@ -283,12 +307,7 @@ impl FontCacheTask {
         let channel_to_self = chan.clone();
         spawn_named("FontCacheTask".to_owned(), move || {
             // TODO: Allow users to specify these.
-            let mut generic_fonts = HashMap::with_capacity(5);
-            add_generic_font(&mut generic_fonts, "serif", "Times New Roman");
-            add_generic_font(&mut generic_fonts, "sans-serif", "Arial");
-            add_generic_font(&mut generic_fonts, "cursive", "Apple Chancery");
-            add_generic_font(&mut generic_fonts, "fantasy", "Papyrus");
-            add_generic_font(&mut generic_fonts, "monospace", "Menlo");
+            let generic_fonts = populate_generic_fonts();
 
             let mut cache = FontCache {
                 port: port,
@@ -309,7 +328,7 @@ impl FontCacheTask {
         }
     }
 
-    pub fn find_font_template(&self, family: String, desc: FontTemplateDescriptor)
+    pub fn find_font_template(&self, family: FontFamily, desc: FontTemplateDescriptor)
                                                 -> Option<Arc<FontTemplateData>> {
 
         let (response_chan, response_port) = ipc::channel().unwrap();
@@ -339,7 +358,7 @@ impl FontCacheTask {
         }
     }
 
-    pub fn add_web_font(&self, family: Atom, src: Source, sender: IpcSender<()>) {
+    pub fn add_web_font(&self, family: FontFamily, src: Source, sender: IpcSender<()>) {
         self.chan.send(Command::AddWebFont(family, src, sender)).unwrap();
     }
 
@@ -349,4 +368,3 @@ impl FontCacheTask {
         response_port.recv().unwrap();
     }
 }
-

components/gfx/font_context.rs

@@ -208,8 +208,7 @@ impl FontContext {
             }
 
             if !cache_hit {
-                let font_template = self.font_cache_task.find_font_template(family.name()
-                                                                            .to_owned(),
+                let font_template = self.font_cache_task.find_font_template(family.clone(),
                                                                             desc.clone());
                 match font_template {
                     Some(font_template) => {
@@ -340,4 +339,3 @@ impl Hash for LayoutFontGroupCacheKey {
 pub fn invalidate_font_caches() {
     FONT_CACHE_EPOCH.fetch_add(1, Ordering::SeqCst);
 }
-

components/script/dom/element.rs

@@ -326,7 +326,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
                 PropertyDeclaration::FontFamily(
                     DeclaredValue::Value(
                         font_family::computed_value::T(vec![
-                            font_family::computed_value::FontFamily::FamilyName(
+                            font_family::computed_value::FontFamily::from_atom(
                                 font_family)])))));
         }
 

components/style/font_face.rs

@@ -7,14 +7,13 @@ use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
 use parser::{ParserContext, log_css_error};
 use properties::longhands::font_family::parse_one_family;
 use std::ascii::AsciiExt;
-use string_cache::Atom;
 use url::Url;
 use util::mem::HeapSizeOf;
 
 #[derive(Clone, Debug, HeapSizeOf, PartialEq, Eq, Deserialize, Serialize)]
 pub enum Source {
     Url(UrlSource),
-    Local(Atom),
+    Local(FontFamily),
 }
 
 #[derive(Clone, Debug, HeapSizeOf, PartialEq, Eq, Deserialize, Serialize)]
@@ -25,7 +24,7 @@ pub struct UrlSource {
 
 #[derive(Debug, HeapSizeOf, PartialEq, Eq)]
 pub struct FontFaceRule {
-    pub family: Atom,
+    pub family: FontFamily,
     pub sources: Vec<Source>,
 }
 
@@ -62,7 +61,7 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
 }
 
 enum FontFaceDescriptorDeclaration {
-    Family(Atom),
+    Family(FontFamily),
     Src(Vec<Source>),
 }
 
@@ -86,7 +85,7 @@ impl<'a, 'b> DeclarationParser for FontFaceRuleParser<'a, 'b> {
         match_ignore_ascii_case! { name,
             "font-family" => {
                 Ok(FontFaceDescriptorDeclaration::Family(try!(
-                            parse_one_non_generic_family_name(input))))
+                            parse_one_family(input))))
             },
             "src" => {
                 Ok(FontFaceDescriptorDeclaration::Src(try!(input.parse_comma_separated(|input| {
@@ -98,17 +97,9 @@ impl<'a, 'b> DeclarationParser for FontFaceRuleParser<'a, 'b> {
     }
 }
 
-fn parse_one_non_generic_family_name(input: &mut Parser) -> Result<Atom, ()> {
-    match parse_one_family(input) {
-        Ok(FontFamily::FamilyName(name)) => Ok(name.clone()),
-        _ => Err(())
-    }
-}
-
-
 fn parse_one_src(context: &ParserContext, input: &mut Parser) -> Result<Source, ()> {
     if input.try(|input| input.expect_function_matching("local")).is_ok() {
-        return Ok(Source::Local(try!(input.parse_nested_block(parse_one_non_generic_family_name))))
+        return Ok(Source::Local(try!(input.parse_nested_block(parse_one_family))))
     }
     let url = try!(input.expect_url());
     let url = context.base_url.join(&url).unwrap_or_else(

components/style/properties.mako.rs

@@ -1705,35 +1705,60 @@ pub mod longhands {
         use values::computed::ComputedValueAsSpecified;
         pub use self::computed_value::T as SpecifiedValue;
 
+        const SERIF: &'static str = "serif";
+        const SANS_SERIF: &'static str = "sans-serif";
+        const CURSIVE: &'static str = "cursive";
+        const FANTASY: &'static str = "fantasy";
+        const MONOSPACE: &'static str = "monospace";
+
         impl ComputedValueAsSpecified for SpecifiedValue {}
         pub mod computed_value {
             use cssparser::ToCss;
             use std::fmt;
             use string_cache::Atom;
 
-            #[derive(Debug, PartialEq, Eq, Clone, Hash, HeapSizeOf)]
+            #[derive(Debug, PartialEq, Eq, Clone, Hash, HeapSizeOf, Deserialize, Serialize)]
             pub enum FontFamily {
                 FamilyName(Atom),
-                // Generic
-//                Serif,
-//                SansSerif,
-//                Cursive,
-//                Fantasy,
-//                Monospace,
+                // Generic,
+                Serif,
+                SansSerif,
+                Cursive,
+                Fantasy,
+                Monospace,
             }
             impl FontFamily {
                 #[inline]
                 pub fn name(&self) -> &str {
                     match *self {
                         FontFamily::FamilyName(ref name) => &*name,
+                        FontFamily::Serif => super::SERIF,
+                        FontFamily::SansSerif => super::SANS_SERIF,
+                        FontFamily::Cursive => super::CURSIVE,
+                        FontFamily::Fantasy => super::FANTASY,
+                        FontFamily::Monospace => super::MONOSPACE
+                    }
+                }
+
+                pub fn from_atom(input: Atom) -> FontFamily {
+                    let option = match_ignore_ascii_case! { &input,
+                        super::SERIF => Some(FontFamily::Serif),
+                        super::SANS_SERIF => Some(FontFamily::SansSerif),
+                        super::CURSIVE => Some(FontFamily::Cursive),
+                        super::FANTASY => Some(FontFamily::Fantasy),
+                        super::MONOSPACE => Some(FontFamily::Monospace)
+                        _ => None
+                    };
+
+                    match option {
+                        Some(family) => family,
+                        None => FontFamily::FamilyName(input)
                     }
                 }
             }
             impl ToCss for FontFamily {
                 fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                    match *self {
-                        FontFamily::FamilyName(ref name) => dest.write_str(&*name),
-                    }
+                    dest.write_str(self.name())
                 }
             }
             impl ToCss for T {
@@ -1753,7 +1778,7 @@ pub mod longhands {
 
         #[inline]
         pub fn get_initial_value() -> computed_value::T {
-            computed_value::T(vec![FontFamily::FamilyName(atom!("serif"))])
+            computed_value::T(vec![FontFamily::Serif])
         }
         /// <family-name>#
         /// <family-name> = <string> | [ <ident>+ ]
@@ -1766,14 +1791,15 @@ pub mod longhands {
                 return Ok(FontFamily::FamilyName(Atom::from(&*value)))
             }
             let first_ident = try!(input.expect_ident());
-//            match_ignore_ascii_case! { first_ident,
-//                "serif" => return Ok(Serif),
-//                "sans-serif" => return Ok(SansSerif),
-//                "cursive" => return Ok(Cursive),
-//                "fantasy" => return Ok(Fantasy),
-//                "monospace" => return Ok(Monospace)
-//                _ => {}
-//            }
+
+            match_ignore_ascii_case! { first_ident,
+                SERIF => return Ok(FontFamily::Serif),
+                SANS_SERIF => return Ok(FontFamily::SansSerif),
+                CURSIVE => return Ok(FontFamily::Cursive),
+                FANTASY => return Ok(FontFamily::Fantasy),
+                MONOSPACE => return Ok(FontFamily::Monospace)
+                _ => {}
+            }
             let mut value = first_ident.into_owned();
             while let Ok(ident) = input.try(|input| input.expect_ident()) {
                 value.push_str(" ");