Bug 1399228 - stylo: Clean up keyword values; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Mon, 18 Sep 2017 14:12:10 -0700
changeset 669460 1c6d0ec22002ed59b168060aa6b712191fccfe07
parent 669459 79029a15ed190e253dda0c9f2716cf643c11df6e
child 732961 49678093c1ac674a98bb318fc4a49d3768e3298c
push id81335
push userbmo:manishearth@gmail.com
push dateSat, 23 Sep 2017 08:49:12 +0000
reviewersemilio
bugs1399228
milestone58.0a1
Bug 1399228 - stylo: Clean up keyword values; r?emilio MozReview-Commit-ID: HnNgeH8FtpR
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/values/computed/font.rs
servo/components/style/values/specified/font.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2159,17 +2159,17 @@ fn static_assert() {
         self.gecko.mScriptUnconstrainedSize = device.unzoom_text(Au(self.gecko.mScriptUnconstrainedSize)).0;
         self.gecko.mFont.size = device.unzoom_text(Au(self.gecko.mFont.size)).0;
     }
 
     pub fn set_font_size(&mut self, v: longhands::font_size::computed_value::T) {
         use values::specified::font::KeywordSize;
         self.gecko.mSize = v.size().0;
         self.gecko.mScriptUnconstrainedSize = v.size().0;
-        if let Some(info) = v.info {
+        if let Some(info) = v.keyword_info {
             self.gecko.mFontSizeKeyword = match info.kw {
                 KeywordSize::XXSmall => structs::NS_STYLE_FONT_SIZE_XXSMALL,
                 KeywordSize::XSmall => structs::NS_STYLE_FONT_SIZE_XSMALL,
                 KeywordSize::Small => structs::NS_STYLE_FONT_SIZE_SMALL,
                 KeywordSize::Medium => structs::NS_STYLE_FONT_SIZE_MEDIUM,
                 KeywordSize::Large => structs::NS_STYLE_FONT_SIZE_LARGE,
                 KeywordSize::XLarge => structs::NS_STYLE_FONT_SIZE_XLARGE,
                 KeywordSize::XXLarge => structs::NS_STYLE_FONT_SIZE_XXLARGE,
@@ -2380,24 +2380,24 @@ fn static_assert() {
             structs::NS_STYLE_FONT_SIZE_MEDIUM => KeywordSize::Medium,
             structs::NS_STYLE_FONT_SIZE_LARGE => KeywordSize::Large,
             structs::NS_STYLE_FONT_SIZE_XLARGE => KeywordSize::XLarge,
             structs::NS_STYLE_FONT_SIZE_XXLARGE => KeywordSize::XXLarge,
             structs::NS_STYLE_FONT_SIZE_XXXLARGE => KeywordSize::XXXLarge,
             structs::NS_STYLE_FONT_SIZE_NO_KEYWORD => {
                 return longhands::font_size::computed_value::T {
                     size: size,
-                    info: None,
+                    keyword_info: None,
                 }
             }
             _ => unreachable!("mFontSizeKeyword should be an absolute keyword or NO_KEYWORD")
         };
         longhands::font_size::computed_value::T {
             size: size,
-            info: Some(KeywordInfo {
+            keyword_info: Some(KeywordInfo {
                 kw: kw,
                 factor: self.gecko.mFontSizeFactor,
                 offset: Au(self.gecko.mFontSizeOffset).into()
             })
         }
     }
 
     pub fn set_font_weight(&mut self, v: longhands::font_weight::computed_value::T) {
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -598,40 +598,37 @@ macro_rules! impl_gecko_keyword_conversi
 
 <%helpers:longhand name="font-size" animation_value_type="ComputedValue"
                    flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER"
                    allow_quirks="True" spec="https://drafts.csswg.org/css-fonts/#propdef-font-size">
     use app_units::Au;
     use values::specified::AllowQuirks;
     use values::specified::length::FontBaseSize;
     use values::specified::font::{FONT_MEDIUM_PX, KeywordSize};
+    use values::computed::font::{KeywordInfo};
 
     pub mod computed_value {
         use values::computed::font;
         pub type T = font::FontSize;
     }
 
     pub use values::specified::font::FontSize as SpecifiedValue;
 
     #[inline]
     #[allow(missing_docs)]
     pub fn get_initial_value() -> computed_value::T {
         computed_value::T {
             size: Au::from_px(FONT_MEDIUM_PX).into(),
-            info: Some(::values::computed::font::KeywordInfo {
-                kw: KeywordSize::Medium,
-                factor: 1.,
-                offset: Au(0).into(),
-            })
+            keyword_info: Some(KeywordInfo::medium())
         }
     }
 
     #[inline]
     pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue::Keyword(KeywordSize::Medium, 1., Au(0).into())
+        SpecifiedValue::Keyword(KeywordInfo::medium())
     }
 
 
     /// <length> | <percentage> | <absolute-size> | <relative-size>
     pub fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                          -> Result<SpecifiedValue, ParseError<'i>> {
         parse_quirky(context, input, AllowQuirks::No)
     }
@@ -642,17 +639,17 @@ macro_rules! impl_gecko_keyword_conversi
                                 allow_quirks: AllowQuirks)
                                 -> Result<SpecifiedValue, ParseError<'i>> {
         use self::specified::LengthOrPercentage;
         if let Ok(lop) = input.try(|i| LengthOrPercentage::parse_non_negative_quirky(context, i, allow_quirks)) {
             return Ok(SpecifiedValue::Length(lop))
         }
 
         if let Ok(kw) = input.try(KeywordSize::parse) {
-            return Ok(SpecifiedValue::Keyword(kw, 1., Au(0).into()))
+            return Ok(SpecifiedValue::Keyword(kw.into()))
         }
 
         try_match_ident_ignore_ascii_case! { input.expect_ident()?,
             "smaller" => Ok(SpecifiedValue::Smaller),
             "larger" => Ok(SpecifiedValue::Larger),
         }
     }
 
@@ -665,17 +662,17 @@ macro_rules! impl_gecko_keyword_conversi
         % if product == "gecko":
             use gecko_bindings::structs::nsIAtom;
             // if the language or generic changed, we need to recalculate
             // the font size from the stored font-size origin information.
             if context.builder.get_font().gecko().mLanguage.raw::<nsIAtom>() !=
                context.builder.get_parent_font().gecko().mLanguage.raw::<nsIAtom>() ||
                context.builder.get_font().gecko().mGenericID !=
                context.builder.get_parent_font().gecko().mGenericID {
-                if let Some(info) = computed.info {
+                if let Some(info) = computed.keyword_info {
                     computed.size = context.maybe_zoom_text(info.kw.to_computed_value(context)
                                                                 .scale_by(info.factor) + info.offset)
                 }
             }
         % endif
 
         let device = context.builder.device;
         let mut font = context.builder.take_font();
@@ -698,18 +695,18 @@ macro_rules! impl_gecko_keyword_conversi
     /// FIXME(emilio): This is very complex. Also, it should move to
     /// StyleBuilder.
     pub fn cascade_inherit_font_size(context: &mut Context) {
         // If inheriting, we must recompute font-size in case of language
         // changes using the font_size_keyword. We also need to do this to
         // handle mathml scriptlevel changes
         let kw_inherited_size = context.builder.get_parent_font()
                                        .clone_font_size()
-                                       .info.map(|info| {
-            context.maybe_zoom_text(SpecifiedValue::Keyword(info.kw, info.factor, info.offset)
+                                       .keyword_info.map(|info| {
+            context.maybe_zoom_text(SpecifiedValue::Keyword(info)
                   .to_computed_value(context).size)
         });
         let mut font = context.builder.take_font();
         font.inherit_font_size_from(context.builder.get_parent_font(),
                                     kw_inherited_size,
                                     context.builder.device);
         context.builder.put_font(font);
     }
@@ -2200,17 +2197,20 @@ https://drafts.csswg.org/css-fonts-4/#lo
                     FontFamily::FamilyName(FamilyName {
                         name: (&*font.mName).into(),
                         syntax: FamilyNameSyntax::Quoted,
                     })
                 }).collect::<Vec<_>>();
                 let weight = longhands::font_weight::computed_value::T::from_gecko_weight(system.weight);
                 let ret = ComputedSystemFont {
                     font_family: longhands::font_family::computed_value::T(family),
-                    font_size: longhands::font_size::computed_value::T { size: Au(system.size).into(), info: None },
+                    font_size: longhands::font_size::computed_value::T {
+                            size: Au(system.size).into(),
+                            keyword_info: None
+                    },
                     font_weight: weight,
                     font_size_adjust: longhands::font_size_adjust::computed_value
                                                ::T::from_gecko_adjust(system.sizeAdjust),
                     % for kwprop in kw_font_props:
                         ${kwprop}: longhands::${kwprop}::computed_value::T::from_gecko_keyword(
                             system.${to_camel_case_lower(kwprop.replace('font_', ''))}
                             % if kwprop in kw_cast:
                                 as u32
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@@ -14,17 +14,17 @@ use values::specified::font as specified
 #[derive(ToAnimatedValue, Animate, ToAnimatedZero, ComputeSquaredDistance)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 /// The computed value of font-size
 pub struct FontSize {
     /// The size.
     pub size: NonNegativeLength,
     /// If derived from a keyword, the keyword and additional transformations applied to it
-    pub info: Option<KeywordInfo>,
+    pub keyword_info: Option<KeywordInfo>,
 }
 
 #[derive(Copy, Clone, PartialEq, Debug)]
 #[derive(ToAnimatedValue, Animate, ToAnimatedZero, ComputeSquaredDistance)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 /// Additional information for keyword-derived font sizes.
 pub struct KeywordInfo {
@@ -40,16 +40,31 @@ impl KeywordInfo {
     /// Given a parent keyword info (self), apply an additional factor/offset to it
     pub fn compose(self, factor: f32, offset: NonNegativeLength) -> Self {
         KeywordInfo {
             kw: self.kw,
             factor: self.factor * factor,
             offset: self.offset.scale_by(factor) + offset,
         }
     }
+
+    /// KeywordInfo value for font-size: medium
+    pub fn medium() -> Self {
+        specified::KeywordSize::Medium.into()
+    }
+}
+
+impl From<specified::KeywordSize> for KeywordInfo {
+    fn from(x: specified::KeywordSize) -> Self {
+        KeywordInfo {
+            kw: x,
+            factor: 1.,
+            offset: Au(0).into(),
+        }
+    }
 }
 
 impl FontSize {
     /// The actual computed font size.
     pub fn size(self) -> Au {
         self.size.into()
     }
 }
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -27,30 +27,30 @@ pub enum FontSize {
     /// will be 1 (with offset 0), but we cascade keywordness even
     /// after font-relative (percent and em) values
     /// have been applied, which is where the ratio
     /// comes in. The offset comes in if we cascaded a calc value,
     /// where the font-relative portion (em and percentage) will
     /// go into the ratio, and the remaining units all computed together
     /// will go into the offset.
     /// See bug 1355707.
-    Keyword(KeywordSize, f32, NonNegativeLength),
+    Keyword(computed::KeywordInfo),
     /// font-size: smaller
     Smaller,
     /// font-size: larger
     Larger,
     /// Derived from a specified system font.
     System(SystemFont)
 }
 
 impl ToCss for FontSize {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
             FontSize::Length(ref lop) => lop.to_css(dest),
-            FontSize::Keyword(kw, _, _) => kw.to_css(dest),
+            FontSize::Keyword(info) => info.kw.to_css(dest),
             FontSize::Smaller => dest.write_str("smaller"),
             FontSize::Larger => dest.write_str("larger"),
             FontSize::System(sys) => sys.to_css(dest),
         }
     }
 }
 
 impl From<LengthOrPercentage> for FontSize {
@@ -221,30 +221,30 @@ impl FontSize {
             0 | 1 => KeywordSize::XSmall,
             2 => KeywordSize::Small,
             3 => KeywordSize::Medium,
             4 => KeywordSize::Large,
             5 => KeywordSize::XLarge,
             6 => KeywordSize::XXLarge,
             // If value is greater than 7, let it be 7.
             _ => KeywordSize::XXXLarge,
-        }, 1., Au(0).into())
+        }.into())
     }
 
     /// Compute it against a given base font size
     pub fn to_computed_value_against(
         &self,
         context: &Context,
         base_size: FontBaseSize,
     ) -> computed::FontSize {
         use values::specified::length::FontRelativeLength;
 
         let compose_keyword = |factor| {
             context.style().get_parent_font()
-                   .clone_font_size().info
+                   .clone_font_size().keyword_info
                    .map(|i| i.compose(factor, Au(0).into()))
         };
         let mut info = None;
         let size = match *self {
             FontSize::Length(LengthOrPercentage::Length(
                     NoCalcLength::FontRelative(value))) => {
                 if let FontRelativeLength::Em(em) = value {
                     // If the parent font was keyword-derived, this is too.
@@ -269,39 +269,35 @@ impl FontSize {
                 // Tack the % onto the factor
                 info = compose_keyword(pc.0);
                 base_size.resolve(context).scale_by(pc.0).into()
             }
             FontSize::Length(LengthOrPercentage::Calc(ref calc)) => {
                 let parent = context.style().get_parent_font().clone_font_size();
                 // if we contain em/% units and the parent was keyword derived, this is too
                 // Extract the ratio/offset and compose it
-                if (calc.em.is_some() || calc.percentage.is_some()) && parent.info.is_some() {
+                if (calc.em.is_some() || calc.percentage.is_some()) && parent.keyword_info.is_some() {
                     let ratio = calc.em.unwrap_or(0.) + calc.percentage.map_or(0., |pc| pc.0);
                     // Compute it, but shave off the font-relative part (em, %)
                     // This will mean that other font-relative units like ex and ch will be computed against
                     // the old font even when the font changes. There's no particular "right answer" for what
                     // to do here -- Gecko recascades as if the font had changed, we instead track the changes
                     // and reapply, which means that we carry over old computed ex/ch values whilst Gecko
                     // recomputes new ones. This is enough of an edge case to not really matter.
                     let abs = calc.to_computed_value_zoomed(context, FontBaseSize::Custom(Au(0).into()))
                                   .length_component().into();
-                    info = parent.info.map(|i| i.compose(ratio, abs));
+                    info = parent.keyword_info.map(|i| i.compose(ratio, abs));
                 }
                 let calc = calc.to_computed_value_zoomed(context, base_size);
                 calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()
             }
-            FontSize::Keyword(key, fraction, offset) => {
+            FontSize::Keyword(i) => {
                 // As a specified keyword, this is keyword derived
-                info = Some(computed::KeywordInfo {
-                    kw: key,
-                    factor: fraction,
-                    offset: offset,
-                });
-                context.maybe_zoom_text(key.to_computed_value(context).scale_by(fraction) + offset)
+                info = Some(i);
+                context.maybe_zoom_text(i.kw.to_computed_value(context).scale_by(i.factor) + i.offset)
             }
             FontSize::Smaller => {
                 info = compose_keyword(1. / LARGER_FONT_SIZE_RATIO);
                 FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO)
                     .to_computed_value(context, base_size).into()
             }
             FontSize::Larger => {
                 info = compose_keyword(LARGER_FONT_SIZE_RATIO);
@@ -313,17 +309,20 @@ impl FontSize {
                 #[cfg(feature = "servo")] {
                     unreachable!()
                 }
                 #[cfg(feature = "gecko")] {
                     context.cached_system_font.as_ref().unwrap().font_size.size
                 }
             }
         };
-        computed::FontSize { size, info }
+        computed::FontSize {
+            size: size,
+            keyword_info: info,
+        }
     }
 }
 
 impl ToComputedValue for FontSize {
     type ComputedValue = computed::FontSize;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> computed::FontSize {