Simplify `FontHandle` and rename it to `PlatformFont` (#32101)

* Simplify `FontHandle` and rename it to `PlatformFont`

Rename it to `PlatformFont` and move the `FontTemplate` member to
`Font`, because it's shared by all platforms.

* Update components/gfx/platform/freetype/font.rs

Co-authored-by: Mukilan Thiyagarajan <mukilanthiagarajan@gmail.com>

* Fix build for MacOS and Windows

---------

Co-authored-by: Mukilan Thiyagarajan <mukilanthiagarajan@gmail.com>

9 files changed, 71 insertions, 80 deletions

diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs
index 8b62a183733..5bb4640dfd1 100644
--- a/components/canvas/canvas_data.rs
+++ b/components/canvas/canvas_data.rs
@@ -14,7 +14,6 @@ use font_kit::font::Font;
 use font_kit::metrics::Metrics;
 use font_kit::properties::{Properties, Stretch, Style, Weight};
 use font_kit::source::SystemSource;
-use gfx::font::FontHandleMethods;
 use gfx::font_cache_thread::FontCacheThread;
 use gfx::font_context::FontContext;
 use gfx::font_template::FontTemplateRefMethods;
@@ -502,7 +501,7 @@ impl<'a> CanvasData<'a> {
                         .first(font_context)
                         .expect("couldn't find font");
                     let font = font.borrow_mut();
-                    Font::from_bytes(font.handle.template().data(), 0)
+                    Font::from_bytes(font.template.data(), 0)
                         .ok()
                         .or_else(|| load_system_font_from_style(Some(style)))
                 })
diff --git a/components/gfx/font.rs b/components/gfx/font.rs
index ad1ef24e313..50486aa9690 100644
--- a/components/gfx/font.rs
+++ b/components/gfx/font.rs
@@ -27,7 +27,7 @@ use webrender_api::FontInstanceKey;
 use crate::font_cache_thread::FontIdentifier;
 use crate::font_context::{FontContext, FontSource};
 use crate::font_template::{FontTemplateDescriptor, FontTemplateRef};
-use crate::platform::font::{FontHandle, FontTable};
+use crate::platform::font::{FontTable, PlatformFont};
 pub use crate::platform::font_list::fallback_font_families;
 use crate::text::glyph::{ByteIndex, GlyphData, GlyphId, GlyphStore};
 use crate::text::shaping::ShaperMethods;
@@ -47,18 +47,17 @@ pub const LAST_RESORT_GLYPH_ADVANCE: FractionalPixel = 10.0;
 
 static TEXT_SHAPING_PERFORMANCE_COUNTER: AtomicUsize = AtomicUsize::new(0);
 
-// FontHandle encapsulates access to the platform's font API,
+// PlatformFont encapsulates access to the platform's font API,
 // e.g. quartz, FreeType. It provides access to metrics and tables
 // needed by the text shaper as well as access to the underlying font
 // resources needed by the graphics layer to draw glyphs.
 
