Auto merge of #17041 - Manishearth:root_font_size, r=emilio

Move root_font_size to the device

ComputedValues is heap allocated, and the number of CVs grows with the number of elements, so we should keep it small. The root_font_size is the same for the document, so we should stash it on the device and use that instead.

Gecko actually just manually walks up the tree here, but we can't do that. A simple AtomicRefcell should be enough.

Nothing else can be styled in parallel with the root so the refcell should never panic.

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

6 files changed, 105 insertions, 53 deletions

diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs
index cdf79c27079..7ba4959eeb5 100644
--- a/components/style/gecko/media_queries.rs
+++ b/components/style/gecko/media_queries.rs
@@ -17,7 +17,9 @@ use gecko_bindings::structs::RawGeckoPresContextOwned;
 use media_queries::MediaType;
 use parser::ParserContext;
 use properties::{ComputedValues, StyleBuilder};
+use properties::longhands::font_size;
 use std::fmt::{self, Write};
+use std::sync::atomic::{AtomicIsize, Ordering};
 use str::starts_with_ignore_ascii_case;
 use string_cache::Atom;
 use style_traits::ToCss;
@@ -35,8 +37,20 @@ pub struct Device {
     pub pres_context: RawGeckoPresContextOwned,
     default_values: Arc<ComputedValues>,
     viewport_override: Option<ViewportConstraints>,
+    /// The font size of the root element
+    /// This is set when computing the style of the root
+    /// element, and used for rem units in other elements.
+    ///
+    /// When computing the style of the root element, there can't be any
+    /// other style being computed at the same time, given we need the style of
+    /// the parent to compute everything else. So it is correct to just use
+    /// a relaxed atomic here.
+    root_font_size: AtomicIsize,
 }
 
+unsafe impl Sync for Device {}
+unsafe impl Send for Device {}
+
 impl Device {
     /// Trivially constructs a new `Device`.
     pub fn new(pres_context: RawGeckoPresContextOwned) -> Self {
@@ -45,6 +59,7 @@ impl Device {
             pres_context: pres_context,
             default_values: ComputedValues::default_values(unsafe { &*pres_context }),
             viewport_override: None,
+            root_font_size: AtomicIsize::new(font_size::get_initial_value().0 as isize), // FIXME(bz): Seems dubious?
         }
     }
 
@@ -73,6 +88,16 @@ impl Device {
         &self.default_values
     }
 
+    /// Get the font size of the root element (for rem)
+    pub fn root_font_size(&self) -> Au {
+        Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
+    }
+
+    /// Set the font size of the root element (for rem)
+    pub fn set_root_font_size(&self, size: Au) {
+        self.root_font_size.store(size.0 as isize, Ordering::Relaxed)
+    }
+
     /// Recreates all the temporary state that the `Device` stores.
     ///
     /// This includes the viewport override from `@viewport` rules, and also the
@@ -111,9 +136,6 @@ impl Device {
     }
 }
 
-unsafe impl Sync for Device {}
-unsafe impl Send for Device {}
-
 /// A expression for gecko contains a reference to the media feature, the value
 /// the media query contained, and the range to evaluate.
 #[derive(Debug, Clone)]
diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index fd4975826d7..bf55c66f90e 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -57,6 +57,7 @@ use logical_geometry::WritingMode;
 use media_queries::Device;
 use properties::animated_properties::TransitionProperty;
 use properties::longhands;
+use properties:: FontComputationData;
 use properties::{Importance, LonghandId};
 use properties::{PropertyDeclaration, PropertyDeclarationBlock, PropertyDeclarationId};
 use std::fmt::{self, Debug};
@@ -74,6 +75,7 @@ pub mod style_structs {
     % endfor
 }
 
