Auto merge of #20021 - jonleighton:lazy-font-group, r=mbrubeck,glennw

Lazy load fonts in a FontGroup

The first commit message explains this so I'll just copy it here:

---

This is a step towards fixing #17267. To fix that, we need to be able to
try various different fallback fonts in turn, which would become
unweildy with the prior eager-loading strategy.

Prior to this change, FontGroup loaded up all Font instances, including
the fallback font, before any of them were checked for the presence of
the glyphs we're trying to render.

So for the following CSS:

    font-family: Helvetica, Arial;

The FontGroup would contain a Font instance for Helvetica, and a Font
instance for Arial, and a Font instance for the fallback font.

It may be that Helvetica contains glyphs for every character in the
document, and therefore Arial and the fallback font are not needed at
all.

This change makes the strategy lazy, so that we'll only create a Font
for Arial if we cannot find a glyph within Helvetica. I've also
substantially refactored the existing code in the process and added
some documentation along the way.

---

I've added some tests in the second commit, but it required quite a bit of gymnastics to make it possible to write such a test. I'm not sure if the added complexity to the production code is worth it?

On the other hand, having this infrastructure in place may be useful for testing future changes in this area, and also possibly brings us a step closer to extracting a library as discussed in #4901. (What I mean by that is: it reduces coupling between `FontCacheThread` and `FontContext` -- the latter would have a place in such a library, the former wouldn't.)

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

components/gfx/font.rs

@@ -4,11 +4,13 @@
 
 use app_units::Au;
 use euclid::{Point2D, Rect, Size2D};
+use font_context::{FontContext, FontSource};
 use font_template::FontTemplateDescriptor;
 use ordered_float::NotNaN;
 use platform::font::{FontHandle, FontTable};
 use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
+use servo_atoms::Atom;
 use smallvec::SmallVec;
 use std::borrow::ToOwned;
 use std::cell::RefCell;
@@ -18,6 +20,8 @@ use std::str;
 use std::sync::Arc;
 use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering};
 use style::computed_values::{font_stretch, font_variant_caps, font_weight};
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::SingleFontFamily;
 use text::Shaper;
 use text::glyph::{ByteIndex, GlyphData, GlyphId, GlyphStore};
 use text::shaping::ShaperMethods;
@@ -59,6 +63,9 @@ pub trait FontHandleMethods: Sized {
     fn can_do_fast_shaping(&self) -> bool;
     fn metrics(&self) -> FontMetrics;
     fn table_for_tag(&self, FontTableTag) -> Option<FontTable>;
+
+    /// A unique identifier for the font, allowing comparison.
+    fn identifier(&self) -> Atom;
 }
 
 // Used to abstract over the shaper's choice of fixed int representation.
@@ -100,13 +107,32 @@ pub struct FontMetrics {
     pub line_gap:         Au,
 }
 
