Bug 1831509 - Hardening color checks r=emilio

Avoid trying to resolve a currentcolor when a foreground color is not
available.

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

servo/components/style/properties/cascade.rs

@@ -442,10 +442,10 @@ fn tweak_when_ignoring_colors(
 
     fn alpha_channel(color: &Color, context: &computed::Context) -> f32 {
         // We assume here currentColor is opaque.
-        let color = color
+        color
             .to_computed_value(context)
-            .resolve_into_absolute(&AbsoluteColor::black());
-        color.alpha
+            .resolve_to_absolute(&AbsoluteColor::black())
+            .alpha
     }
 
     // A few special-cases ahead.

servo/components/style/properties/properties.mako.rs

@@ -3226,7 +3226,7 @@ impl ComputedValues {
     #[inline]
     pub fn resolve_color(&self, color: computed::Color) -> crate::color::AbsoluteColor {
         let current_color = self.get_inherited_text().clone_color();
-        color.resolve_into_absolute(&current_color)
+        color.resolve_to_absolute(&current_color)
     }
 
     /// Returns which longhand properties have different values in the two

servo/components/style/values/animated/color.rs

@@ -22,8 +22,7 @@ impl Animate for AbsoluteColor {
             other,
             right_weight as f32,
             /* normalize_weights = */ false,
-        )
-        .into())
+        ))
     }
 }
 
@@ -60,7 +59,7 @@ impl Animate for Color {
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
         let (left_weight, right_weight) = procedure.weights();
-        let mut color = Color::ColorMix(Box::new(ColorMix {
+        Ok(Self::from_color_mix(ColorMix {
             interpolation: ColorInterpolationMethod::srgb(),
             left: self.clone(),
             left_percentage: Percentage(left_weight as f32),
@@ -68,9 +67,7 @@ impl Animate for Color {
             right_percentage: Percentage(right_weight as f32),
             // See https://github.com/w3c/csswg-drafts/issues/7324
             normalize_weights: false,
-        }));
-        color.simplify(None);
-        Ok(color)
+        }))
     }
 }
 
@@ -78,9 +75,8 @@ impl ComputeSquaredDistance for Color {
     #[inline]
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
         let current_color = AbsoluteColor::transparent();
-        self.clone()
-            .resolve_into_absolute(&current_color)
-            .compute_squared_distance(&other.clone().resolve_into_absolute(&current_color))
+        self.resolve_to_absolute(&current_color)
+            .compute_squared_distance(&other.resolve_to_absolute(&current_color))
     }
 }
 

servo/components/style/values/computed/color.rs

@@ -7,7 +7,9 @@
 use crate::color::AbsoluteColor;
 use crate::values::animated::ToAnimatedZero;
 use crate::values::computed::percentage::Percentage;
-use crate::values::generics::color::{GenericCaretColor, GenericColor, GenericColorOrAuto};
+use crate::values::generics::color::{
+    GenericCaretColor, GenericColor, GenericColorMix, GenericColorOrAuto,
+};
 use cssparser::Color as CSSParserColor;
 use std::fmt;
 use style_traits::{CssWriter, ToCss};
@@ -23,6 +25,9 @@ pub type MozFontSmoothingBackgroundColor = AbsoluteColor;
 /// A computed value for `<color>`.
 pub type Color = GenericColor<Percentage>;
 
