Bug 1351200 - Part 5: stylo: Use font metrics provider as a cache for font size results; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Tue, 04 Apr 2017 11:11:27 -0700
changeset 556353 5f6692e072c5c59056f9382ca065dcad62993a50
parent 556352 3368c13c34eb5f3e4d12e6c32d7222dfe902b73b
child 622860 c25babdef4e49b03b7a6ecc6dd0be96edf4d7eda
push id52517
push userbmo:manishearth@gmail.com
push dateWed, 05 Apr 2017 17:18:20 +0000
reviewersemilio
bugs1351200
milestone55.0a1
Bug 1351200 - Part 5: stylo: Use font metrics provider as a cache for font size results; r?emilio MozReview-Commit-ID: KGR6Tbyu3sT
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
servo/components/style/build_gecko.rs
servo/components/style/font_metrics.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/gecko_bindings/bindings.rs
servo/components/style/gecko_bindings/structs_debug.rs
servo/components/style/gecko_bindings/structs_release.rs
servo/components/style/gecko_string_cache/mod.rs
servo/components/style/properties/longhand/font.mako.rs
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1524,22 +1524,20 @@ Gecko_nsStyleFont_SetLang(nsStyleFont* a
 
 void
 Gecko_nsStyleFont_CopyLangFrom(nsStyleFont* aFont, const nsStyleFont* aSource)
 {
   aFont->mLanguage = aSource->mLanguage;
 }
 
 FontSizePrefs
-Gecko_GetBaseSize(nsIAtom* aLanguage, RawGeckoPresContextBorrowed aPresContext)
+Gecko_GetBaseSize(nsIAtom* aLanguage)
 {
-  nsIAtom* lang =  aLanguage ? aLanguage : aPresContext->GetLanguageFromCharset();
   LangGroupFontPrefs prefs;
-
-  nsIAtom *langGroupAtom = StaticPresData::Get()->GetLangGroup(lang);
+  nsIAtom *langGroupAtom = StaticPresData::Get()->GetLangGroup(aLanguage);
 
   prefs.Initialize(langGroupAtom);
   FontSizePrefs sizes;
 
   sizes.mDefaultVariableSize = prefs.mDefaultVariableFont.size;
   sizes.mDefaultFixedSize = prefs.mDefaultFixedFont.size;
   sizes.mDefaultSerifSize = prefs.mDefaultSerifFont.size;
   sizes.mDefaultSansSerifSize = prefs.mDefaultSansSerifFont.size;
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -375,17 +375,17 @@ void Gecko_CSSValue_SetArray(nsCSSValueB
 void Gecko_CSSValue_SetURL(nsCSSValueBorrowedMut css_value, ServoBundledURI uri);
 void Gecko_CSSValue_SetInt(nsCSSValueBorrowedMut css_value, int32_t integer, nsCSSUnit unit);
 void Gecko_CSSValue_Drop(nsCSSValueBorrowedMut css_value);
 NS_DECL_THREADSAFE_FFI_REFCOUNTING(nsCSSValueSharedList, CSSValueSharedList);
 bool Gecko_PropertyId_IsPrefEnabled(nsCSSPropertyID id);
 
 void Gecko_nsStyleFont_SetLang(nsStyleFont* font, nsIAtom* atom);
 void Gecko_nsStyleFont_CopyLangFrom(nsStyleFont* aFont, const nsStyleFont* aSource);
-FontSizePrefs Gecko_GetBaseSize(nsIAtom* lang, RawGeckoPresContextBorrowed pres_context);
+FontSizePrefs Gecko_GetBaseSize(nsIAtom* lang);
 
 const nsMediaFeature* Gecko_GetMediaFeatures();
 
 // Font face rule
 // Creates and returns a new (already-addrefed) nsCSSFontFaceRule object.
 nsCSSFontFaceRule* Gecko_CSSFontFaceRule_Create();
 void Gecko_CSSFontFaceRule_GetCssText(const nsCSSFontFaceRule* rule, nsAString* result);
 NS_DECL_FFI_REFCOUNTING(nsCSSFontFaceRule, CSSFontFaceRule);
--- a/servo/components/style/build_gecko.rs
+++ b/servo/components/style/build_gecko.rs
@@ -300,16 +300,17 @@ mod bindings {
             "NS_FONT_.*",
             "NS_STYLE_.*",
             "NS_RADIUS_.*",
             "BORDER_COLOR_.*",
             "BORDER_STYLE_.*",
             "mozilla::SERVO_PREF_.*",
             "kNameSpaceID_.*",
             "kGenericFont_.*",
+            "kPresContext_.*",
         ];
         let whitelist = [
             "RawGecko.*",
             "mozilla::ServoStyleSheet",
             "mozilla::ServoElementSnapshot.*",
             "mozilla::CSSPseudoClassType",
             "mozilla::css::SheetParsingMode",
             "mozilla::HalfCorner",
--- a/servo/components/style/font_metrics.rs
+++ b/servo/components/style/font_metrics.rs
@@ -28,37 +28,39 @@ pub enum FontMetricsQueryResult {
     /// The font is available, but we may or may not have found any font metrics
     /// for it.
     Available(Option<FontMetrics>),
     /// The font is not available.
     NotAvailable,
 }
 
 /// A trait used to represent something capable of providing us font metrics.
-pub trait FontMetricsProvider: Send + fmt::Debug {
+pub trait FontMetricsProvider: fmt::Debug {
     /// Obtain the metrics for given font family.
     ///
     /// TODO: We could make this take the full list, I guess, and save a few
     /// virtual calls in the case we are repeatedly unable to find font metrics?
     /// That is not too common in practice though.
     fn query(&self, _font_name: &Atom) -> FontMetricsQueryResult {
         FontMetricsQueryResult::NotAvailable
     }
 
     /// Get default size of a given language and generic family
-    fn get_size(&self, _font_name: &Atom, _font_family: u8) -> Au {
-        unimplemented!()
-    }
+    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au;
 
     /// Construct from a shared style context
     fn create_from(context: &SharedStyleContext) -> Self where Self: Sized;
 }
 
 #[derive(Debug)]
 /// Dummy font metrics provider, for use by Servo
 /// and in cases where gecko doesn't need font metrics
 pub struct DummyProvider;
 
 impl FontMetricsProvider for DummyProvider {
     fn create_from(_: &SharedStyleContext) -> Self {
         DummyProvider
     }
+
+    fn get_size(&self, _font_name: &Atom, _font_family: u8) -> Au {
+        unreachable!("Dummy provider should never be used to compute font size")
+    }
 }
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -9,23 +9,25 @@
 //!
 //! This really follows the Servo pattern in
 //! `components/script/layout_wrapper.rs`.
 //!
 //! This theoretically should live in its own crate, but now it lives in the
 //! style system it's kind of pointless in the Stylo case, and only Servo forces
 //! the separation between the style system implementation and everything else.
 
+use app_units::Au;
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, UpdateAnimationsTasks};
 use data::ElementData;
 use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthetizer};
 use element_state::ElementState;
 use error_reporting::StdoutErrorReporter;
+use font_metrics::FontMetricsProvider;
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{SelectorImpl, NonTSPseudoClass, PseudoElement};
 use gecko::snapshot_helpers;
 use gecko_bindings::bindings;
 use gecko_bindings::bindings::{Gecko_DropStyleChildrenIterator, Gecko_MaybeCreateStyleChildrenIterator};
 use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetLastChild, Gecko_GetNextStyleChild};
 use gecko_bindings::bindings::{Gecko_IsRootElement, Gecko_MatchesElement, Gecko_Namespace};
 use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
@@ -426,32 +428,60 @@ fn get_animation_rule(element: &GeckoEle
         None
     }
 }
 
 #[derive(Debug)]
 /// Gecko font metrics provider
 pub struct FontMetrics {
     /// Cache of font sizes
-    pub font_size_cache: RefCell<Vec<(Atom, ::gecko_bindings::structs::FontSizePrefs)>>,
+    pub font_size_cache: RefCell<Vec<(Atom, structs::FontSizePrefs)>>,
 }
 
 impl FontMetrics {
     /// Construct
     pub fn new() -> Self {
         FontMetrics {
             font_size_cache: RefCell::new(Vec::new()),
         }
     }
 }
 
-impl ::font_metrics::FontMetricsProvider for FontMetrics {
-    fn create_from(_: &SharedStyleContext) -> Self {
+impl FontMetricsProvider for FontMetrics {
+    fn create_from(_: &SharedStyleContext) -> FontMetrics {
         FontMetrics::new()
     }
+
+    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au {
+        use gecko_bindings::bindings::Gecko_GetBaseSize;
+        let mut cache = self.font_size_cache.borrow_mut();
+        if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) {
+            return sizes.1.size_for_generic(font_family);
+        }
+        let sizes = unsafe {
+            Gecko_GetBaseSize(font_name.as_ptr())
+        };
+        cache.push((font_name.clone(), sizes));
+        sizes.size_for_generic(font_family)
+    }
+}
+
+impl structs::FontSizePrefs {
+    fn size_for_generic(&self, font_family: u8) -> Au {
+        Au(match font_family {
+            structs::kPresContext_DefaultVariableFont_ID => self.mDefaultVariableSize,
+            structs::kPresContext_DefaultFixedFont_ID => self.mDefaultFixedSize,
+            structs::kGenericFont_serif => self.mDefaultSerifSize,
+            structs::kGenericFont_sans_serif => self.mDefaultSansSerifSize,
+            structs::kGenericFont_monospace => self.mDefaultMonospaceSize,
+            structs::kGenericFont_cursive => self.mDefaultCursiveSize,
+            structs::kGenericFont_fantasy => self.mDefaultFantasySize,
+            x => unreachable!("Unknown generic ID {}", x),
+        })
+    }
 }
 
 impl<'le> TElement for GeckoElement<'le> {
     type ConcreteNode = GeckoNode<'le>;
     type FontMetrics = FontMetrics;
 
     fn as_node(&self) -> Self::ConcreteNode {
         unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) }
--- a/servo/components/style/gecko_bindings/bindings.rs
+++ b/servo/components/style/gecko_bindings/bindings.rs
@@ -1017,19 +1017,17 @@ extern "C" {
     pub fn Gecko_nsStyleFont_SetLang(font: *mut nsStyleFont,
                                      atom: *mut nsIAtom);
 }
 extern "C" {
     pub fn Gecko_nsStyleFont_CopyLangFrom(aFont: *mut nsStyleFont,
                                           aSource: *const nsStyleFont);
 }
 extern "C" {
-    pub fn Gecko_GetBaseSize(lang: *mut nsIAtom,
-                             pres_context: RawGeckoPresContextBorrowed)
-     -> FontSizePrefs;
+    pub fn Gecko_GetBaseSize(lang: *mut nsIAtom) -> FontSizePrefs;
 }
 extern "C" {
     pub fn Gecko_GetMediaFeatures() -> *const nsMediaFeature;
 }
 extern "C" {
     pub fn Gecko_CSSFontFaceRule_Create() -> *mut nsCSSFontFaceRule;
 }
 extern "C" {
--- a/servo/components/style/gecko_bindings/structs_debug.rs
+++ b/servo/components/style/gecko_bindings/structs_debug.rs
@@ -22614,16 +22614,18 @@ pub mod root {
     #[derive(Debug, Copy, Clone)]
     pub struct nsAnimationManager([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct nsDeviceContext([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct gfxMissingFontRecorder([u8; 0]);
+    pub const kPresContext_DefaultVariableFont_ID: u8 = 0;
+    pub const kPresContext_DefaultFixedFont_ID: u8 = 1;
     #[repr(C)]
     #[derive(Debug)]
     pub struct nsRootPresContext {
         pub _base: root::nsPresContext,
         pub mNotifyDidPaintTimers: [u64; 10usize],
         pub mApplyPluginGeometryTimer: root::nsCOMPtr<root::nsITimer>,
         pub mRegisteredPlugins: [u64; 6usize],
         pub mWillPaintObservers: root::nsTArray<root::nsCOMPtr<root::nsIRunnable>>,
--- a/servo/components/style/gecko_bindings/structs_release.rs
+++ b/servo/components/style/gecko_bindings/structs_release.rs
@@ -22034,16 +22034,18 @@ pub mod root {
     #[derive(Debug, Copy, Clone)]
     pub struct nsAnimationManager([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct nsDeviceContext([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct gfxMissingFontRecorder([u8; 0]);
+    pub const kPresContext_DefaultVariableFont_ID: u8 = 0;
+    pub const kPresContext_DefaultFixedFont_ID: u8 = 1;
     #[repr(C)]
     #[derive(Debug)]
     pub struct nsRootPresContext {
         pub _base: root::nsPresContext,
         pub mNotifyDidPaintTimers: [u64; 10usize],
         pub mApplyPluginGeometryTimer: root::nsCOMPtr<root::nsITimer>,
         pub mRegisteredPlugins: [u64; 5usize],
         pub mWillPaintObservers: root::nsTArray<root::nsCOMPtr<root::nsIRunnable>>,
--- a/servo/components/style/gecko_string_cache/mod.rs
+++ b/servo/components/style/gecko_string_cache/mod.rs
@@ -174,20 +174,21 @@ impl fmt::Display for WeakAtom {
             try!(w.write_char(c.unwrap_or(char::REPLACEMENT_CHARACTER)))
         }
         Ok(())
     }
 }
 
 impl Atom {
     /// Execute a callback with the atom represented by `ptr`.
-    pub unsafe fn with<F>(ptr: *mut nsIAtom, callback: &mut F) where F: FnMut(&Atom) {
+    pub unsafe fn with<F, R: 'static>(ptr: *mut nsIAtom, callback: &mut F) -> R where F: FnMut(&Atom) -> R {
         let atom = Atom(WeakAtom::new(ptr));
-        callback(&atom);
+        let ret = callback(&atom);
         mem::forget(atom);
+        ret
     }
 
     /// Creates an atom from an static atom pointer without checking in release
     /// builds.
     ///
     /// Right now it's only used by the atom macro, and ideally it should keep
     /// that way, now we have sugar for is_static, creating atoms using
     /// Atom::from should involve almost no overhead.
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -557,28 +557,20 @@
                     [9,   10,    13,    15,    18,    23,    30,    45],
                     [9,   10,    13,    16,    18,    24,    32,    48]
                 ];
 
                 static FONT_SIZE_FACTORS: [i32; 8] = [60, 75, 89, 100, 120, 150, 200, 300];
 
                 // XXXManishearth handle quirks mode
 
-                let base_sizes = unsafe {
-                    Gecko_GetBaseSize(cx.style().get_font().gecko().mLanguage.raw(),
-                                      &*cx.device.pres_context)
-                };
-                let base_size = match cx.style().get_font().gecko().mGenericID {
-                    structs::kGenericFont_serif => base_sizes.mDefaultSerifSize,
-                    structs::kGenericFont_sans_serif => base_sizes.mDefaultSansSerifSize,
-                    structs::kGenericFont_monospace => base_sizes.mDefaultMonospaceSize,
-                    structs::kGenericFont_cursive => base_sizes.mDefaultCursiveSize,
-                    structs::kGenericFont_fantasy => base_sizes.mDefaultFantasySize,
-                    x => unreachable!("Unknown generic ID {}", x),
-                };
+                let ref gecko_font = cx.style().get_font().gecko();
+                let base_size = unsafe { Atom::with(gecko_font.mLanguage.raw(), &mut |atom| {
+                    cx.font_metrics_provider.get_size(atom, gecko_font.mGenericID).0
+                }) };
 
                 let base_size_px = au_to_int_px(base_size as f32);
                 let html_size = *self as usize;
                 if base_size_px >= 9 && base_size_px <= 16 {
                     Au::from_px(FONT_SIZE_MAPPING[(base_size_px - 9) as usize][html_size])
                 } else {
                     Au(FONT_SIZE_FACTORS[html_size] * base_size / 100)
                 }