+/// `FontDescriptor` describes the parameters of a `Font`. It represents rendering a given font
+/// template at a particular size, with a particular font-variant-caps applied, etc. This contrasts
+/// with `FontTemplateDescriptor` in that the latter represents only the parameters inherent in the
+/// font data (weight, stretch, etc.).
+#[derive(Clone, Debug, PartialEq)]
+pub struct FontDescriptor {
+    pub template_descriptor: FontTemplateDescriptor,
+    pub variant: font_variant_caps::T,
+    pub pt_size: Au,
+}
+
+impl<'a> From<&'a FontStyleStruct> for FontDescriptor {
+    fn from(style: &'a FontStyleStruct) -> Self {
+        FontDescriptor {
+            template_descriptor: FontTemplateDescriptor::from(style),
+            variant: style.font_variant_caps,
+            pt_size: style.font_size.size(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct Font {
     pub handle: FontHandle,
     pub metrics: FontMetrics,
-    pub variant: font_variant_caps::T,
-    pub descriptor: FontTemplateDescriptor,
-    pub requested_pt_size: Au,
+    pub descriptor: FontDescriptor,
     pub actual_pt_size: Au,
     shaper: Option<Shaper>,
     shape_cache: RefCell<HashMap<ShapeCacheEntry, Arc<GlyphStore>>>,
@@ -116,25 +142,27 @@ pub struct Font {
 
 impl Font {
     pub fn new(handle: FontHandle,
-               variant: font_variant_caps::T,
-               descriptor: FontTemplateDescriptor,
-               requested_pt_size: Au,
+               descriptor: FontDescriptor,
                actual_pt_size: Au,
                font_key: webrender_api::FontInstanceKey) -> Font {
         let metrics = handle.metrics();
+
         Font {
             handle: handle,
             shaper: None,
-            variant: variant,
-            descriptor: descriptor,
-            requested_pt_size: requested_pt_size,
-            actual_pt_size: actual_pt_size,
-            metrics: metrics,
+            descriptor,
+            actual_pt_size,
+            metrics,
             shape_cache: RefCell::new(HashMap::new()),
             glyph_advance_cache: RefCell::new(HashMap::new()),
-            font_key: font_key,
+            font_key,
         }
     }
+
+    /// A unique identifier for the font, allowing comparison.
+    pub fn identifier(&self) -> Atom {
+        self.handle.identifier()
+    }
 }
 
 bitflags! {
@@ -260,13 +288,17 @@ impl Font {
 
     #[inline]
     pub fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {
-        let codepoint = match self.variant {
+        let codepoint = match self.descriptor.variant {
             font_variant_caps::T::SmallCaps => codepoint.to_uppercase().next().unwrap(), //FIXME: #5938
             font_variant_caps::T::Normal => codepoint,
         };
         self.handle.glyph_index(codepoint)
     }
 
+    pub fn has_glyph_for(&self, codepoint: char) -> bool {
+        self.glyph_index(codepoint).is_some()
+    }
+
     pub fn glyph_h_kerning(&self, first_glyph: GlyphId, second_glyph: GlyphId)
                            -> FractionalPixel {
         self.handle.glyph_h_kerning(first_glyph, second_glyph)
@@ -282,17 +314,102 @@ impl Font {
     }
 }
 
+pub type FontRef = Rc<RefCell<Font>>;
+
+/// A `FontGroup` is a prioritised list of fonts for a given set of font styles. It is used by
+/// `TextRun` to decide which font to render a character with. If none of the fonts listed in the
+/// styles are suitable, a fallback font may be used.
 #[derive(Debug)]
 pub struct FontGroup {
-    pub fonts: SmallVec<[Rc<RefCell<Font>>; 8]>,
+    descriptor: FontDescriptor,
+    families: SmallVec<[FontGroupFamily; 8]>,
 }
 
 impl FontGroup {
-    pub fn new(fonts: SmallVec<[Rc<RefCell<Font>>; 8]>) -> FontGroup {
-        FontGroup {
-            fonts: fonts,
+    pub fn new(style: &FontStyleStruct) -> FontGroup {
+        let descriptor = FontDescriptor::from(style);
+
+        let families =
+            style.font_family.0.iter()
+                .map(|family| FontGroupFamily::new(descriptor.clone(), family.clone()))
+                .collect();
+
+        FontGroup { descriptor, families }
+    }
+
+    /// Finds the first font, or else the first fallback font, which contains a glyph for
+    /// `codepoint`. If no such font is found, returns the first available font or fallback font
+    /// (which will cause a "glyph not found" character to be rendered). If no font at all can be
+    /// found, returns None.
+    pub fn find_by_codepoint<S: FontSource>(
+        &mut self,
+        mut font_context: &mut FontContext<S>,
+        codepoint: char
+    ) -> Option<FontRef> {
+        self.find(&mut font_context, |font| font.borrow().has_glyph_for(codepoint))
+            .or_else(|| self.first(&mut font_context))
+    }
+
+    pub fn first<S: FontSource>(
+        &mut self,
+        mut font_context: &mut FontContext<S>
+    ) -> Option<FontRef> {
+        self.find(&mut font_context, |_| true)
+    }
+
+    /// Find a font which returns true for `predicate`. This method mutates because we may need to
+    /// load new font data in the process of finding a suitable font.
+    fn find<S, P>(
+        &mut self,
+        mut font_context: &mut FontContext<S>,
+        mut predicate: P
+    ) -> Option<FontRef>
+    where
+        S: FontSource,
+        P: FnMut(&FontRef) -> bool
+    {
+        self.families.iter_mut()
+            .filter_map(|family| family.font(&mut font_context))
+            .find(|f| predicate(f))
+            .or_else(|| {
+                font_context.fallback_font(&self.descriptor)
+                    .into_iter().find(predicate)
+            })
+    }
+}
+
+/// A `FontGroupFamily` is a single font family in a `FontGroup`. It corresponds to one of the
+/// families listed in the `font-family` CSS property. The corresponding font data is lazy-loaded,
+/// only if actually needed.
+#[derive(Debug)]
+struct FontGroupFamily {
+    descriptor: FontDescriptor,
+    family: SingleFontFamily,
+    loaded: bool,
+    font: Option<FontRef>,
+}
+
+impl FontGroupFamily {
+    fn new(descriptor: FontDescriptor, family: SingleFontFamily) -> FontGroupFamily {
+        FontGroupFamily {
+            descriptor,
+            family,
+            loaded: false,
+            font: None,
         }
     }
+
+    /// Returns the font within this family which matches the style. We'll fetch the data from the
+    /// `FontContext` the first time this method is called, and return a cached reference on
+    /// subsequent calls.
+    fn font<S: FontSource>(&mut self, font_context: &mut FontContext<S>) -> Option<FontRef> {
+        if !self.loaded {
+            self.font = font_context.font(&self.descriptor, &self.family);
+            self.loaded = true;
+        }
+
+        self.font.clone()
+    }
 }
 
 pub struct RunMetrics {

components/gfx/font_cache_thread.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use app_units::Au;
+use font_context::FontSource;
 use font_template::{FontTemplate, FontTemplateDescriptor};
 use fontsan;
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
@@ -30,7 +31,7 @@ use style::values::computed::font::{SingleFontFamily, FamilyName};
 use webrender_api;
 
 /// A list of font templates that make up a given font family.
-struct FontTemplates {
+pub struct FontTemplates {
     templates: Vec<FontTemplate>,
 }
 
@@ -41,14 +42,14 @@ pub struct FontTemplateInfo {
 }
 
 impl FontTemplates {
-    fn new() -> FontTemplates {
+    pub fn new() -> FontTemplates {
         FontTemplates {
             templates: vec!(),
         }
     }
 
     /// Find a font in this family that matches a given descriptor.
-    fn find_font_for_style(&mut self, desc: &FontTemplateDescriptor, fctx: &FontContextHandle)
+    pub fn find_font_for_style(&mut self, desc: &FontTemplateDescriptor, fctx: &FontContextHandle)
                                -> Option<Arc<FontTemplateData>> {
         // TODO(Issue #189): optimize lookup for
         // regular/bold/italic/bolditalic with fixed offsets and a
@@ -89,7 +90,7 @@ impl FontTemplates {
         None
     }
 
-    fn add_template(&mut self, identifier: Atom, maybe_data: Option<Vec<u8>>) {
+    pub fn add_template(&mut self, identifier: Atom, maybe_data: Option<Vec<u8>>) {
         for template in &self.templates {
             if *template.identifier() == identifier {
                 return;
@@ -414,8 +415,8 @@ impl FontCache {
     }
 }
 
-/// The public interface to the font cache thread, used exclusively by
-/// the per-thread/thread FontContext structures.
+/// The public interface to the font cache thread, used by per-thread `FontContext` instances (via
+/// the `FontSource` trait), and also by layout.
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct FontCacheThread {
     chan: IpcSender<Command>,
@@ -453,7 +454,31 @@ impl FontCacheThread {
         }
     }
 
-    pub fn find_font_template(&self, family: SingleFontFamily, desc: FontTemplateDescriptor)
+    pub fn add_web_font(&self, family: FamilyName, sources: EffectiveSources, sender: IpcSender<()>) {
+        self.chan.send(Command::AddWebFont(LowercaseString::new(&family.name), sources, sender)).unwrap();
+    }
+
+    pub fn exit(&self) {
+        let (response_chan, response_port) = ipc::channel().unwrap();
+        self.chan.send(Command::Exit(response_chan)).expect("Couldn't send FontCacheThread exit message");
+        response_port.recv().expect("Couldn't receive FontCacheThread reply");
+    }
+}
+
+impl FontSource for FontCacheThread {
+    fn get_font_instance(&mut self, key: webrender_api::FontKey, size: Au) -> webrender_api::FontInstanceKey {
+        let (response_chan, response_port) =
+            ipc::channel().expect("failed to create IPC channel");
+        self.chan.send(Command::GetFontInstance(key, size, response_chan))
+            .expect("failed to send message to font cache thread");
+
+        let instance_key = response_port.recv()
+            .expect("failed to receive response to font request");
+
+        instance_key
+    }
+
+    fn find_font_template(&mut self, family: SingleFontFamily, desc: FontTemplateDescriptor)
                                                 -> Option<FontTemplateInfo> {
         let (response_chan, response_port) =
             ipc::channel().expect("failed to create IPC channel");
@@ -470,7 +495,7 @@ impl FontCacheThread {
         }
     }
 
-    pub fn last_resort_font_template(&self, desc: FontTemplateDescriptor)
+    fn last_resort_font_template(&mut self, desc: FontTemplateDescriptor)
                                                 -> FontTemplateInfo {
         let (response_chan, response_port) =
             ipc::channel().expect("failed to create IPC channel");
@@ -486,31 +511,8 @@ impl FontCacheThread {
             }
         }
     }
-
-    pub fn add_web_font(&self, family: FamilyName, sources: EffectiveSources, sender: IpcSender<()>) {
-        self.chan.send(Command::AddWebFont(LowercaseString::new(&family.name), sources, sender)).unwrap();
-    }
-
-    pub fn get_font_instance(&self, key: webrender_api::FontKey, size: Au) -> webrender_api::FontInstanceKey {
-        let (response_chan, response_port) =
-            ipc::channel().expect("failed to create IPC channel");
-        self.chan.send(Command::GetFontInstance(key, size, response_chan))
-            .expect("failed to send message to font cache thread");
-
-        let instance_key = response_port.recv()
-            .expect("failed to receive response to font request");
-
-        instance_key
-    }
-
-    pub fn exit(&self) {
-        let (response_chan, response_port) = ipc::channel().unwrap();
-        self.chan.send(Command::Exit(response_chan)).expect("Couldn't send FontCacheThread exit message");
-        response_port.recv().expect("Couldn't receive FontCacheThread reply");
-    }
 }
 
-
 #[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
 pub struct LowercaseString {
     inner: String,

components/gfx/font_context.rs

@@ -4,98 +4,128 @@
 
 use app_units::Au;
 use fnv::FnvHasher;
-use font::{Font, FontGroup, FontHandleMethods};
-use font_cache_thread::FontCacheThread;
+use font::{Font, FontDescriptor, FontGroup, FontHandleMethods, FontRef};
+use font_cache_thread::FontTemplateInfo;
 use font_template::FontTemplateDescriptor;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use platform::font::FontHandle;
 pub use platform::font_context::FontContextHandle;
-use platform::font_template::FontTemplateData;
-use servo_arc::Arc as ServoArc;
-use smallvec::SmallVec;
+use servo_arc::Arc;
+use servo_atoms::Atom;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::default::Default;
 use std::hash::{BuildHasherDefault, Hash, Hasher};
 use std::rc::Rc;
-use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
-use style::computed_values::font_style::T as FontStyle;
 use style::computed_values::font_variant_caps::T as FontVariantCaps;
-use style::properties::style_structs;
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::SingleFontFamily;
 use webrender_api;
 
 static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8;      // Matches FireFox (see gfxFont.h)
 
 #[derive(Debug)]
-struct LayoutFontCacheEntry {
-    family: String,
-    font: Option<Rc<RefCell<Font>>>,
+struct FontCacheEntry {
+    family: Atom,
+    font: Option<FontRef>,
+}
+
+impl FontCacheEntry {
+    fn matches(&self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> bool {
+        if self.family != *family.atom() {
+            return false
+        }
+
+        if let Some(ref font) = self.font {
+            (*font).borrow().descriptor == *descriptor
+        } else {
+            true
+        }
+    }
 }
 
 #[derive(Debug)]
 struct FallbackFontCacheEntry {
-    font: Rc<RefCell<Font>>,
+    font: FontRef,
+}
+
+impl FallbackFontCacheEntry {
+    fn matches(&self, descriptor: &FontDescriptor) -> bool {
+        self.font.borrow().descriptor == *descriptor
+    }
 }
 
 /// An epoch for the font context cache. The cache is flushed if the current epoch does not match
 /// this one.
 static FONT_CACHE_EPOCH: AtomicUsize = ATOMIC_USIZE_INIT;
 
+pub trait FontSource {
+    fn get_font_instance(&mut self, key: webrender_api::FontKey, size: Au) -> webrender_api::FontInstanceKey;
+
+    fn find_font_template(
+        &mut self,
+        family: SingleFontFamily,
+        desc: FontTemplateDescriptor
+    ) -> Option<FontTemplateInfo>;
+
+    fn last_resort_font_template(&mut self, desc: FontTemplateDescriptor) -> FontTemplateInfo;
+}
+
 /// The FontContext represents the per-thread/thread state necessary for
 /// working with fonts. It is the public API used by the layout and
 /// paint code. It talks directly to the font cache thread where
 /// required.
 #[derive(Debug)]
-pub struct FontContext {
+pub struct FontContext<S: FontSource> {
     platform_handle: FontContextHandle,
-    font_cache_thread: FontCacheThread,
+    font_source: S,
 
-    /// TODO: See bug https://github.com/servo/servo/issues/3300.
-    layout_font_cache: Vec<LayoutFontCacheEntry>,
+    // TODO: The font context holds a strong ref to the cached fonts
+    // so they will never be released. Find out a good time to drop them.
+    // See bug https://github.com/servo/servo/issues/3300
+    //
+    // GWTODO: Check on real pages if this is faster as Vec() or HashMap().
+    font_cache: Vec<FontCacheEntry>,
     fallback_font_cache: Vec<FallbackFontCacheEntry>,
 
-    layout_font_group_cache:
-        HashMap<LayoutFontGroupCacheKey, Rc<FontGroup>, BuildHasherDefault<FnvHasher>>,
+    font_group_cache:
+        HashMap<FontGroupCacheKey, Rc<RefCell<FontGroup>>, BuildHasherDefault<FnvHasher>>,
 
     epoch: usize,
 }
 
-impl FontContext {
-    pub fn new(font_cache_thread: FontCacheThread) -> FontContext {
+impl<S: FontSource> FontContext<S> {
+    pub fn new(font_source: S) -> FontContext<S> {
         let handle = FontContextHandle::new();
         FontContext {
             platform_handle: handle,
-            font_cache_thread: font_cache_thread,
-            layout_font_cache: vec!(),
+            font_source,
+            font_cache: vec!(),
             fallback_font_cache: vec!(),
-            layout_font_group_cache: HashMap::with_hasher(Default::default()),
+            font_group_cache: HashMap::with_hasher(Default::default()),
             epoch: 0,
         }
     }
 
-    /// Create a font for use in layout calculations.
-    fn create_layout_font(&self,
-                          template: Arc<FontTemplateData>,
-                          descriptor: FontTemplateDescriptor,
-                          pt_size: Au,
-                          variant: FontVariantCaps,
-                          font_key: webrender_api::FontKey) -> Result<Font, ()> {
+    /// Create a `Font` for use in layout calculations, from a `FontTemplateInfo` returned by the
+    /// cache thread (which contains the underlying font data) and a `FontDescriptor` which
+    /// contains the styling parameters.
+    fn create_font(&mut self, info: FontTemplateInfo, descriptor: FontDescriptor) -> Result<Font, ()> {
         // TODO: (Bug #3463): Currently we only support fake small-caps
         // painting. We should also support true small-caps (where the
         // font supports it) in the future.
-        let actual_pt_size = match variant {
-            FontVariantCaps::SmallCaps => pt_size.scale_by(SMALL_CAPS_SCALE_FACTOR),
-            FontVariantCaps::Normal => pt_size,
+        let actual_pt_size = match descriptor.variant {
+            FontVariantCaps::SmallCaps => descriptor.pt_size.scale_by(SMALL_CAPS_SCALE_FACTOR),
+            FontVariantCaps::Normal => descriptor.pt_size,
         };
 
         let handle = FontHandle::new_from_template(&self.platform_handle,
-                                                        template,
+                                                        info.font_template,
                                                         Some(actual_pt_size))?;
 
-        let font_instance_key = self.font_cache_thread
-                                    .get_font_instance(font_key, actual_pt_size);
-        Ok(Font::new(handle, variant, descriptor, pt_size, actual_pt_size, font_instance_key))
+        let font_instance_key = self.font_source.get_font_instance(info.font_key, actual_pt_size);
+        Ok(Font::new(handle, descriptor.to_owned(), actual_pt_size, font_instance_key))
     }
 
     fn expire_font_caches_if_necessary(&mut self) {
@@ -104,136 +134,106 @@ impl FontContext {
             return
         }
 
-        self.layout_font_cache.clear();
+        self.font_cache.clear();
         self.fallback_font_cache.clear();
-        self.layout_font_group_cache.clear();
+        self.font_group_cache.clear();
         self.epoch = current_epoch
     }
 
-    /// Create a group of fonts for use in layout calculations. May return
-    /// a cached font if this font instance has already been used by
-    /// this context.
-    pub fn layout_font_group_for_style(&mut self, style: ServoArc<style_structs::Font>)
-                                       -> Rc<FontGroup> {
+    /// Returns a `FontGroup` representing fonts which can be used for layout, given the `style`.
+    /// Font groups are cached, so subsequent calls with the same `style` will return a reference
+    /// to an existing `FontGroup`.
+    pub fn font_group(&mut self, style: Arc<FontStyleStruct>) -> Rc<RefCell<FontGroup>> {
         self.expire_font_caches_if_necessary();
 
-        let layout_font_group_cache_key = LayoutFontGroupCacheKey {
-            pointer: style.clone(),
+        let cache_key = FontGroupCacheKey {
             size: style.font_size.size(),
+            style,
         };
-        if let Some(ref cached_font_group) = self.layout_font_group_cache.get(
-                &layout_font_group_cache_key) {
-            return (*cached_font_group).clone()
+
+        if let Some(ref font_group) = self.font_group_cache.get(&cache_key) {
+            return (*font_group).clone()
         }
 
-        // TODO: The font context holds a strong ref to the cached fonts
-        // so they will never be released. Find out a good time to drop them.
-
-        let desc = FontTemplateDescriptor::new(style.font_weight,
-                                               style.font_stretch,
-                                               style.font_style == FontStyle::Italic ||
-                                                style.font_style == FontStyle::Oblique);
-
-        let mut fonts: SmallVec<[Rc<RefCell<Font>>; 8]> = SmallVec::new();
-
-        for family in style.font_family.0.iter() {
-            // GWTODO: Check on real pages if this is faster as Vec() or HashMap().
-            let mut cache_hit = false;
-            for cached_font_entry in &self.layout_font_cache {
-                if cached_font_entry.family == family.name() {
-                    match cached_font_entry.font {
-                        None => {
-                            cache_hit = true;
-                            break;
-                        }
-                        Some(ref cached_font_ref) => {
-                            let cached_font = (*cached_font_ref).borrow();
-                            if cached_font.descriptor == desc &&
-                               cached_font.requested_pt_size == style.font_size.size() &&
-                               cached_font.variant == style.font_variant_caps {
-                                fonts.push((*cached_font_ref).clone());
-                                cache_hit = true;
-                                break;
-                            }
-                        }
-                    }
-                }
-            }
-
-            if !cache_hit {
-                let template_info = self.font_cache_thread.find_font_template(family.clone(),
-                                                                             desc.clone());
-                match template_info {
-                    Some(template_info) => {
-                        let layout_font = self.create_layout_font(template_info.font_template,
-                                                                  desc.clone(),
-                                                                  style.font_size.size(),
-                                                                  style.font_variant_caps,
-                                                                  template_info.font_key);
-                        let font = match layout_font {
-                            Ok(layout_font) => {
-                                let layout_font = Rc::new(RefCell::new(layout_font));
-                                fonts.push(layout_font.clone());
-
-                                Some(layout_font)
-                            }
-                            Err(_) => None
-                        };
-
-                        self.layout_font_cache.push(LayoutFontCacheEntry {
-                            family: family.name().to_owned(),
-                            font: font
-                        });
-                    }
-                    None => {
-                        self.layout_font_cache.push(LayoutFontCacheEntry {
-                            family: family.name().to_owned(),
-                            font: None,
-                        });
-                    }
-                }
-            }
-        }
-
-        // Add a last resort font as a fallback option.
-        let mut cache_hit = false;
-        for cached_font_entry in &self.fallback_font_cache {
-            let cached_font = cached_font_entry.font.borrow();
-            if cached_font.descriptor == desc &&
-                        cached_font.requested_pt_size == style.font_size.size() &&
-                        cached_font.variant == style.font_variant_caps {
-                fonts.push(cached_font_entry.font.clone());
-                cache_hit = true;
-                break;
-            }
-        }
-
-        if !cache_hit {
-            let template_info = self.font_cache_thread.last_resort_font_template(desc.clone());
-            let layout_font = self.create_layout_font(template_info.font_template,
-                                                      desc.clone(),
-                                                      style.font_size.size(),
-                                                      style.font_variant_caps,
-                                                      template_info.font_key);
-            match layout_font {
-                Ok(layout_font) => {
-                    let layout_font = Rc::new(RefCell::new(layout_font));
-                    self.fallback_font_cache.push(FallbackFontCacheEntry {
-                        font: layout_font.clone(),
-                    });
-                    fonts.push(layout_font);
-                }
-                Err(_) => debug!("Failed to create fallback layout font!")
-            }
-        }
-
-        let font_group = Rc::new(FontGroup::new(fonts));
-        self.layout_font_group_cache.insert(layout_font_group_cache_key, font_group.clone());
+        let font_group = Rc::new(RefCell::new(FontGroup::new(&cache_key.style)));
+        self.font_group_cache.insert(cache_key, font_group.clone());
         font_group
     }
+
+    /// Returns a reference to an existing font cache entry matching `descriptor` and `family`, if
+    /// there is one.
+    fn font_cache_entry(&self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> Option<&FontCacheEntry> {
+        self.font_cache.iter()
+            .find(|cache_entry| cache_entry.matches(&descriptor, &family))
+    }
+
+    /// Creates a new font cache entry matching `descriptor` and `family`.
+    fn create_font_cache_entry(&mut self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> FontCacheEntry {
+        let font =
+            self.font_source.find_font_template(family.clone(), descriptor.template_descriptor.clone())
+                .and_then(|template_info|
+                    self.create_font(template_info, descriptor.to_owned()).ok()
+                )
+                .map(|font| Rc::new(RefCell::new(font)));
+
+        FontCacheEntry { family: family.atom().to_owned(), font }
+    }
+
+    /// Returns a font from `family` matching the `descriptor`. Fonts are cached, so repeated calls
+    /// will return a reference to the same underlying `Font`.
+    pub fn font(&mut self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> Option<FontRef> {
+        if let Some(entry) = self.font_cache_entry(descriptor, family) {
+            return entry.font.clone()
+        }
+
+        let entry = self.create_font_cache_entry(descriptor, family);
+        let font = entry.font.clone();
+        self.font_cache.push(entry);
+        font
+    }
+
+    /// Returns a reference to an existing fallback font cache entry matching `descriptor`, if
+    /// there is one.
+    fn fallback_font_cache_entry(&self, descriptor: &FontDescriptor) -> Option<&FallbackFontCacheEntry> {
+        self.fallback_font_cache.iter()
+            .find(|cache_entry| cache_entry.matches(descriptor))
+    }
+
+    /// Creates a new fallback font cache entry matching `descriptor`.
+    fn create_fallback_font_cache_entry(&mut self, descriptor: &FontDescriptor) -> Option<FallbackFontCacheEntry> {
+        let template_info = self.font_source.last_resort_font_template(descriptor.template_descriptor.clone());
+
+        match self.create_font(template_info, descriptor.to_owned()) {
+            Ok(font) =>
+                Some(FallbackFontCacheEntry {
+                    font: Rc::new(RefCell::new(font))
+                }),
+
+            Err(_) => {
+                debug!("Failed to create fallback font!");
+                None
+            }
+        }
+    }
+
+    /// Returns a fallback font matching the `descriptor`. Fonts are cached, so repeated calls will
+    /// return a reference to the same underlying `Font`.
+    pub fn fallback_font(&mut self, descriptor: &FontDescriptor) -> Option<FontRef> {
+        if let Some(cached_entry) = self.fallback_font_cache_entry(descriptor) {
+            return Some(cached_entry.font.clone())
+        };
+
+        if let Some(entry) = self.create_fallback_font_cache_entry(descriptor) {
+            let font = entry.font.clone();
+            self.fallback_font_cache.push(entry);
+            Some(font)
+        } else {
+            None
+        }
+    }
 }
 
-impl MallocSizeOf for FontContext {
+impl<S: FontSource> MallocSizeOf for FontContext<S> {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         // FIXME(njn): Measure other fields eventually.
         self.platform_handle.size_of(ops)
@@ -241,22 +241,22 @@ impl MallocSizeOf for FontContext {
 }
 
 #[derive(Debug)]
-struct LayoutFontGroupCacheKey {
-    pointer: ServoArc<style_structs::Font>,
+struct FontGroupCacheKey {
+    style: Arc<FontStyleStruct>,
     size: Au,
 }
 
-impl PartialEq for LayoutFontGroupCacheKey {
-    fn eq(&self, other: &LayoutFontGroupCacheKey) -> bool {
-        self.pointer == other.pointer && self.size == other.size
+impl PartialEq for FontGroupCacheKey {
+    fn eq(&self, other: &FontGroupCacheKey) -> bool {
+        self.style == other.style && self.size == other.size
     }
 }
 
-impl Eq for LayoutFontGroupCacheKey {}
+impl Eq for FontGroupCacheKey {}
 
-impl Hash for LayoutFontGroupCacheKey {
+impl Hash for FontGroupCacheKey {
     fn hash<H>(&self, hasher: &mut H) where H: Hasher {
-        self.pointer.hash.hash(hasher)
+        self.style.hash.hash(hasher)
     }
 }
 

components/gfx/font_template.rs

@@ -11,7 +11,10 @@ use std::fmt::{Debug, Error, Formatter};
 use std::io::Error as IoError;
 use std::sync::{Arc, Weak};
 use std::u32;
-use style::computed_values::{font_stretch, font_weight};
+use style::computed_values::font_stretch::T as FontStretch;
+use style::computed_values::font_style::T as FontStyle;
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::FontWeight;
 
 /// Describes how to select a font from a given family. This is very basic at the moment and needs
 /// to be expanded or refactored when we support more of the font styling parameters.
@@ -19,14 +22,14 @@ use style::computed_values::{font_stretch, font_weight};
 /// NB: If you change this, you will need to update `style::properties::compute_font_hash()`.
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Serialize)]
 pub struct FontTemplateDescriptor {
-    pub weight: font_weight::T,
-    pub stretch: font_stretch::T,
+    pub weight: FontWeight,
+    pub stretch: FontStretch,
     pub italic: bool,
 }
 
 impl FontTemplateDescriptor {
     #[inline]
-    pub fn new(weight: font_weight::T, stretch: font_stretch::T, italic: bool)
+    pub fn new(weight: FontWeight, stretch: FontStretch, italic: bool)
                -> FontTemplateDescriptor {
         FontTemplateDescriptor {
             weight: weight,
@@ -57,15 +60,25 @@ impl FontTemplateDescriptor {
     #[inline]
     fn stretch_number(&self) -> i32 {
         match self.stretch {
-            font_stretch::T::UltraCondensed => 1,
-            font_stretch::T::ExtraCondensed => 2,
-            font_stretch::T::Condensed      => 3,
-            font_stretch::T::SemiCondensed  => 4,
-            font_stretch::T::Normal         => 5,
-            font_stretch::T::SemiExpanded   => 6,
-            font_stretch::T::Expanded       => 7,
-            font_stretch::T::ExtraExpanded  => 8,
-            font_stretch::T::UltraExpanded  => 9,
+            FontStretch::UltraCondensed => 1,
+            FontStretch::ExtraCondensed => 2,
+            FontStretch::Condensed      => 3,
+            FontStretch::SemiCondensed  => 4,
+            FontStretch::Normal         => 5,
+            FontStretch::SemiExpanded   => 6,
+            FontStretch::Expanded       => 7,
+            FontStretch::ExtraExpanded  => 8,
+            FontStretch::UltraExpanded  => 9,
+        }
+    }
+}
+
+impl<'a> From<&'a FontStyleStruct> for FontTemplateDescriptor {
+    fn from(style: &'a FontStyleStruct) -> Self {
+        FontTemplateDescriptor {
+            weight: style.font_weight,
+            stretch: style.font_stretch,
+            italic: style.font_style == FontStyle::Italic || style.font_style == FontStyle::Oblique,
         }
     }
 }

components/gfx/platform/freetype/font.rs

@@ -17,6 +17,7 @@ use freetype::freetype::FT_Sfnt_Tag;
 use freetype::tt_os2::TT_OS2;
 use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
+use servo_atoms::Atom;
 use std::{mem, ptr};
 use std::os::raw::{c_char, c_long};
 use std::sync::Arc;
@@ -306,6 +307,10 @@ impl FontHandleMethods for FontHandle {
             Some(FontTable { buffer: buf })
         }
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }
 
 impl<'a> FontHandle {

components/gfx/platform/macos/font.rs

@@ -18,6 +18,7 @@ use font::{FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, Fract
 use font::{GPOS, GSUB, KERN};
 use platform::font_template::FontTemplateData;
 use platform::macos::font_context::FontContextHandle;
+use servo_atoms::Atom;
 use std::{fmt, ptr};
 use std::ops::Range;
 use std::sync::Arc;
@@ -318,4 +319,8 @@ impl FontHandleMethods for FontHandle {
             Some(FontTable::wrap(data))
         })
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }

components/gfx/platform/windows/font.rs

@@ -15,6 +15,7 @@ use font::{FontTableTag, FractionalPixel};
 use platform::font_template::FontTemplateData;
 use platform::windows::font_context::FontContextHandle;
 use platform::windows::font_list::font_from_atom;
+use servo_atoms::Atom;
 use std::sync::Arc;
 use style::computed_values::font_stretch::T as StyleFontStretch;
 use style::computed_values::font_weight::T as StyleFontWeight;
@@ -374,4 +375,8 @@ impl FontHandleMethods for FontHandle {
     fn table_for_tag(&self, tag: FontTableTag) -> Option<FontTable> {
         self.face.get_font_table(tag).map(|bytes| FontTable { data: bytes })
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }

components/gfx/tests/font_context.rs

@@ -0,0 +1,174 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+extern crate app_units;
+extern crate gfx;
+extern crate servo_arc;
+extern crate servo_atoms;
+extern crate style;
+extern crate webrender_api;
+
+use app_units::Au;
+use gfx::font::FontHandleMethods;
+use gfx::font_cache_thread::{FontTemplates, FontTemplateInfo};
+use gfx::font_context::{FontContext, FontContextHandle, FontSource};
+use gfx::font_template::FontTemplateDescriptor;
+use servo_arc::Arc;
+use servo_atoms::Atom;
+use std::cell::Cell;
+use std::collections::HashMap;
+use std::fs::File;
+use std::io::prelude::*;
+use std::path::PathBuf;
+use std::rc::Rc;
+use style::properties::longhands::font_stretch::computed_value::T as FontStretch;
+use style::properties::longhands::font_style::computed_value::T as FontStyle;
+use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps;
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::{FamilyName, FamilyNameSyntax, FontFamily, FontFamilyList, FontSize};
+use style::values::computed::font::{FontWeight, SingleFontFamily};
+
+struct TestFontSource {
+    handle: FontContextHandle,
+    families: HashMap<Atom, FontTemplates>,
+    find_font_count: Rc<Cell<isize>>,
+}
+
+impl TestFontSource {
+    fn new() -> TestFontSource {
+        let mut csstest_ascii = FontTemplates::new();
+        Self::add_face(&mut csstest_ascii, "csstest-ascii");
+
+        let mut csstest_basic = FontTemplates::new();
+        Self::add_face(&mut csstest_basic, "csstest-basic-regular");
+
+        let mut families = HashMap::new();
+        families.insert(Atom::from("CSSTest ASCII"), csstest_ascii);
+        families.insert(Atom::from("CSSTest Basic"), csstest_basic);
+
+        TestFontSource {
+            handle: FontContextHandle::new(),
+            families,
+            find_font_count: Rc::new(Cell::new(0)),
+        }
+    }
+
+    fn add_face(family: &mut FontTemplates, name: &str) {
+        let mut path: PathBuf = [
+            env!("CARGO_MANIFEST_DIR"),
+            "tests",
+            "support",
+            "CSSTest",
+        ].iter().collect();
+        path.push(format!("{}.ttf", name));
+
+        let file = File::open(path).unwrap();
+
+        family.add_template(
+            Atom::from(name),
+            Some(file.bytes().map(|b| b.unwrap()).collect())
+        )
+    }
+}
+
+impl FontSource for TestFontSource {
+    fn get_font_instance(&mut self, _key: webrender_api::FontKey, _size: Au) -> webrender_api::FontInstanceKey {
+        webrender_api::FontInstanceKey(webrender_api::IdNamespace(0), 0)
+    }
+
+    fn find_font_template(
+        &mut self,
+        family: SingleFontFamily,
+        desc: FontTemplateDescriptor
+    ) -> Option<FontTemplateInfo> {
+        let handle = &self.handle;
+
+        self.find_font_count.set(self.find_font_count.get() + 1);
+        self.families
+            .get_mut(family.atom())
+            .and_then(|family| family.find_font_for_style(&desc, handle))
+            .map(|template| {
+                FontTemplateInfo {
+                    font_template: template,
+                    font_key: webrender_api::FontKey(webrender_api::IdNamespace(0), 0),
+                }
+            })
+    }
+
+    fn last_resort_font_template(&mut self, _desc: FontTemplateDescriptor) -> FontTemplateInfo {
+        unimplemented!();
+    }
+}
+
+fn style() -> FontStyleStruct {
+    let mut style = FontStyleStruct {
+        font_family: FontFamily::serif(),
+        font_style: FontStyle::Normal,
+        font_variant_caps: FontVariantCaps::Normal,
+        font_weight: FontWeight::normal(),
+        font_size: FontSize::medium(),
+        font_stretch: FontStretch::Normal,
+        hash: 0,
+    };
+    style.compute_font_hash();
+    style
+}
+
+fn font_family(names: Vec<&str>) -> FontFamily {
+    let names: Vec<SingleFontFamily> = names.into_iter().map(|name|
+        SingleFontFamily::FamilyName(FamilyName {
+            name: Atom::from(name),
+            syntax: FamilyNameSyntax::Quoted,
+        })
+    ).collect();
+
+    FontFamily(FontFamilyList::new(names.into_boxed_slice()))
+}
+
+#[test]
+fn test_font_group_is_cached_by_style() {
+    let source = TestFontSource::new();
+    let mut context = FontContext::new(source);
+
+    let style1 = style();
+
+    let mut style2 = style();
+    style2.set_font_style(FontStyle::Italic);
+
+    assert_eq!(
+        context.font_group(Arc::new(style1.clone())).as_ptr(),
+        context.font_group(Arc::new(style1.clone())).as_ptr(),
+        "the same font group should be returned for two styles with the same hash"
+    );
+
+    assert_ne!(
+        context.font_group(Arc::new(style1.clone())).as_ptr(),
+        context.font_group(Arc::new(style2.clone())).as_ptr(),
+        "different font groups should be returned for two styles with different hashes"
+    )
+}
+
+#[test]
+fn test_font_group_find_by_codepoint() {
+    let source = TestFontSource::new();
+    let count = source.find_font_count.clone();
+    let mut context = FontContext::new(source);
+
+    let mut style = style();
+    style.set_font_family(font_family(vec!("CSSTest ASCII", "CSSTest Basic")));
+
+    let group = context.font_group(Arc::new(style));
+
+    let font = group.borrow_mut().find_by_codepoint(&mut context, 'a').unwrap();
+    assert_eq!(font.borrow().handle.family_name(), "CSSTest ASCII");
+    assert_eq!(count.get(), 1, "only the first font in the list should have been loaded");
+
+    let font = group.borrow_mut().find_by_codepoint(&mut context, 'a').unwrap();
+    assert_eq!(font.borrow().handle.family_name(), "CSSTest ASCII");
+    assert_eq!(count.get(), 1, "we shouldn't load the same font a second time");
+
+    let font = group.borrow_mut().find_by_codepoint(&mut context, 'á').unwrap();
+    assert_eq!(font.borrow().handle.family_name(), "CSSTest Basic");
+    assert_eq!(count.get(), 2, "both fonts should now have been loaded");
+}

components/gfx/tests/support/CSSTest/LICENSE

@@ -0,0 +1,94 @@
+Copyright (c) 2003-2008 SIL International (http://www.sil.org/),
+with Reserved Font Names "Gentium" and "SIL".
+
+This Font Software is licensed under the SIL Open Font License, Version 1.1.
+This license is copied below, and is also available with a FAQ at:
+http://scripts.sil.org/OFL
+
+
+-----------------------------------------------------------
+SIL OPEN FONT LICENSE Version 1.1 - 1 February 2007
+-----------------------------------------------------------
+
+PREAMBLE
+The goals of the Open Font License (OFL) are to stimulate worldwide
+development of collaborative font projects, to support the font creation
+efforts of academic and linguistic communities, and to provide a free and
+open framework in which fonts may be shared and improved in partnership
+with others.
+
+The OFL allows the licensed fonts to be used, studied, modified and
+redistributed freely as long as they are not sold by themselves. The
+fonts, including any derivative works, can be bundled, embedded,
+redistributed and/or sold with any software provided that the font
+names of derivative works are changed. The fonts and derivatives,
+however, cannot be released under any other type of license. The
+requirement for fonts to remain under this license does not apply
+to any document created using the fonts or their derivatives.
+
+DEFINITIONS
+"Font Software" refers to the set of files released by the Copyright
+Holder(s) under this license and clearly marked as such. This may
+include source files, build scripts and documentation.
+
+"Reserved Font Name" refers to any names specified as such after the
+copyright statement(s).
+
+"Original Version" refers to the collection of Font Software components as
+distributed by the Copyright Holder(s).
+
+"Modified Version" refers to any derivative made by adding to, deleting,
+or substituting -- in part or in whole -- any of the components of the
+Original Version, by changing formats or by porting the Font Software to a
+new environment.
+
+"Author" refers to any designer, engineer, programmer, technical
+writer or other person who contributed to the Font Software.
+
+PERMISSION & CONDITIONS
+Permission is hereby granted, free of charge, to any person obtaining
+a copy of the Font Software, to use, study, copy, merge, embed, modify,
+redistribute, and sell modified and unmodified copies of the Font
+Software, subject to the following conditions:
+
+1) Neither the Font Software nor any of its individual components,
+in Original or Modified Versions, may be sold by itself.
+
+2) Original or Modified Versions of the Font Software may be bundled,
+redistributed and/or sold with any software, provided that each copy
+contains the above copyright notice and this license. These can be
+included either as stand-alone text files, human-readable headers or
+in the appropriate machine-readable metadata fields within text or
+binary files as long as those fields can be easily viewed by the user.
+
+3) No Modified Version of the Font Software may use the Reserved Font
+Name(s) unless explicit written permission is granted by the corresponding
+Copyright Holder. This restriction only applies to the primary font name as
+presented to the users.
+
+4) The name(s) of the Copyright Holder(s) or the Author(s) of the Font
+Software shall not be used to promote, endorse or advertise any
+Modified Version, except to acknowledge the contribution(s) of the
+Copyright Holder(s) and the Author(s) or with their explicit written
+permission.
+
+5) The Font Software, modified or unmodified, in part or in whole,
+must be distributed entirely under this license, and must not be
+distributed under any other license. The requirement for fonts to
+remain under this license does not apply to any document created
+using the Font Software.
+
+TERMINATION
+This license becomes null and void if any of the above conditions are
+not met.
+
+DISCLAIMER
+THE FONT SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO ANY WARRANTIES OF
+MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT
+OF COPYRIGHT, PATENT, TRADEMARK, OR OTHER RIGHT. IN NO EVENT SHALL THE
+COPYRIGHT HOLDER BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+INCLUDING ANY GENERAL, SPECIAL, INDIRECT, INCIDENTAL, OR CONSEQUENTIAL
+DAMAGES, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+FROM, OUT OF THE USE OR INABILITY TO USE THE FONT SOFTWARE OR FROM
+OTHER DEALINGS IN THE FONT SOFTWARE.

components/gfx/tests/support/CSSTest/README

@@ -0,0 +1,25 @@
+These fonts are a copy of the CSSTest fonts in web-platform-tests, so that we
+can use them for unit-testing our font code. Here is the README from
+web-platform-tests:
+
+-----
+
+These fonts were created to support the testing of the font features
+in CSS, and are required to run some of the tests for those features.
+
+The fonts are modified versions of Gentium Basic, licensed by SIL under
+the Open Font License which allows modifications as long as the terms
+of the license are met.
+
+The original fonts were used to create the family 'CSSTest Basic'. This
+family has four faces and can be used for testing bold / italics.
+A subsetted version of this font with only glyphs for basic ASCII
+characters is 'CSSTest ASCII'.  This was used to make the other
+variations.  Most of the modications are to the name table and character
+maps, for the most part glyphs were not modified.
+
+The fonts are available for download both individually and as a
+ZIP package below.
+
+The files test.html and test.xhtml test that the fonts have been
+correctly installed.

components/layout/context.rs

@@ -27,10 +27,12 @@ use std::thread;
 use style::context::RegisteredSpeculativePainter;
 use style::context::SharedStyleContext;
 
-thread_local!(static FONT_CONTEXT_KEY: RefCell<Option<FontContext>> = RefCell::new(None));
+pub type LayoutFontContext = FontContext<FontCacheThread>;
+
+thread_local!(static FONT_CONTEXT_KEY: RefCell<Option<LayoutFontContext>> = RefCell::new(None));
 
 pub fn with_thread_local_font_context<F, R>(layout_context: &LayoutContext, f: F) -> R
-    where F: FnOnce(&mut FontContext) -> R
+    where F: FnOnce(&mut LayoutFontContext) -> R
 {
     FONT_CONTEXT_KEY.with(|k| {
         let mut font_context = k.borrow_mut();

components/layout/inline.rs

@@ -7,7 +7,7 @@
 use ServoArc;
 use app_units::{Au, MIN_AU};
 use block::AbsoluteAssignBSizesTraversal;
-use context::LayoutContext;
+use context::{LayoutContext, LayoutFontContext};
 use display_list::{DisplayListBuildState, InlineFlowDisplayListBuilding};
 use display_list::StackingContextCollectionState;
 use euclid::{Point2D, Size2D};
@@ -20,7 +20,6 @@ use fragment::FragmentFlags;
 use fragment::SpecificFragmentInfo;
 use gfx::display_list::OpaqueNode;
 use gfx::font::FontMetrics;
-use gfx::font_context::FontContext;
 use gfx_traits::print_tree::PrintTree;
 use layout_debug;
 use model::IntrinsicISizesContribution;
@@ -1132,7 +1131,7 @@ impl InlineFlow {
     /// Computes the minimum metrics for each line. This is done during flow construction.
     ///
     /// `style` is the style of the block.
-    pub fn minimum_line_metrics(&self, font_context: &mut FontContext, style: &ComputedValues)
+    pub fn minimum_line_metrics(&self, font_context: &mut LayoutFontContext, style: &ComputedValues)
                                 -> LineMetrics {
         InlineFlow::minimum_line_metrics_for_fragments(&self.fragments.fragments,
                                                        font_context,
@@ -1144,7 +1143,7 @@ impl InlineFlow {
     ///
     /// `style` is the style of the block that these fragments belong to.
     pub fn minimum_line_metrics_for_fragments(fragments: &[Fragment],
-                                              font_context: &mut FontContext,
+                                              font_context: &mut LayoutFontContext,
                                               style: &ComputedValues)
                                               -> LineMetrics {
         // As a special case, if this flow contains only hypothetical fragments, then the entire

components/layout/text.rs

@@ -7,10 +7,10 @@
 #![deny(unsafe_code)]
 
 use app_units::Au;
+use context::LayoutFontContext;
 use fragment::{Fragment, ScannedTextFlags};
 use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo, UnscannedTextFragmentInfo};
-use gfx::font::{FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
-use gfx::font_context::FontContext;
+use gfx::font::{FontRef, FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
 use gfx::text::glyph::ByteIndex;
 use gfx::text::text_run::TextRun;
 use gfx::text::util::{self, CompressionMode};
@@ -18,6 +18,7 @@ use inline::{InlineFragmentNodeFlags, InlineFragments};
 use linked_list::split_off_head;
 use ordered_float::NotNaN;
 use range::Range;
+use servo_atoms::Atom;
 use std::borrow::ToOwned;
 use std::collections::LinkedList;
 use std::mem;
@@ -28,7 +29,7 @@ use style::computed_values::white_space::T as WhiteSpace;
 use style::computed_values::word_break::T as WordBreak;
 use style::logical_geometry::{LogicalSize, WritingMode};
 use style::properties::ComputedValues;
-use style::properties::style_structs;
+use style::properties::style_structs::Font as FontStyleStruct;
 use style::values::generics::text::LineHeight;
 use unicode_bidi as bidi;
 use unicode_script::{Script, get_script};
@@ -68,7 +69,7 @@ impl TextRunScanner {
     }
 
     pub fn scan_for_runs(&mut self,
-                         font_context: &mut FontContext,
+                         font_context: &mut LayoutFontContext,
                          mut fragments: LinkedList<Fragment>)
                          -> InlineFragments {
         debug!("TextRunScanner: scanning {} fragments for text runs...", fragments.len());
@@ -136,7 +137,7 @@ impl TextRunScanner {
     /// for correct painting order. Since we compress several leaf fragments here, the mapping must
     /// be adjusted.
     fn flush_clump_to_list(&mut self,
-                           font_context: &mut FontContext,
+                           mut font_context: &mut LayoutFontContext,
                            out_fragments: &mut Vec<Fragment>,
                            paragraph_bytes_processed: &mut usize,
                            bidi_levels: Option<&[bidi::Level]>,
@@ -159,7 +160,7 @@ impl TextRunScanner {
         // Concatenate all of the transformed strings together, saving the new character indices.
         let mut mappings: Vec<RunMapping> = Vec::new();
         let runs = {
-            let fontgroup;
+            let font_group;
             let compression;
             let text_transform;
             let letter_spacing;
@@ -170,7 +171,7 @@ impl TextRunScanner {
                 let in_fragment = self.clump.front().unwrap();
                 let font_style = in_fragment.style().clone_font();
                 let inherited_text_style = in_fragment.style().get_inheritedtext();
-                fontgroup = font_context.layout_font_group_for_style(font_style);
+                font_group = font_context.font_group(font_style);
                 compression = match in_fragment.white_space() {
                     WhiteSpace::Normal |
                     WhiteSpace::Nowrap => CompressionMode::CompressWhitespaceNewline,
@@ -214,14 +215,7 @@ impl TextRunScanner {
 
                 let (mut start_position, mut end_position) = (0, 0);
                 for (byte_index, character) in text.char_indices() {
-                    // Search for the first font in this font group that contains a glyph for this
-                    // character.
-                    let font_index = fontgroup.fonts.iter().position(|font| {
-                        font.borrow().glyph_index(character).is_some()
-                    }).unwrap_or(0);
-
-                    // The following code panics one way or another if this condition isn't met.
-                    assert!(fontgroup.fonts.len() > 0);
+                    let font = font_group.borrow_mut().find_by_codepoint(&mut font_context, character);
 
                     let bidi_level = match bidi_levels {
                         Some(levels) => levels[*paragraph_bytes_processed],
@@ -245,7 +239,7 @@ impl TextRunScanner {
                     };
 
                     // Now, if necessary, flush the mapping we were building up.
-                    let flush_run = run_info.font_index != font_index ||
+                    let flush_run = !run_info.has_font(&font) ||
                                     run_info.bidi_level != bidi_level ||
                                     !compatible_script;
                     let new_mapping_needed = flush_run || mapping.selected != selected;
@@ -272,7 +266,7 @@ impl TextRunScanner {
                             mapping = RunMapping::new(&run_info_list[..],
                                                       fragment_index);
                         }
-                        run_info.font_index = font_index;
+                        run_info.font = font;
                         run_info.bidi_level = bidi_level;
                         run_info.script = script;
                         mapping.selected = selected;
@@ -328,9 +322,14 @@ impl TextRunScanner {
                 if run_info.bidi_level.is_rtl() {
                     options.flags.insert(ShapingFlags::RTL_FLAG);
                 }
-                let mut font = fontgroup.fonts.get(run_info.font_index).unwrap().borrow_mut();
 
-                let (run, break_at_zero) = TextRun::new(&mut *font,
+                // If no font is found (including fallbacks), there's no way we can render.
+                let font =
+                    run_info.font
+                        .or_else(|| font_group.borrow_mut().first(&mut font_context))
+                        .expect("No font found for text run!");
+
+                let (run, break_at_zero) = TextRun::new(&mut *font.borrow_mut(),
                                                         run_info.text,
                                                         &options,
                                                         run_info.bidi_level,
@@ -456,15 +455,20 @@ fn bounding_box_for_run_metrics(metrics: &RunMetrics, writing_mode: WritingMode)
         metrics.bounding_box.size.height)
 }
 
-/// Returns the metrics of the font represented by the given `style_structs::Font`, respectively.
+/// Returns the metrics of the font represented by the given `FontStyleStruct`.
 ///
 /// `#[inline]` because often the caller only needs a few fields from the font metrics.
+///
+/// # Panics
+///
+/// Panics if no font can be found for the given font style.
 #[inline]
-pub fn font_metrics_for_style(font_context: &mut FontContext, font_style: ::ServoArc<style_structs::Font>)
+pub fn font_metrics_for_style(mut font_context: &mut LayoutFontContext, style: ::ServoArc<FontStyleStruct>)
                               -> FontMetrics {
-    let fontgroup = font_context.layout_font_group_for_style(font_style);
-    // FIXME(https://github.com/rust-lang/rust/issues/23338)
-    let font = fontgroup.fonts[0].borrow();
+    let font_group = font_context.font_group(style);
+    let font = font_group.borrow_mut().first(&mut font_context);
+    let font = font.as_ref().unwrap().borrow();
+
     font.metrics.clone()
 }
 
@@ -546,8 +550,8 @@ struct RunInfo {
     text: String,
     /// The insertion point in this text run, if applicable.
     insertion_point: Option<ByteIndex>,
-    /// The index of the applicable font in the font group.
-    font_index: usize,
+    /// The font that the text should be rendered with.
+    font: Option<FontRef>,
     /// The bidirection embedding level of this text run.
     bidi_level: bidi::Level,
     /// The Unicode script property of this text run.
@@ -559,7 +563,7 @@ impl RunInfo {
         RunInfo {
             text: String::new(),
             insertion_point: None,
-            font_index: 0,
+            font: None,
             bidi_level: bidi::Level::ltr(),
             script: Script::Common,
         }
@@ -584,6 +588,14 @@ impl RunInfo {
         }
         list.push(self);
     }
+
+    fn has_font(&self, font: &Option<FontRef>) -> bool {
+        fn identifier(font: &Option<FontRef>) -> Option<Atom> {
+            font.as_ref().map(|f| f.borrow().identifier())
+        }
+
+        identifier(&self.font) == identifier(font)
+    }
 }
 
 /// A mapping from a portion of an unscanned text fragment to the text run we're going to create