+/// A computed color-mix().
+pub type ColorMix = GenericColorMix<Color, Percentage>;
+
 impl ToCss for Color {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
@@ -37,6 +42,16 @@ impl ToCss for Color {
 }
 
 impl Color {
+    /// Create a new computed [`Color`] from a given color-mix, simplifying it to an absolute color
+    /// if possible.
+    pub fn from_color_mix(color_mix: ColorMix) -> Self {
+        if let Some(absolute) = color_mix.mix_to_absolute() {
+            Self::Absolute(absolute)
+        } else {
+            Self::ColorMix(Box::new(color_mix))
+        }
+    }
+
     /// Returns a complex color value representing transparent.
     pub fn transparent() -> Color {
         Color::Absolute(AbsoluteColor::transparent())
@@ -54,9 +69,25 @@ impl Color {
 
     /// Combine this complex color with the given foreground color into an
     /// absolute color.
-    pub fn resolve_into_absolute(mut self, current_color: &AbsoluteColor) -> AbsoluteColor {
-        self.simplify(Some(&current_color));
-        *self.as_absolute().unwrap()
+    pub fn resolve_to_absolute(&self, current_color: &AbsoluteColor) -> AbsoluteColor {
+        use crate::values::specified::percentage::ToPercentage;
+
+        match *self {
+            Self::Absolute(c) => c,
+            Self::CurrentColor => *current_color,
+            Self::ColorMix(ref mix) => {
+                let left = mix.left.resolve_to_absolute(current_color);
+                let right = mix.right.resolve_to_absolute(current_color);
+                crate::color::mix::mix(
+                    mix.interpolation,
+                    &left,
+                    mix.left_percentage.to_percentage(),
+                    &right,
+                    mix.right_percentage.to_percentage(),
+                    mix.normalize_weights,
+                )
+            },
+        }
     }
 }
 

servo/components/style/values/generics/color.rs

@@ -6,7 +6,6 @@
 
 use crate::color::mix::ColorInterpolationMethod;
 use crate::color::AbsoluteColor;
-use crate::values::animated::ToAnimatedValue;
 use crate::values::specified::percentage::ToPercentage;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
@@ -94,23 +93,21 @@ impl<Color: ToCss, Percentage: ToCss + ToPercentage> ToCss for ColorMix<Color, P
 impl<Percentage> ColorMix<GenericColor<Percentage>, Percentage> {
     /// Mix the colors so that we get a single color. If any of the 2 colors are
     /// not mixable (perhaps not absolute?), then return None.
-    fn mix_into_absolute(&self) -> Option<AbsoluteColor>
+    pub fn mix_to_absolute(&self) -> Option<AbsoluteColor>
     where
         Percentage: ToPercentage,
     {
-        let left = self.left.as_absolute()?.to_animated_value();
-        let right = self.right.as_absolute()?.to_animated_value();
+        let left = self.left.as_absolute()?;
+        let right = self.right.as_absolute()?;
 
-        let mixed = crate::color::mix::mix(
+        Some(crate::color::mix::mix(
             self.interpolation,
             &left,
             self.left_percentage.to_percentage(),
             &right,
             self.right_percentage.to_percentage(),
             self.normalize_weights,
-        );
-
-        Some(ToAnimatedValue::from_animated_value(mixed.into()))
+        ))
     }
 }
 
@@ -125,30 +122,6 @@ impl<Percentage> Color<Percentage> {
         }
     }
 
-    /// Simplifies the color-mix()es to the extent possible given a current
-    /// color (or not).
-    pub fn simplify(&mut self, current_color: Option<&AbsoluteColor>)
-    where
-        Percentage: ToPercentage,
-    {
-        match *self {
-            Self::Absolute(..) => {},
-            Self::CurrentColor => {
-                if let Some(c) = current_color {
-                    *self = Self::Absolute(c.clone());
-                }
-            },
-            Self::ColorMix(ref mut mix) => {
-                mix.left.simplify(current_color);
-                mix.right.simplify(current_color);
-
-                if let Some(mix) = mix.mix_into_absolute() {
-                    *self = Self::Absolute(mix);
-                }
-            },
-        }
-    }
-
     /// Returns a color value representing currentcolor.
     pub fn currentcolor() -> Self {
         Self::CurrentColor

servo/components/style/values/specified/color.rs

@@ -864,16 +864,15 @@ impl Color {
 
                 let left = mix.left.to_computed_color(context)?;
                 let right = mix.right.to_computed_color(context)?;
-                let mut color = ComputedColor::ColorMix(Box::new(GenericColorMix {
+
+                ComputedColor::from_color_mix(GenericColorMix {
                     interpolation: mix.interpolation,
                     left,
                     left_percentage: Percentage(mix.left_percentage.get()),
                     right,
                     right_percentage: Percentage(mix.right_percentage.get()),
                     normalize_weights: mix.normalize_weights,
-                }));
-                color.simplify(None);
-                color
+                })
             },
             #[cfg(feature = "gecko")]
             Color::System(system) => system.compute(context?),
@@ -927,7 +926,7 @@ impl ToComputedValue for MozFontSmoothingBackgroundColor {
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
         self.0
             .to_computed_value(context)
-            .resolve_into_absolute(&AbsoluteColor::transparent())
+            .resolve_to_absolute(&AbsoluteColor::transparent())
     }
 
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
@@ -976,7 +975,7 @@ impl ToComputedValue for ColorPropertyValue {
         let current_color = context.builder.get_parent_inherited_text().clone_color();
         self.0
             .to_computed_value(context)
-            .resolve_into_absolute(&current_color)
+            .resolve_to_absolute(&current_color)
     }
 
     #[inline]

servo/ports/geckolib/glue.rs

@@ -768,7 +768,7 @@ pub extern "C" fn Servo_AnimationValue_GetColor(
         AnimationValue::BackgroundColor(ref color) => {
             let computed: ComputedColor = color.clone();
             let foreground_color = convert_nscolor_to_absolute_color(foreground_color);
-            convert_absolute_color_to_nscolor(&computed.resolve_into_absolute(&foreground_color))
+            convert_absolute_color_to_nscolor(&computed.resolve_to_absolute(&foreground_color))
         },
         _ => panic!("Other color properties are not supported yet"),
     }
@@ -7199,7 +7199,7 @@ pub unsafe extern "C" fn Servo_ComputeColor(
         *was_current_color = computed.is_currentcolor();
     }
 
-    let rgba = computed.resolve_into_absolute(&current_color);
+    let rgba = computed.resolve_to_absolute(&current_color);
     *result_color = style::gecko::values::convert_absolute_color_to_nscolor(&rgba);
     true
 }
@@ -7209,7 +7209,7 @@ pub extern "C" fn Servo_ResolveColor(
     color: &computed::Color,
     foreground: &style::color::AbsoluteColor,
 ) -> style::color::AbsoluteColor {
-    color.clone().resolve_into_absolute(foreground)
+    color.resolve_to_absolute(foreground)
 }
 
 #[no_mangle]