+
 #[derive(Clone, Debug)]
 pub struct ComputedValues {
     % for style_struct in data.style_structs:
@@ -82,24 +84,7 @@ pub struct ComputedValues {
 
     custom_properties: Option<Arc<ComputedValuesMap>>,
     pub writing_mode: WritingMode,
-    pub root_font_size: Au,
-    /// font-size keyword values (and font-size-relative values applied
-    /// to keyword values) need to preserve their identity as originating
-    /// from keywords and relative font sizes. We store this information
-    /// out of band in the ComputedValues. When None, the font size on the
-    /// current struct was computed from a value that was not a keyword
-    /// or a chain of font-size-relative values applying to successive parents
-    /// terminated by a keyword. When Some, this means the font-size was derived
-    /// from a keyword value or a keyword value on some ancestor with only
-    /// font-size-relative keywords and regular inheritance in between. The
-    /// integer stores the final ratio of the chain of font size relative values.
-    /// and is 1 when there was just a keyword and no relative values.
-    ///
-    /// When this is Some, we compute font sizes by computing the keyword against
-    /// the generic font, and then multiplying it by the ratio.
-    pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
-    /// The cached system font. See longhand/font.mako.rs
-    pub cached_system_font: Option<longhands::system_font::ComputedSystemFont>,
+    pub font_computation_data: FontComputationData,
 
     /// The element's computed values if visited, only computed if there's a
     /// relevant link for this element. A element's "relevant link" is the
@@ -110,7 +95,6 @@ pub struct ComputedValues {
 impl ComputedValues {
     pub fn new(custom_properties: Option<Arc<ComputedValuesMap>>,
                writing_mode: WritingMode,
-               root_font_size: Au,
                font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
                visited_style: Option<Arc<ComputedValues>>,
                % for style_struct in data.style_structs:
@@ -120,9 +104,7 @@ impl ComputedValues {
         ComputedValues {
             custom_properties: custom_properties,
             writing_mode: writing_mode,
-            root_font_size: root_font_size,
-            cached_system_font: None,
-            font_size_keyword: font_size_keyword,
+            font_computation_data: FontComputationData::new(font_size_keyword),
             visited_style: visited_style,
             % for style_struct in data.style_structs:
             ${style_struct.ident}: ${style_struct.ident},
@@ -134,9 +116,7 @@ impl ComputedValues {
         Arc::new(ComputedValues {
             custom_properties: None,
             writing_mode: WritingMode::empty(), // FIXME(bz): This seems dubious
-            root_font_size: longhands::font_size::get_initial_value(), // FIXME(bz): Also seems dubious?
-            font_size_keyword: Some((Default::default(), 1.)),
-            cached_system_font: None,
+            font_computation_data: FontComputationData::default_values(),
             visited_style: None,
             % for style_struct in data.style_structs:
                 ${style_struct.ident}: style_structs::${style_struct.name}::default(pres_context),
@@ -144,6 +124,7 @@ impl ComputedValues {
         })
     }
 
+
     #[inline]
     pub fn is_display_contents(&self) -> bool {
         self.get_box().clone_display() == longhands::display::computed_value::T::contents
diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs
index bbeb892d1a9..110ae7d6800 100644
--- a/components/style/properties/longhand/font.mako.rs
+++ b/components/style/properties/longhand/font.mako.rs
@@ -900,7 +900,7 @@ ${helpers.single_keyword_system("font-variant-caps",
             // recomputed from the base size for the keyword and the relative size.
             //
             // See bug 1355707
-            if let Some((kw, fraction)) = context.inherited_style().font_size_keyword {
+            if let Some((kw, fraction)) = context.inherited_style().font_computation_data.font_size_keyword {
                 context.mutate_style().font_size_keyword = Some((kw, fraction * ratio));
             } else {
                 context.mutate_style().font_size_keyword = None;
@@ -950,7 +950,7 @@ ${helpers.single_keyword_system("font-variant-caps",
                .inherit_font_size_from(parent, kw_inherited_size);
         if used_kw {
             context.mutate_style().font_size_keyword =
-                context.inherited_style.font_size_keyword;
+                context.inherited_style.font_computation_data.font_size_keyword;
         } else {
             context.mutate_style().font_size_keyword = None;
         }
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 92429d554dd..5036055d379 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -65,6 +65,44 @@ pub trait MaybeBoxed<Out> {
     fn maybe_boxed(self) -> Out;
 }
 
+
+/// This is where we store extra font data while
+/// while computing font sizes.
+#[derive(Clone, Debug)]
+pub struct FontComputationData {
+    /// font-size keyword values (and font-size-relative values applied
+    /// to keyword values) need to preserve their identity as originating
+    /// from keywords and relative font sizes. We store this information
+    /// out of band in the ComputedValues. When None, the font size on the
+    /// current struct was computed from a value that was not a keyword
+    /// or a chain of font-size-relative values applying to successive parents
+    /// terminated by a keyword. When Some, this means the font-size was derived
+    /// from a keyword value or a keyword value on some ancestor with only
+    /// font-size-relative keywords and regular inheritance in between. The
+    /// integer stores the final ratio of the chain of font size relative values.
+    /// and is 1 when there was just a keyword and no relative values.
+    ///
+    /// When this is Some, we compute font sizes by computing the keyword against
+    /// the generic font, and then multiplying it by the ratio.
+   pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>
+}
+
+
+impl FontComputationData{
+        /// Assigns values for variables in struct FontComputationData
+    pub fn new(font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>) -> Self {
+        FontComputationData {
+            font_size_keyword: font_size_keyword
+        }
+    }
+        /// Assigns default values for variables in struct FontComputationData
+   pub fn default_values() -> Self {
+        FontComputationData{
+            font_size_keyword: Some((Default::default(), 1.))
+        }
+    }
+}
+
 impl<T> MaybeBoxed<T> for T {
     #[inline]
     fn maybe_boxed(self) -> T { self }
@@ -1790,10 +1828,8 @@ pub struct ComputedValues {
     custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
     /// The writing mode of this computed values struct.
     pub writing_mode: WritingMode,
-    /// The root element's computed font size.
-    pub root_font_size: Au,
     /// The keyword behind the current font-size property, if any
-    pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
+    pub font_computation_data: FontComputationData,
 
     /// The element's computed values if visited, only computed if there's a
     /// relevant link for this element. A element's "relevant link" is the
@@ -1806,7 +1842,6 @@ impl ComputedValues {
     /// Construct a `ComputedValues` instance.
     pub fn new(custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
                writing_mode: WritingMode,
-               root_font_size: Au,
                font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
                visited_style: Option<Arc<ComputedValues>>,
             % for style_struct in data.active_style_structs():
@@ -1816,8 +1851,7 @@ impl ComputedValues {
         ComputedValues {
             custom_properties: custom_properties,
             writing_mode: writing_mode,
-            root_font_size: root_font_size,
-            font_size_keyword: font_size_keyword,
+            font_computation_data: FontComputationData::new(font_size_keyword),
             visited_style: visited_style,
         % for style_struct in data.active_style_structs():
             ${style_struct.ident}: ${style_struct.ident},
@@ -2254,8 +2288,6 @@ pub struct StyleBuilder<'a> {
     ///
     /// TODO(emilio): Make private.
     pub writing_mode: WritingMode,
-    /// The font size of the root element.
-    pub root_font_size: Au,
     /// The keyword behind the current font-size property, if any.
     pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
     /// The element's style if visited, only computed if there's a relevant link
@@ -2272,7 +2304,6 @@ impl<'a> StyleBuilder<'a> {
     pub fn new(
         custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
         writing_mode: WritingMode,
-        root_font_size: Au,
         font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
         visited_style: Option<Arc<ComputedValues>>,
         % for style_struct in data.active_style_structs():
@@ -2282,7 +2313,6 @@ impl<'a> StyleBuilder<'a> {
         StyleBuilder {
             custom_properties: custom_properties,
             writing_mode: writing_mode,
-            root_font_size: root_font_size,
             font_size_keyword: font_size_keyword,
             visited_style: visited_style,
         % for style_struct in data.active_style_structs():
@@ -2302,8 +2332,7 @@ impl<'a> StyleBuilder<'a> {
     pub fn for_inheritance(parent: &'a ComputedValues, default: &'a ComputedValues) -> Self {
         Self::new(parent.custom_properties(),
                   parent.writing_mode,
-                  parent.root_font_size,
-                  parent.font_size_keyword,
+                  parent.font_computation_data.font_size_keyword,
                   parent.clone_visited_style(),
                   % for style_struct in data.active_style_structs():
                   % if style_struct.inherited:
@@ -2376,7 +2405,6 @@ impl<'a> StyleBuilder<'a> {
     pub fn build(self) -> ComputedValues {
         ComputedValues::new(self.custom_properties,
                             self.writing_mode,
-                            self.root_font_size,
                             self.font_size_keyword,
                             self.visited_style,
                             % for style_struct in data.active_style_structs():
@@ -2403,7 +2431,7 @@ pub use self::lazy_static_module::INITIAL_SERVO_VALUES;
 mod lazy_static_module {
     use logical_geometry::WritingMode;
     use stylearc::Arc;
-    use super::{ComputedValues, longhands, style_structs};
+    use super::{ComputedValues, longhands, style_structs, FontComputationData};
 
     /// The initial values for all style structs as defined by the specification.
     lazy_static! {
@@ -2420,8 +2448,7 @@ mod lazy_static_module {
             % endfor
             custom_properties: None,
             writing_mode: WritingMode::empty(),
-            root_font_size: longhands::font_size::get_initial_value(),
-            font_size_keyword: Some((Default::default(), 1.)),
+            font_computation_data: FontComputationData::default_values(),
             visited_style: None,
         };
     }
@@ -2570,8 +2597,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
     let builder = if !flags.contains(INHERIT_ALL) {
         StyleBuilder::new(custom_properties,
                           WritingMode::empty(),
-                          inherited_style.root_font_size,
-                          inherited_style.font_size_keyword,
+                          inherited_style.font_computation_data.font_size_keyword,
                           visited_style,
                           % for style_struct in data.active_style_structs():
                               % if style_struct.inherited:
@@ -2584,8 +2610,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
     } else {
         StyleBuilder::new(custom_properties,
                           WritingMode::empty(),
-                          inherited_style.root_font_size,
-                          inherited_style.font_size_keyword,
+                          inherited_style.font_computation_data.font_size_keyword,
                           visited_style,
                           % for style_struct in data.active_style_structs():
                               inherited_style.${style_struct.name_lower}_arc(),
@@ -2750,7 +2775,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
 
             if is_root_element {
                 let s = context.style.get_font().clone_font_size();
-                context.style.root_font_size = s;
+                context.device.set_root_font_size(s);
             }
         % endif
     % endfor
diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs
index 299970405db..2aaadfcd32b 100644
--- a/components/style/servo/media_queries.rs
+++ b/components/style/servo/media_queries.rs
@@ -12,7 +12,9 @@ use font_metrics::ServoMetricsProvider;
 use media_queries::MediaType;
 use parser::ParserContext;
 use properties::{ComputedValues, StyleBuilder};
+use properties::longhands::font_size;
 use std::fmt;
+use std::sync::atomic::{AtomicIsize, Ordering};
 use style_traits::{CSSPixel, ToCss};
 use style_traits::viewport::ViewportConstraints;
 use values::computed::{self, ToComputedValue};
@@ -22,12 +24,23 @@ use values::specified;
 /// is displayed in.
 ///
 /// This is the struct against which media queries are evaluated.
-#[derive(Debug, HeapSizeOf)]
+#[derive(HeapSizeOf)]
 pub struct Device {
     /// The current media type used by de device.
     media_type: MediaType,
     /// The current viewport size, in CSS pixels.
     viewport_size: TypedSize2D<f32, CSSPixel>,
+
+    /// The font size of the root element
+    /// This is set when computing the style of the root
+    /// element, and used for rem units in other elements
+    ///
+    /// When computing the style of the root element, there can't be any
+    /// other style being computed at the same time, given we need the style of
+    /// the parent to compute everything else. So it is correct to just use
+    /// a relaxed atomic here.
+    #[ignore_heap_size_of = "Pure stack type"]
+    root_font_size: AtomicIsize,
 }
 
 impl Device {
@@ -38,6 +51,7 @@ impl Device {
         Device {
             media_type: media_type,
             viewport_size: viewport_size,
+            root_font_size: AtomicIsize::new(font_size::get_initial_value().0 as isize), // FIXME(bz): Seems dubious?
         }
     }
 
@@ -49,6 +63,16 @@ impl Device {
         ComputedValues::initial_values()
     }
 
+    /// Get the font size of the root element (for rem)
+    pub fn root_font_size(&self) -> Au {
+        Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
+    }
+
+    /// Set the font size of the root element (for rem)
+    pub fn set_root_font_size(&self, size: Au) {
+        self.root_font_size.store(size.0 as isize, Ordering::Relaxed)
+    }
+
     /// Returns the viewport size of the current device in app units, needed,
     /// among other things, to resolve viewport units.
     #[inline]
diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs
index ef4083544ae..4c3adfc4d63 100644
--- a/components/style/values/specified/length.rs
+++ b/components/style/values/specified/length.rs
@@ -112,7 +112,7 @@ impl FontRelativeLength {
 
         let reference_font_size = base_size.resolve(context);
 
-        let root_font_size = context.style().root_font_size;
+        let root_font_size = context.device.root_font_size();
         match *self {
             FontRelativeLength::Em(length) => reference_font_size.scale_by(length),
             FontRelativeLength::Ex(length) => {