-pub trait FontHandleMethods: Sized {
+pub trait PlatformFontMethods: Sized {
     fn new_from_template(
         template: FontTemplateRef,
         pt_size: Option<Au>,
     ) -> Result<Self, &'static str>;
 
-    fn template(&self) -> FontTemplateRef;
     fn family_name(&self) -> Option<String>;
     fn face_name(&self) -> Option<String>;
 
@@ -161,7 +160,8 @@ impl<'a> From<&'a FontStyleStruct> for FontDescriptor {
 
 #[derive(Debug)]
 pub struct Font {
-    pub handle: FontHandle,
+    pub handle: PlatformFont,
+    pub template: FontTemplateRef,
     pub metrics: FontMetrics,
     pub descriptor: FontDescriptor,
     shaper: Option<Shaper>,
@@ -177,15 +177,17 @@ pub struct Font {
 
 impl Font {
     pub fn new(
-        handle: FontHandle,
+        template: FontTemplateRef,
         descriptor: FontDescriptor,
         font_key: FontInstanceKey,
         synthesized_small_caps: Option<FontRef>,
-    ) -> Font {
+    ) -> Result<Font, &'static str> {
+        let handle = PlatformFont::new_from_template(template.clone(), Some(descriptor.pt_size))?;
         let metrics = handle.metrics();
 
-        Font {
+        Ok(Font {
             handle,
+            template,
             shaper: None,
             descriptor,
             metrics,
@@ -193,12 +195,12 @@ impl Font {
             glyph_advance_cache: RefCell::new(HashMap::new()),
             font_key,
             synthesized_small_caps,
-        }
+        })
     }
 
     /// A unique identifier for the font, allowing comparison.
     pub fn identifier(&self) -> FontIdentifier {
-        self.handle.template().borrow().identifier.clone()
+        self.template.borrow().identifier.clone()
     }
 }
 
diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs
index 257aeeb41c2..16ba06cf2b1 100644
--- a/components/gfx/font_context.rs
+++ b/components/gfx/font_context.rs
@@ -17,14 +17,11 @@ use style::computed_values::font_variant_caps::T as FontVariantCaps;
 use style::properties::style_structs::Font as FontStyleStruct;
 use webrender_api::{FontInstanceKey, FontKey};
 
-use crate::font::{
-    Font, FontDescriptor, FontFamilyDescriptor, FontGroup, FontHandleMethods, FontRef,
-};
+use crate::font::{Font, FontDescriptor, FontFamilyDescriptor, FontGroup, FontRef};
 use crate::font_cache_thread::FontTemplateAndWebRenderFontKey;
 #[cfg(target_os = "macos")]
 use crate::font_template::FontTemplate;
 use crate::font_template::FontTemplateDescriptor;
-use crate::platform::font::FontHandle;
 
 static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8; // Matches FireFox (see gfxFont.h)
 
@@ -214,17 +211,15 @@ impl<S: FontSource> FontContext<S> {
         descriptor: FontDescriptor,
         synthesized_small_caps: Option<FontRef>,
     ) -> Result<Font, &'static str> {
-        let handle = FontHandle::new_from_template(info.font_template, Some(descriptor.pt_size))?;
-
         let font_instance_key = self
             .font_source
             .get_font_instance(info.font_key, descriptor.pt_size);
-        Ok(Font::new(
-            handle,
+        Font::new(
+            info.font_template,
             descriptor,
             font_instance_key,
             synthesized_small_caps,
-        ))
+        )
     }
 }
 
diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs
index 226017efa79..ccc7ac781bd 100644
--- a/components/gfx/font_template.rs
+++ b/components/gfx/font_template.rs
@@ -16,9 +16,9 @@ use style::properties::style_structs::Font as FontStyleStruct;
 use style::values::computed::font::FontWeight;
 use webrender_api::NativeFontHandle;
 
-use crate::font::FontHandleMethods;
+use crate::font::PlatformFontMethods;
 use crate::font_cache_thread::FontIdentifier;
-use crate::platform::font::FontHandle;
+use crate::platform::font::PlatformFont;
 
 /// A reference to a [`FontTemplate`] with shared ownership and mutability.
 pub(crate) type FontTemplateRef = Rc<RefCell<FontTemplate>>;
@@ -191,10 +191,10 @@ impl FontTemplateRefMethods for FontTemplateRef {
             return Err("Invalid font template");
         }
 
-        let handle = FontHandleMethods::new_from_template(self.clone(), None);
+        let handle = PlatformFontMethods::new_from_template(self.clone(), None);
         let mut template = self.borrow_mut();
         template.is_valid = handle.is_ok();
-        let handle: FontHandle = handle?;
+        let handle: PlatformFont = handle?;
         template.descriptor = Some(FontTemplateDescriptor::new(
             handle.boldness(),
             handle.stretchiness(),
diff --git a/components/gfx/platform/freetype/font.rs b/components/gfx/platform/freetype/font.rs
index 7d9f9c6a507..13f29e1939c 100644
--- a/components/gfx/platform/freetype/font.rs
+++ b/components/gfx/platform/freetype/font.rs
@@ -4,6 +4,7 @@
 
 use std::ffi::CString;
 use std::os::raw::{c_char, c_long};
+use std::sync::Arc;
 use std::{mem, ptr};
 
 use app_units::Au;
@@ -23,7 +24,7 @@ use style::values::computed::font::FontStyle;
 use super::c_str_to_string;
 use super::library_handle::FreeTypeLibraryHandle;
 use crate::font::{
-    FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, GPOS, GSUB,
+    FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, PlatformFontMethods, GPOS, GSUB,
     KERN,
 };
 use crate::font_cache_thread::FontIdentifier;
@@ -70,15 +71,18 @@ struct OS2Table {
 
 #[derive(Debug)]
 #[allow(unused)]
-pub struct FontHandle {
-    // The template contains the font data, which must stay valid for the
-    // lifetime of the platform [`FT_Face`], if it's created via [`FT_Memory_Face`].
-    font_template: FontTemplateRef,
+pub struct PlatformFont {
+    /// The font data itself, which must stay valid for the lifetime of the
+    /// platform [`FT_Face`], if it's created via [`FT_New_Memory_Face`]. A reference
+    /// to this data is also stored in the [`crate::font::Font`] that holds
+    /// this [`PlatformFont`], but if it's ever separated from it's font,
+    /// this ensures the data stays alive.
+    font_data: Option<Arc<Vec<u8>>>,
     face: FT_Face,
     can_do_fast_shaping: bool,
 }
 
-impl Drop for FontHandle {
+impl Drop for PlatformFont {
     fn drop(&mut self) {
         assert!(!self.face.is_null());
         unsafe {
@@ -134,22 +138,22 @@ fn create_face(template: &FontTemplateRef, pt_size: Option<Au>) -> Result<FT_Fac
         }
 
         if let Some(s) = pt_size {
-            FontHandle::set_char_size(face, s)?
+            PlatformFont::set_char_size(face, s)?
         }
 
         Ok(face)
     }
 }
 
-impl FontHandleMethods for FontHandle {
+impl PlatformFontMethods for PlatformFont {
     fn new_from_template(
         template: FontTemplateRef,
         pt_size: Option<Au>,
-    ) -> Result<FontHandle, &'static str> {
+    ) -> Result<PlatformFont, &'static str> {
         let face = create_face(&template, pt_size)?;
-        let mut handle = FontHandle {
+        let mut handle = PlatformFont {
             face,
-            font_template: template.clone(),
+            font_data: template.borrow().data_if_in_memory(),
             can_do_fast_shaping: false,
         };
         // TODO (#11310): Implement basic support for GPOS and GSUB.
@@ -158,10 +162,6 @@ impl FontHandleMethods for FontHandle {
         Ok(handle)
     }
 
-    fn template(&self) -> FontTemplateRef {
-        self.font_template.clone()
-    }
-
     fn family_name(&self) -> Option<String> {
         unsafe {
             let family_name = (*self.face).family_name;
@@ -362,7 +362,7 @@ impl FontHandleMethods for FontHandle {
     }
 }
 
-impl<'a> FontHandle {
+impl<'a> PlatformFont {
     fn set_char_size(face: FT_Face, pt_size: Au) -> Result<(), &'static str> {
         let char_size = pt_size.to_f64_px() * 64.0 + 0.5;
 
diff --git a/components/gfx/platform/macos/font.rs b/components/gfx/platform/macos/font.rs
index 05e8fdc0614..477ec36e3cd 100644
--- a/components/gfx/platform/macos/font.rs
+++ b/components/gfx/platform/macos/font.rs
@@ -4,6 +4,7 @@
 
 use std::cmp::Ordering;
 use std::ops::Range;
+use std::sync::Arc;
 use std::{fmt, ptr};
 
 /// Implementation of Quartz (CoreGraphics) fonts.
@@ -22,7 +23,7 @@ use style::values::computed::font::{FontStretch, FontStyle, FontWeight};
 
 use super::font_template::CoreTextFontTemplateMethods;
 use crate::font::{
-    FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, GPOS, GSUB,
+    FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, PlatformFontMethods, GPOS, GSUB,
     KERN,
 };
 use crate::font_template::FontTemplateRef;
@@ -52,14 +53,16 @@ impl FontTableMethods for FontTable {
 }
 
 #[derive(Debug)]
-pub struct FontHandle {
-    font_template: FontTemplateRef,
+pub struct PlatformFont {
     ctfont: CTFont,
+    /// A reference to this data used to create this [`PlatformFont`], ensuring the
+    /// data stays alive of the lifetime of this struct.
+    _data: Option<Arc<Vec<u8>>>,
     h_kern_subtable: Option<CachedKernTable>,
     can_do_fast_shaping: bool,
 }
 
-impl FontHandle {
+impl PlatformFont {
     /// Cache all the data needed for basic horizontal kerning. This is used only as a fallback or
     /// fast path (when the GPOS table is missing or unnecessary) so it needn't handle every case.
     fn find_h_kern_subtable(&self) -> Option<CachedKernTable> {
@@ -154,11 +157,11 @@ impl fmt::Debug for CachedKernTable {
     }
 }
 
-impl FontHandleMethods for FontHandle {
+impl PlatformFontMethods for PlatformFont {
     fn new_from_template(
         font_template: FontTemplateRef,
         pt_size: Option<Au>,
-    ) -> Result<FontHandle, &'static str> {
+    ) -> Result<PlatformFont, &'static str> {
         let size = match pt_size {
             Some(s) => s.to_f64_px(),
             None => 0.0,
@@ -167,8 +170,8 @@ impl FontHandleMethods for FontHandle {
             return Err("Could not generate CTFont for FontTemplateData");
         };
 
-        let mut handle = FontHandle {
-            font_template,
+        let mut handle = PlatformFont {
+            _data: font_template.borrow().data_if_in_memory(),
             ctfont: core_text_font.clone_with_font_size(size),
             h_kern_subtable: None,
             can_do_fast_shaping: false,
@@ -181,10 +184,6 @@ impl FontHandleMethods for FontHandle {
         Ok(handle)
     }
 
-    fn template(&self) -> FontTemplateRef {
-        self.font_template.clone()
-    }
-
     fn family_name(&self) -> Option<String> {
         Some(self.ctfont.family_name())
     }
diff --git a/components/gfx/platform/windows/font.rs b/components/gfx/platform/windows/font.rs
index a2156f85e73..3baef67ca28 100644
--- a/components/gfx/platform/windows/font.rs
+++ b/components/gfx/platform/windows/font.rs
@@ -8,6 +8,7 @@
 
 use std::fmt;
 use std::ops::Deref;
+use std::sync::Arc;
 
 use app_units::Au;
 use dwrote::{Font, FontFace, FontFile, FontStretch, FontStyle};
@@ -18,7 +19,7 @@ use style::values::computed::font::FontStyle as StyleFontStyle;
 use style::values::specified::font::FontStretchKeyword;
 
 use crate::font::{
-    FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel,
+    FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, PlatformFontMethods,
 };
 use crate::font_cache_thread::FontIdentifier;
 use crate::font_template::{FontTemplateRef, FontTemplateRefMethods};
@@ -198,9 +199,11 @@ impl FontInfo {
 }
 
 #[derive(Debug)]
-pub struct FontHandle {
-    font_template: FontTemplateRef,
+pub struct PlatformFont {
     face: Nondebug<FontFace>,
+    /// A reference to this data used to create this [`PlatformFont`], ensuring the
+    /// data stays alive of the lifetime of this struct.
+    data: Option<Arc<Vec<u8>>>,
     info: FontInfo,
     em_size: f32,
     du_to_px: f32,
@@ -222,9 +225,7 @@ impl<T> Deref for Nondebug<T> {
     }
 }
 
-impl FontHandle {}
-
-impl FontHandleMethods for FontHandle {
+impl PlatformFontMethods for PlatformFont {
     fn new_from_template(
         font_template: FontTemplateRef,
         pt_size: Option<Au>,
@@ -234,8 +235,12 @@ impl FontHandleMethods for FontHandle {
             FontIdentifier::Web(_) => None,
         };
 
-        let (face, info) = match direct_write_font {
-            Some(font) => (font.create_font_face(), FontInfo::new_from_font(&font)?),
+        let (face, info, data) = match direct_write_font {
+            Some(font) => (
+                font.create_font_face(),
+                FontInfo::new_from_font(&font)?,
+                None,
+            ),
             None => {
                 let bytes = font_template.data();
                 let font_file =
@@ -244,7 +249,7 @@ impl FontHandleMethods for FontHandle {
                     .create_face(0, dwrote::DWRITE_FONT_SIMULATIONS_NONE)
                     .map_err(|_| "Could not create FontFace")?;
                 let info = FontInfo::new_from_face(&face)?;
-                (face, info)
+                (face, info, font_template.borrow().data_if_in_memory())
             },
         };
 
@@ -257,9 +262,9 @@ impl FontHandleMethods for FontHandle {
         let design_units_to_pixels = 1. / design_units_per_pixel;
         let scaled_design_units_to_pixels = em_size / design_units_per_pixel;
 
-        Ok(FontHandle {
-            font_template,
+        Ok(PlatformFont {
             face: Nondebug(face),
+            data,
             info,
             em_size,
             du_to_px: design_units_to_pixels,
@@ -267,10 +272,6 @@ impl FontHandleMethods for FontHandle {
         })
     }
 
-    fn template(&self) -> FontTemplateRef {
-        self.font_template.clone()
-    }
-
     fn family_name(&self) -> Option<String> {
         Some(self.info.family_name.clone())
     }
diff --git a/components/gfx/tests/font_context.rs b/components/gfx/tests/font_context.rs
index 8e8de147d3e..6a787ae9996 100644
--- a/components/gfx/tests/font_context.rs
+++ b/components/gfx/tests/font_context.rs
@@ -11,8 +11,7 @@ use std::rc::Rc;
 
 use app_units::Au;
 use gfx::font::{
-    fallback_font_families, FontDescriptor, FontFamilyDescriptor, FontFamilyName,
-    FontHandleMethods, FontSearchScope,
+    fallback_font_families, FontDescriptor, FontFamilyDescriptor, FontFamilyName, FontSearchScope,
 };
 use gfx::font_cache_thread::{FontIdentifier, FontTemplateAndWebRenderFontKey, FontTemplates};
 use gfx::font_context::{FontContext, FontSource};
@@ -176,7 +175,7 @@ fn test_font_group_find_by_codepoint() {
         .find_by_codepoint(&mut context, 'a')
         .unwrap();
     assert_eq!(
-        font.borrow().handle.template().borrow().identifier,
+        font.borrow().template.borrow().identifier,
         TestFontSource::identifier_for_font_name("csstest-ascii")
     );
     assert_eq!(
@@ -190,7 +189,7 @@ fn test_font_group_find_by_codepoint() {
         .find_by_codepoint(&mut context, 'a')
         .unwrap();
     assert_eq!(
-        font.borrow().handle.template().borrow().identifier,
+        font.borrow().template.borrow().identifier,
         TestFontSource::identifier_for_font_name("csstest-ascii")
     );
     assert_eq!(
@@ -204,7 +203,7 @@ fn test_font_group_find_by_codepoint() {
         .find_by_codepoint(&mut context, 'á')
         .unwrap();
     assert_eq!(
-        font.borrow().handle.template().borrow().identifier,
+        font.borrow().template.borrow().identifier,
         TestFontSource::identifier_for_font_name("csstest-basic-regular")
     );
     assert_eq!(count.get(), 2, "both fonts should now have been loaded");
@@ -225,7 +224,7 @@ fn test_font_fallback() {
         .find_by_codepoint(&mut context, 'a')
         .unwrap();
     assert_eq!(
-        font.borrow().handle.template().borrow().identifier,
+        font.borrow().template.borrow().identifier,
         TestFontSource::identifier_for_font_name("csstest-ascii"),
         "a family in the group should be used if there is a matching glyph"
     );
@@ -235,7 +234,7 @@ fn test_font_fallback() {
         .find_by_codepoint(&mut context, 'á')
         .unwrap();
     assert_eq!(
-        font.borrow().handle.template().borrow().identifier,
+        font.borrow().template.borrow().identifier,
         TestFontSource::identifier_for_font_name("csstest-basic-regular"),
         "a fallback font should be used if there is no matching glyph in the group"
     );
diff --git a/components/gfx/text/text_run.rs b/components/gfx/text/text_run.rs
index 56f66d6cd1f..3973702a026 100644
--- a/components/gfx/text/text_run.rs
+++ b/components/gfx/text/text_run.rs
@@ -16,8 +16,7 @@ use unicode_bidi as bidi;
 use webrender_api::FontInstanceKey;
 use xi_unicode::LineBreakLeafIter;
 
-use crate::font::{Font, FontHandleMethods, FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
-use crate::font_template::FontTemplateRefMethods;
+use crate::font::{Font, FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
 use crate::text::glyph::{ByteIndex, GlyphStore};
 
 thread_local! {
@@ -30,8 +29,6 @@ thread_local! {
 pub struct TextRun {
     /// The UTF-8 string represented by this text run.
     pub text: Arc<String>,
-    /// This ensures the data stays alive as long as this TextRun is using this font.
-    font_data: Arc<Vec<u8>>,
     pub pt_size: Au,
     pub font_metrics: FontMetrics,
     pub font_key: FontInstanceKey,
@@ -194,7 +191,6 @@ impl<'a> TextRun {
             TextRun {
                 text: Arc::new(text),
                 font_metrics: font.metrics.clone(),
-                font_data: font.handle.template().data(),
                 font_key: font.font_key,
                 pt_size: font.descriptor.pt_size,
                 glyphs: Arc::new(glyphs),