style: Don't clamp font-size calc() factors too early.

These two bugs (bug 1572738 and bug 1572451) are stylo regressions.

When font-family changes, we try to recompute the font-size with a length /
percentage combinations in case the generic family changes, so the user
preferences are kept.

When calc() is involved, we clamp to non-negative too early, via
NonNegativeLength::scale_by.

I think we should generally dump this "try to track font-size across calc()"
thingie, as as various comments note it is not quite perfect, and it's not clear
how it should work in presence of min()/max().

This patch fixes the issue and simplifies code a bit, I may consider removing
this altogether in a follow-up.

Differential Revision: https://phabricator.services.mozilla.com/D41776

components/style/properties/gecko.mako.rs

@@ -1146,7 +1146,7 @@ fn static_assert() {
     }
 
     pub fn set_font_size(&mut self, v: FontSize) {
-        use crate::values::generics::font::KeywordSize;
+        use crate::values::specified::font::KeywordSize;
 
         let size = v.size();
         self.gecko.mScriptUnconstrainedSize = size.0;
@@ -1167,7 +1167,7 @@ fn static_assert() {
                 KeywordSize::XXXLarge => structs::NS_STYLE_FONT_SIZE_XXXLARGE,
             } as u8;
             self.gecko.mFontSizeFactor = info.factor;
-            self.gecko.mFontSizeOffset = info.offset.0.to_i32_au();
+            self.gecko.mFontSizeOffset = info.offset.to_i32_au();
         } else {
             self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8;
             self.gecko.mFontSizeFactor = 1.;
@@ -1176,7 +1176,7 @@ fn static_assert() {
     }
 
     pub fn clone_font_size(&self) -> FontSize {
-        use crate::values::generics::font::{KeywordInfo, KeywordSize};
+        use crate::values::specified::font::{KeywordInfo, KeywordSize};
         let size = Au(self.gecko.mSize).into();
         let kw = match self.gecko.mFontSizeKeyword as u32 {
             structs::NS_STYLE_FONT_SIZE_XXSMALL => KeywordSize::XXSmall,

components/style/properties/longhands/font.mako.rs

@@ -401,7 +401,7 @@ ${helpers.predefined_type(
                         is_system_font: true,
                     },
                     font_size: FontSize {
-                        size: cx.maybe_zoom_text(Au(system.size).into()),
+                        size: NonNegative(cx.maybe_zoom_text(Au(system.size).into())),
                         keyword_info: None
                     },
                     font_weight,

components/style/values/computed/font.rs

@@ -9,11 +9,11 @@ use crate::gecko_bindings::sugar::refptr::RefPtr;
 #[cfg(feature = "gecko")]
 use crate::gecko_bindings::{bindings, structs};
 use crate::values::animated::{ToAnimatedValue, ToAnimatedZero};
-use crate::values::computed::{Angle, Context, Integer, NonNegativeLength, NonNegativePercentage};
+use crate::values::computed::{Angle, Context, Integer, Length, NonNegativeLength, NonNegativePercentage};
 use crate::values::computed::{Number, Percentage, ToComputedValue};
-use crate::values::generics::font as generics;
+use crate::values::generics::{font as generics, NonNegative};
 use crate::values::generics::font::{FeatureTagValue, FontSettings, VariationValue};
-use crate::values::specified::font::{self as specified, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT};
+use crate::values::specified::font::{self as specified, KeywordInfo, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT};
 use crate::values::specified::length::{FontBaseSize, NoCalcLength};
 use crate::values::CSSFloat;
 use crate::Atom;
@@ -88,9 +88,6 @@ pub struct FontSize {
     pub keyword_info: Option<KeywordInfo>,
 }
 
-/// Additional information for computed keyword-derived font sizes.
-pub type KeywordInfo = generics::KeywordInfo<NonNegativeLength>;
-
 impl FontWeight {
     /// Value for normal
     pub fn normal() -> Self {
@@ -162,17 +159,17 @@ impl FontSize {
 }
 
 impl ToAnimatedValue for FontSize {
-    type AnimatedValue = NonNegativeLength;
+    type AnimatedValue = Length;
 
     #[inline]
     fn to_animated_value(self) -> Self::AnimatedValue {
-        self.size
+        self.size.0
     }
 
     #[inline]
     fn from_animated_value(animated: Self::AnimatedValue) -> Self {
         FontSize {
-            size: animated.clamp(),
+            size: NonNegative(animated.clamp_to_non_negative()),
             keyword_info: None,
         }
     }

components/style/values/computed/length.rs

@@ -297,7 +297,7 @@ impl specified::CalcLengthPercentage {
     ) -> LengthPercentage {
         self.to_computed_value_with_zoom(
             context,
-            |abs| context.maybe_zoom_text(abs.into()).0,
+            |abs| context.maybe_zoom_text(abs.into()),
             base_size,
         )
     }
@@ -360,8 +360,8 @@ impl LengthPercentage {
         self.unclamped_length().px() == 0.0 && self.percentage.0 == 0.0
     }
 
-    // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we still use Au as the
-    // hash key.
+    // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we
+    // still use Au as the hash key.
     #[allow(missing_docs)]
     pub fn to_hash_key(&self) -> (Au, NotNan<f32>) {
         (
@@ -840,14 +840,6 @@ impl NonNegativeLength {
             self
         }
     }
-
-    /// Scale this NonNegativeLength.
-    /// We scale NonNegativeLength by zero if the factor is negative because it doesn't
-    /// make sense to scale a negative factor on a non-negative length.
-    #[inline]
-    pub fn scale_by(&self, factor: f32) -> Self {
-        Self::new(self.0.px() * factor.max(0.))
-    }
 }
 
 impl From<Length> for NonNegativeLength {

components/style/values/computed/mod.rs

@@ -230,7 +230,7 @@ impl<'a> Context<'a> {
 
     /// Apply text-zoom if enabled.
     #[cfg(feature = "gecko")]
-    pub fn maybe_zoom_text(&self, size: NonNegativeLength) -> NonNegativeLength {
+    pub fn maybe_zoom_text(&self, size: CSSPixelLength) -> CSSPixelLength {
         // We disable zoom for <svg:text> by unsetting the
         // -x-text-zoom property, which leads to a false value
         // in mAllowZoom
@@ -243,7 +243,7 @@ impl<'a> Context<'a> {
 
     /// (Servo doesn't do text-zoom)
     #[cfg(feature = "servo")]
-    pub fn maybe_zoom_text(&self, size: NonNegativeLength) -> NonNegativeLength {
+    pub fn maybe_zoom_text(&self, size: CSSPixelLength) -> CSSPixelLength {
         size
     }
 }

components/style/values/generics/font.rs

@@ -5,14 +5,13 @@
 //! Generic types for font stuff.
 
 use crate::parser::{Parse, ParserContext};
-use app_units::Au;
 use byteorder::{BigEndian, ReadBytesExt};
 use cssparser::Parser;
 use num_traits::One;
 use std::fmt::{self, Write};
 use std::io::Cursor;
-use style_traits::{CssWriter, KeywordsCollectFn, ParseError};
-use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss};
+use style_traits::{CssWriter, ParseError};
+use style_traits::{StyleParseErrorKind, ToCss};
 
 /// https://drafts.csswg.org/css-fonts-4/#feature-tag-value
 #[derive(
@@ -172,104 +171,6 @@ impl Parse for FontTag {
     }
 }
 
-#[derive(
-    Animate,
-    Clone,
-    ComputeSquaredDistance,
-    Copy,
-    Debug,
-    MallocSizeOf,
-    PartialEq,
-    ToAnimatedValue,
-    ToAnimatedZero,
-    ToCss,
-    ToShmem,
-)]
-/// Additional information for keyword-derived font sizes.
-pub struct KeywordInfo<Length> {
-    /// The keyword used
-    pub kw: KeywordSize,
-    /// A factor to be multiplied by the computed size of the keyword
-    #[css(skip)]
-    pub factor: f32,
-    /// An additional Au offset to add to the kw*factor in the case of calcs
-    #[css(skip)]
-    pub offset: Length,
-}
-
-impl<L> KeywordInfo<L>
-where
-    Au: Into<L>,
-{
-    /// KeywordInfo value for font-size: medium
-    pub fn medium() -> Self {
-        KeywordSize::Medium.into()
-    }
-}
-
-impl<L> From<KeywordSize> for KeywordInfo<L>
-where
-    Au: Into<L>,
-{
-    fn from(x: KeywordSize) -> Self {
-        KeywordInfo {
-            kw: x,
-            factor: 1.,
-            offset: Au(0).into(),
-        }
-    }
-}
-
-impl<L> SpecifiedValueInfo for KeywordInfo<L> {
-    fn collect_completion_keywords(f: KeywordsCollectFn) {
-        <KeywordSize as SpecifiedValueInfo>::collect_completion_keywords(f);
-    }
-}
-
-/// CSS font keywords
-#[derive(
-    Animate,
-    Clone,
-    ComputeSquaredDistance,
-    Copy,
-    Debug,
-    MallocSizeOf,
-    Parse,
-    PartialEq,
-    SpecifiedValueInfo,
-    ToAnimatedValue,
-    ToAnimatedZero,
-    ToCss,
-    ToShmem,
-)]
-#[allow(missing_docs)]
-pub enum KeywordSize {
-    #[css(keyword = "xx-small")]
-    XXSmall,
-    XSmall,
-    Small,
-    Medium,
-    Large,
-    XLarge,
-    #[css(keyword = "xx-large")]
-    XXLarge,
-    #[css(keyword = "xxx-large")]
-    XXXLarge,
-}
-
-impl KeywordSize {
-    /// Convert to an HTML <font size> value
-    #[inline]
-    pub fn html_size(self) -> u8 {
-        self as u8
-    }
-}
-
-impl Default for KeywordSize {
-    fn default() -> Self {
-        KeywordSize::Medium
-    }
-}
 
 /// A generic value for the `font-style` property.
 ///