Bug 1532135 - Derive more stuff for clip rects. r=boris
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 06 Mar 2019 21:37:48 +0000
changeset 520594 1e12deff6c1e0a6c5cabc0374627a52499ae880d
parent 520593 26ac004eeb4cc3876cf822e91ef25206586a1537
child 520595 04e9867c3f738e2daf7df1fbe7743bef58f1a08c
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersboris
bugs1532135
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1532135 - Derive more stuff for clip rects. r=boris I feel a bit weird for using LenghtPercentageOrAuto to implement LengthOrAuto, but I don't think much other code will use it so it seemed a bit better to me. Differential Revision: https://phabricator.services.mozilla.com/D21863
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/longhands/column.mako.rs
servo/components/style/values/computed/length.rs
servo/components/style/values/computed/mod.rs
servo/components/style/values/generics/mod.rs
servo/components/style/values/specified/length.rs
servo/components/style/values/specified/mod.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3743,58 +3743,66 @@ fn static_assert() {
         longhands::quotes::computed_value::T(
             Box::<[QuotePair]>::as_arc(&quote_pairs).clone_arc()
         )
     }
 
     #[allow(non_snake_case)]
     pub fn set__moz_image_region(&mut self, v: longhands::_moz_image_region::computed_value::T) {
         use crate::values::Either;
+        use crate::values::generics::length::LengthPercentageOrAuto::*;
 
         match v {
             Either::Second(_auto) => {
                 self.gecko.mImageRegion.x = 0;
                 self.gecko.mImageRegion.y = 0;
                 self.gecko.mImageRegion.width = 0;
                 self.gecko.mImageRegion.height = 0;
             }
             Either::First(rect) => {
-                self.gecko.mImageRegion.x = rect.left.map(Au::from).unwrap_or(Au(0)).0;
-                self.gecko.mImageRegion.y = rect.top.map(Au::from).unwrap_or(Au(0)).0;
+                self.gecko.mImageRegion.x = match rect.left {
+                    LengthPercentage(v) => v.to_i32_au(),
+                    Auto => 0,
+                };
+                self.gecko.mImageRegion.y = match rect.top {
+                    LengthPercentage(v) => v.to_i32_au(),
+                    Auto => 0,
+                };
                 self.gecko.mImageRegion.height = match rect.bottom {
-                    Some(value) => (Au::from(value) - Au(self.gecko.mImageRegion.y)).0,
-                    None => 0,
+                    LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.y)).0,
+                    Auto => 0,
                 };
                 self.gecko.mImageRegion.width = match rect.right {
-                    Some(value) => (Au::from(value) - Au(self.gecko.mImageRegion.x)).0,
-                    None => 0,
+                    LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.x)).0,
+                    Auto => 0,
                 };
             }
         }
     }
 
     #[allow(non_snake_case)]
     pub fn clone__moz_image_region(&self) -> longhands::_moz_image_region::computed_value::T {
         use crate::values::{Auto, Either};
+        use crate::values::generics::length::LengthPercentageOrAuto::*;
         use crate::values::computed::ClipRect;
 
         // There is no ideal way to detect auto type for structs::nsRect and its components, so
         // if all components are zero, we use Auto.
         if self.gecko.mImageRegion.x == 0 &&
            self.gecko.mImageRegion.y == 0 &&
            self.gecko.mImageRegion.width == 0 &&
            self.gecko.mImageRegion.height == 0 {
            return Either::Second(Auto);
         }
 
         Either::First(ClipRect {
-            top: Some(Au(self.gecko.mImageRegion.y).into()),
-            right: Some(Au(self.gecko.mImageRegion.width + self.gecko.mImageRegion.x).into()),
-            bottom: Some(Au(self.gecko.mImageRegion.height + self.gecko.mImageRegion.y).into()),
-            left: Some(Au(self.gecko.mImageRegion.x).into()),
+            top: LengthPercentage(Au(self.gecko.mImageRegion.y).into()),
+            right: LengthPercentage(Au(self.gecko.mImageRegion.width + self.gecko.mImageRegion.x).into()),
+            bottom: LengthPercentage(Au(self.gecko.mImageRegion.height + self.gecko.mImageRegion.y).into()),
+            left: LengthPercentage(Au(self.gecko.mImageRegion.x).into()),
         })
     }
 
     ${impl_simple_copy('_moz_image_region', 'mImageRegion')}
 
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Table" skip_longhands="-x-span">
@@ -3841,48 +3849,53 @@ fn static_assert() {
 
     pub fn set_clip(&mut self, v: longhands::clip::computed_value::T) {
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_RECT;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO;
+        use crate::values::generics::length::LengthPercentageOrAuto::*;
         use crate::values::Either;
 
         match v {
             Either::First(rect) => {
                 self.gecko.mClipFlags = NS_STYLE_CLIP_RECT as u8;
-                if let Some(left) = rect.left {
-                    self.gecko.mClip.x = left.to_i32_au();
-                } else {
-                    self.gecko.mClip.x = 0;
-                    self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8;
-                }
-
-                if let Some(top) = rect.top {
-                    self.gecko.mClip.y = top.to_i32_au();
-                } else {
-                    self.gecko.mClip.y = 0;
-                    self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8;
-                }
-
-                if let Some(bottom) = rect.bottom {
-                    self.gecko.mClip.height = (Au::from(bottom) - Au(self.gecko.mClip.y)).0;
-                } else {
-                    self.gecko.mClip.height = 1 << 30; // NS_MAXSIZE
-                    self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8;
-                }
-
-                if let Some(right) = rect.right {
-                    self.gecko.mClip.width = (Au::from(right) - Au(self.gecko.mClip.x)).0;
-                } else {
-                    self.gecko.mClip.width = 1 << 30; // NS_MAXSIZE
-                    self.gecko.mClipFlags |= NS_STYLE_CLIP_RIGHT_AUTO as u8;
-                }
+                self.gecko.mClip.x = match rect.left {
+                    LengthPercentage(l) => l.to_i32_au(),
+                    Auto => {
+                        self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8;
+                        0
+                    }
+                };
+
+                self.gecko.mClip.y = match rect.top {
+                    LengthPercentage(l) => l.to_i32_au(),
+                    Auto => {
+                        self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8;
+                        0
+                    }
+                };
+
+                self.gecko.mClip.height = match rect.bottom {
+                    LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.y)).0,
+                    Auto => {
+                        self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8;
+                        1 << 30 // NS_MAXSIZE
+                    }
+                };
+
+                self.gecko.mClip.width = match rect.right {
+                    LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.x)).0,
+                    Auto => {
+                        self.gecko.mClipFlags |= NS_STYLE_CLIP_RIGHT_AUTO as u8;
+                        1 << 30 // NS_MAXSIZE
+                    }
+                };
             },
             Either::Second(_auto) => {
                 self.gecko.mClipFlags = NS_STYLE_CLIP_AUTO as u8;
                 self.gecko.mClip.x = 0;
                 self.gecko.mClip.y = 0;
                 self.gecko.mClip.width = 0;
                 self.gecko.mClip.height = 0;
             }
@@ -3899,52 +3912,52 @@ fn static_assert() {
     }
 
     pub fn clone_clip(&self) -> longhands::clip::computed_value::T {
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO;
         use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO;
+        use crate::values::generics::length::LengthPercentageOrAuto::*;
         use crate::values::computed::{ClipRect, ClipRectOrAuto};
         use crate::values::Either;
 
         if self.gecko.mClipFlags == NS_STYLE_CLIP_AUTO as u8 {
-            ClipRectOrAuto::auto()
+            return ClipRectOrAuto::auto()
+        }
+        let left = if self.gecko.mClipFlags & NS_STYLE_CLIP_LEFT_AUTO as u8 != 0 {
+            debug_assert_eq!(self.gecko.mClip.x, 0);
+            Auto
+        } else {
+            LengthPercentage(Au(self.gecko.mClip.x).into())
+        };
+
+        let top = if self.gecko.mClipFlags & NS_STYLE_CLIP_TOP_AUTO as u8 != 0 {
+            debug_assert_eq!(self.gecko.mClip.y, 0);
+            Auto
         } else {
-            let left = if self.gecko.mClipFlags & NS_STYLE_CLIP_LEFT_AUTO as u8 != 0 {
-                debug_assert_eq!(self.gecko.mClip.x, 0);
-                None
-            } else {
-                Some(Au(self.gecko.mClip.x).into())
-            };
-
-            let top = if self.gecko.mClipFlags & NS_STYLE_CLIP_TOP_AUTO as u8 != 0 {
-                debug_assert_eq!(self.gecko.mClip.y, 0);
-                None
-            } else {
-                Some(Au(self.gecko.mClip.y).into())
-            };
-
-            let bottom = if self.gecko.mClipFlags & NS_STYLE_CLIP_BOTTOM_AUTO as u8 != 0 {
-                debug_assert_eq!(self.gecko.mClip.height, 1 << 30); // NS_MAXSIZE
-                None
-            } else {
-                Some(Au(self.gecko.mClip.y + self.gecko.mClip.height).into())
-            };
-
-            let right = if self.gecko.mClipFlags & NS_STYLE_CLIP_RIGHT_AUTO as u8 != 0 {
-                debug_assert_eq!(self.gecko.mClip.width, 1 << 30); // NS_MAXSIZE
-                None
-            } else {
-                Some(Au(self.gecko.mClip.x + self.gecko.mClip.width).into())
-            };
-
-            Either::First(ClipRect { top, right, bottom, left })
-        }
+            LengthPercentage(Au(self.gecko.mClip.y).into())
+        };
+
+        let bottom = if self.gecko.mClipFlags & NS_STYLE_CLIP_BOTTOM_AUTO as u8 != 0 {
+            debug_assert_eq!(self.gecko.mClip.height, 1 << 30); // NS_MAXSIZE
+            Auto
+        } else {
+            LengthPercentage(Au(self.gecko.mClip.y + self.gecko.mClip.height).into())
+        };
+
+        let right = if self.gecko.mClipFlags & NS_STYLE_CLIP_RIGHT_AUTO as u8 != 0 {
+            debug_assert_eq!(self.gecko.mClip.width, 1 << 30); // NS_MAXSIZE
+            Auto
+        } else {
+            LengthPercentage(Au(self.gecko.mClip.x + self.gecko.mClip.width).into())
+        };
+
+        Either::First(ClipRect { top, right, bottom, left })
     }
 
     <%
     # This array is several filter function which has percentage or
     # number value for function of clone / set.
     # The setting / cloning process of other function(e.g. Blur / HueRotate) is
     # different from these function. So this array don't include such function.
     FILTER_FUNCTIONS = [ 'Brightness', 'Contrast', 'Grayscale', 'Invert',
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -797,39 +797,39 @@ impl ToAnimatedZero for Visibility {
         Err(())
     }
 }
 
 /// <https://drafts.csswg.org/css-transitions/#animtype-rect>
 impl Animate for ClipRect {
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
-        use crate::values::computed::Length;
-        let animate_component = |this: &Option<Length>, other: &Option<Length>| {
-            match (this.animate(other, procedure)?, procedure) {
-                (None, Procedure::Interpolate { .. }) => Ok(None),
-                (None, _) => Err(()),
-                (result, _) => Ok(result),
+        use crate::values::computed::LengthOrAuto;
+        let animate_component = |this: &LengthOrAuto, other: &LengthOrAuto| {
+            let result = this.animate(other, procedure)?;
+            if let Procedure::Interpolate { .. } = procedure {
+                return Ok(result);
             }
+            if result.is_auto() {
+                // FIXME(emilio): Why? A couple SMIL tests fail without this,
+                // but it seems extremely fishy.
+                return Err(());
+            }
+            Ok(result)
         };
 
         Ok(ClipRect {
-            top:    animate_component(&self.top, &other.top)?,
-            right:  animate_component(&self.right, &other.right)?,
+            top: animate_component(&self.top, &other.top)?,
+            right: animate_component(&self.right, &other.right)?,
             bottom: animate_component(&self.bottom, &other.bottom)?,
-            left:   animate_component(&self.left, &other.left)?,
+            left: animate_component(&self.left, &other.left)?,
         })
     }
 }
 
-impl ToAnimatedZero for ClipRect {
-    #[inline]
-    fn to_animated_zero(&self) -> Result<Self, ()> { Err(()) }
-}
-
 <%
     FILTER_FUNCTIONS = [ 'Blur', 'Brightness', 'Contrast', 'Grayscale',
                          'HueRotate', 'Invert', 'Opacity', 'Saturate',
                          'Sepia' ]
 %>
 
 /// <https://drafts.fxtf.org/filters/#animation-of-filters>
 impl Animate for AnimatedFilter {
--- a/servo/components/style/properties/longhands/column.mako.rs
+++ b/servo/components/style/properties/longhands/column.mako.rs
@@ -4,18 +4,18 @@
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 <% data.new_style_struct("Column", inherited=False) %>
 
 ${helpers.predefined_type(
     "column-width",
     "length::NonNegativeLengthOrAuto",
-    "Either::Second(Auto)",
-    initial_specified_value="Either::Second(Auto)",
+    "computed::length::NonNegativeLengthOrAuto::auto()",
+    initial_specified_value="specified::length::NonNegativeLengthOrAuto::auto()",
     extra_prefixes="moz",
     animation_value_type="NonNegativeLengthOrAuto",
     servo_pref="layout.columns.enabled",
     spec="https://drafts.csswg.org/css-multicol/#propdef-column-width",
     servo_restyle_damage="rebuild_and_reflow",
 )}
 
 ${helpers.predefined_type(
--- a/servo/components/style/values/computed/length.rs
+++ b/servo/components/style/values/computed/length.rs
@@ -10,17 +10,17 @@ use crate::values::computed::NonNegative
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::generics::length as generics;
 use crate::values::generics::length::{
     GenericLengthOrNumber, MaxSize as GenericMaxSize, Size as GenericSize,
 };
 use crate::values::generics::NonNegative;
 use crate::values::specified::length::ViewportPercentageLength;
 use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength};
-use crate::values::{specified, Auto, CSSFloat, Either, Normal};
+use crate::values::{specified, CSSFloat, Either, Normal};
 use crate::Zero;
 use app_units::Au;
 use ordered_float::NotNan;
 use std::fmt::{self, Write};
 use std::ops::{Add, Neg};
 use style_traits::values::specified::AllowedNumericType;
 use style_traits::{CssWriter, ToCss};
 
@@ -696,17 +696,20 @@ impl From<Au> for CSSPixelLength {
         CSSPixelLength::new(len.to_f32_px())
     }
 }
 
 /// An alias of computed `<length>` value.
 pub type Length = CSSPixelLength;
 
 /// Either a computed `<length>` or the `auto` keyword.
-pub type LengthOrAuto = Either<Length, Auto>;
+pub type LengthOrAuto = generics::GenericLengthPercentageOrAuto<Length>;
+
+/// Either a non-negative `<length>` or the `auto` keyword.
+pub type NonNegativeLengthOrAuto = generics::GenericLengthPercentageOrAuto<NonNegativeLength>;
 
 /// Either a computed `<length>` or a `<number>` value.
 pub type LengthOrNumber = GenericLengthOrNumber<Length, Number>;
 
 /// Either a computed `<length>` or the `normal` keyword.
 pub type LengthOrNormal = Either<Length, Normal>;
 
 /// A wrapper of Length, whose value must be >= 0.
@@ -774,19 +777,16 @@ impl From<Au> for NonNegativeLength {
 
 impl From<NonNegativeLength> for Au {
     #[inline]
     fn from(non_negative_len: NonNegativeLength) -> Self {
         Au::from(non_negative_len.0)
     }
 }
 
-/// Either a computed NonNegativeLength or the `auto` keyword.
-pub type NonNegativeLengthOrAuto = Either<NonNegativeLength, Auto>;
-
 /// Either a computed NonNegativeLength or the `normal` keyword.
 pub type NonNegativeLengthOrNormal = Either<NonNegativeLength, Normal>;
 
 /// Either a computed NonNegativeLengthPercentage or the `normal` keyword.
 pub type NonNegativeLengthPercentageOrNormal = Either<NonNegativeLengthPercentage, Normal>;
 
 /// Either a non-negative `<length>` or a `<number>`.
 pub type NonNegativeLengthOrNumber = GenericLengthOrNumber<NonNegativeLength, NonNegativeNumber>;
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -5,35 +5,33 @@
 //! Computed values.
 
 use self::transform::DirectionVector;
 use super::animated::ToAnimatedValue;
 use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent;
 use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth};
 use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize};
 use super::generics::transform::IsParallelTo;
-use super::generics::{GreaterThanOrEqualToOne, NonNegative};
+use super::generics::{self, GreaterThanOrEqualToOne, NonNegative};
 use super::specified;
 use super::{CSSFloat, CSSInteger};
 use crate::context::QuirksMode;
 use crate::font_metrics::{get_metrics_provider_for_product, FontMetricsProvider};
 use crate::media_queries::Device;
 #[cfg(feature = "gecko")]
 use crate::properties;
 use crate::properties::{ComputedValues, LonghandId, StyleBuilder};
 use crate::rule_cache::RuleCacheConditions;
 use crate::Atom;
 #[cfg(feature = "servo")]
 use crate::Prefix;
 use euclid::Size2D;
 use std::cell::RefCell;
 use std::cmp;
 use std::f32;
-use std::fmt::{self, Write};
-use style_traits::{CssWriter, ToCss};
 
 #[cfg(feature = "gecko")]
 pub use self::align::{AlignContent, AlignItems, JustifyContent, JustifyItems, SelfAlignment};
 #[cfg(feature = "gecko")]
 pub use self::align::{AlignSelf, JustifySelf};
 pub use self::angle::Angle;
 pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::basic_shape::FillRule;
@@ -57,17 +55,17 @@ pub use self::font::{FontSize, FontSizeA
 pub use self::font::{FontVariantAlternates, FontWeight};
 pub use self::font::{FontVariantEastAsian, FontVariationSettings};
 pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom};
 #[cfg(feature = "gecko")]
 pub use self::gecko::ScrollSnapPoint;
 pub use self::image::{Gradient, GradientItem, Image, ImageLayer, LineDirection, MozImageRect};
 pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength};
 pub use self::length::{Length, LengthOrNumber, LengthPercentage, NonNegativeLengthOrNumber};
-pub use self::length::{LengthPercentageOrAuto, MaxSize, Size};
+pub use self::length::{LengthOrAuto, LengthPercentageOrAuto, MaxSize, Size};
 pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto};
 #[cfg(feature = "gecko")]
 pub use self::list::ListStyleType;
 pub use self::list::{QuotePair, Quotes};
 pub use self::motion::OffsetPath;
 pub use self::outline::OutlineStyle;
 pub use self::percentage::{NonNegativePercentage, Percentage};
 pub use self::position::{GridAutoFlow, GridTemplateAreas, Position, ZIndex};
@@ -632,62 +630,18 @@ impl ToAnimatedValue for PositiveInteger
 
 impl From<CSSInteger> for PositiveInteger {
     #[inline]
     fn from(int: CSSInteger) -> PositiveInteger {
         GreaterThanOrEqualToOne::<CSSInteger>(int)
     }
 }
 
-#[allow(missing_docs)]
-#[cfg_attr(feature = "servo", derive(MallocSizeOf))]
-#[derive(Clone, ComputeSquaredDistance, Copy, Debug, PartialEq)]
-/// A computed cliprect for clip and image-region
-pub struct ClipRect {
-    pub top: Option<Length>,
-    pub right: Option<Length>,
-    pub bottom: Option<Length>,
-    pub left: Option<Length>,
-}
-
-impl ToCss for ClipRect {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("rect(")?;
-        if let Some(top) = self.top {
-            top.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(right) = self.right {
-            right.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(bottom) = self.bottom {
-            bottom.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(left) = self.left {
-            left.to_css(dest)?;
-        } else {
-            dest.write_str("auto")?;
-        }
-        dest.write_str(")")
-    }
-}
+/// rect(...)
+pub type ClipRect = generics::ClipRect<LengthOrAuto>;
 
 /// rect(...) | auto
 pub type ClipRectOrAuto = Either<ClipRect, Auto>;
 
 /// The computed value of a grid `<track-breadth>`
 pub type TrackBreadth = GenericTrackBreadth<LengthPercentage>;
 
 /// The computed value of a grid `<track-size>`
--- a/servo/components/style/values/generics/mod.rs
+++ b/servo/components/style/values/generics/mod.rs
@@ -207,8 +207,19 @@ impl<T: Zero> Zero for NonNegative<T> {
     PartialEq,
     PartialOrd,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,
     ToCss,
 )]
 pub struct GreaterThanOrEqualToOne<T>(pub T);
+
+/// A clip rect for clip and image-region
+#[allow(missing_docs)]
+#[derive(Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)]
+#[css(function = "rect", comma)]
+pub struct ClipRect<LengthOrAuto> {
+    pub top: LengthOrAuto,
+    pub right: LengthOrAuto,
+    pub bottom: LengthOrAuto,
+    pub left: LengthOrAuto,
+}
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -12,17 +12,17 @@ use crate::parser::{Parse, ParserContext
 use crate::values::computed::{self, CSSPixelLength, Context};
 use crate::values::generics::length as generics;
 use crate::values::generics::length::{
     GenericLengthOrNumber, MaxSize as GenericMaxSize, Size as GenericSize,
 };
 use crate::values::generics::NonNegative;
 use crate::values::specified::calc::CalcNode;
 use crate::values::specified::NonNegativeNumber;
-use crate::values::{Auto, CSSFloat, Either, Normal};
+use crate::values::{CSSFloat, Either, Normal};
 use crate::Zero;
 use app_units::Au;
 use cssparser::{Parser, Token};
 use euclid::Size2D;
 use std::cmp;
 use std::ops::{Add, Mul};
 use style_traits::values::specified::AllowedNumericType;
 use style_traits::{ParseError, SpecifiedValueInfo, StyleParseErrorKind};
@@ -722,19 +722,16 @@ impl NonNegativeLength {
         Ok(NonNegative(Length::parse_non_negative_quirky(
             context,
             input,
             allow_quirks,
         )?))
     }
 }
 
-/// Either a NonNegativeLength or the `auto` keyword.
-pub type NonNegativeLengthOrAuto = Either<NonNegativeLength, Auto>;
-
 /// A `<length-percentage>` value. This can be either a `<length>`, a
 /// `<percentage>`, or a combination of both via `calc()`.
 ///
 /// https://drafts.csswg.org/css-values-4/#typedef-length-percentage
 #[allow(missing_docs)]
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
 pub enum LengthPercentage {
     Length(NoCalcLength),
@@ -990,17 +987,35 @@ impl NonNegativeLengthPercentage {
         LengthPercentage::parse_non_negative_quirky(context, input, allow_quirks).map(NonNegative)
     }
 }
 
 /// Either a `<length>` or the `normal` keyword.
 pub type LengthOrNormal = Either<Length, Normal>;
 
 /// Either a `<length>` or the `auto` keyword.
-pub type LengthOrAuto = Either<Length, Auto>;
+pub type LengthOrAuto = generics::LengthPercentageOrAuto<Length>;
+
+impl LengthOrAuto {
+    /// Parses a length, allowing the unitless length quirk.
+    /// <https://quirks.spec.whatwg.org/#the-unitless-length-quirk>
+    #[inline]
+    pub fn parse_quirky<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+        allow_quirks: AllowQuirks,
+    ) -> Result<Self, ParseError<'i>> {
+        Self::parse_with(context, input, |context, input| {
+            Length::parse_quirky(context, input, allow_quirks)
+        })
+    }
+}
+
+/// Either a non-negative `<length>` or the `auto` keyword.
+pub type NonNegativeLengthOrAuto = generics::LengthPercentageOrAuto<NonNegativeLength>;
 
 /// Either a `<length>` or a `<number>`.
 pub type LengthOrNumber = GenericLengthOrNumber<Length, Number>;
 
 /// A specified value for `min-width`, `min-height`, `width` or `height` property.
 pub type Size = GenericSize<NonNegativeLengthPercentage>;
 
 impl Parse for Size {
--- a/servo/components/style/values/specified/mod.rs
+++ b/servo/components/style/values/specified/mod.rs
@@ -6,17 +6,17 @@
 //!
 //! TODO(emilio): Enhance docs.
 
 use super::computed::transform::DirectionVector;
 use super::computed::{Context, ToComputedValue};
 use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth};
 use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize};
 use super::generics::transform::IsParallelTo;
-use super::generics::{GreaterThanOrEqualToOne, NonNegative};
+use super::generics::{self, GreaterThanOrEqualToOne, NonNegative};
 use super::{Auto, CSSFloat, CSSInteger, Either};
 use crate::context::QuirksMode;
 use crate::parser::{Parse, ParserContext};
 use crate::values::serialize_atom_identifier;
 use crate::values::specified::calc::CalcNode;
 use crate::{Atom, Namespace, Prefix};
 use cssparser::{Parser, Token};
 use num_traits::{One, Zero};
@@ -55,17 +55,17 @@ pub use self::font::{FontVariantAlternat
 pub use self::font::{FontVariantEastAsian, FontVariationSettings};
 pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom};
 #[cfg(feature = "gecko")]
 pub use self::gecko::ScrollSnapPoint;
 pub use self::image::{ColorStop, EndingShape as GradientEndingShape, Gradient};
 pub use self::image::{GradientItem, GradientKind, Image, ImageLayer, MozImageRect};
 pub use self::length::{AbsoluteLength, CalcLengthPercentage, CharacterWidth};
 pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLengthOrNumber};
-pub use self::length::{LengthPercentage, LengthPercentageOrAuto};
+pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto};
 pub use self::length::{MaxSize, Size};
 pub use self::length::{NoCalcLength, ViewportPercentageLength};
 pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto};
 #[cfg(feature = "gecko")]
 pub use self::list::ListStyleType;
 pub use self::list::{QuotePair, Quotes};
 pub use self::motion::OffsetPath;
 pub use self::outline::OutlineStyle;
@@ -600,109 +600,18 @@ pub type TrackSize = GenericTrackSize<Le
 pub type TrackList = GenericTrackList<LengthPercentage, Integer>;
 
 /// The specified value of a `<grid-line>`.
 pub type GridLine = GenericGridLine<Integer>;
 
 /// `<grid-template-rows> | <grid-template-columns>`
 pub type GridTemplateComponent = GenericGridTemplateComponent<LengthPercentage, Integer>;
 
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo)]
-/// rect(<top>, <left>, <bottom>, <right>) used by clip and image-region
-#[css(function = "rect")]
-pub struct ClipRect {
-    /// <top> (<length> | <auto>)
-    pub top: Option<Length>,
-    /// <right> (<length> | <auto>)
-    pub right: Option<Length>,
-    /// <bottom> (<length> | <auto>)
-    pub bottom: Option<Length>,
-    /// <left> (<length> | <auto>)
-    pub left: Option<Length>,
-}
-
-impl ToCss for ClipRect {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("rect(")?;
-
-        if let Some(ref top) = self.top {
-            top.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(ref right) = self.right {
-            right.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(ref bottom) = self.bottom {
-            bottom.to_css(dest)?;
-            dest.write_str(", ")?;
-        } else {
-            dest.write_str("auto, ")?;
-        }
-
-        if let Some(ref left) = self.left {
-            left.to_css(dest)?;
-        } else {
-            dest.write_str("auto")?;
-        }
-
-        dest.write_str(")")?;
-        Ok(())
-    }
-}
-
-impl ToComputedValue for ClipRect {
-    type ComputedValue = super::computed::ClipRect;
-
-    #[inline]
-    fn to_computed_value(&self, context: &Context) -> super::computed::ClipRect {
-        super::computed::ClipRect {
-            top: self.top.as_ref().map(|top| top.to_computed_value(context)),
-            right: self
-                .right
-                .as_ref()
-                .map(|right| right.to_computed_value(context)),
-            bottom: self
-                .bottom
-                .as_ref()
-                .map(|bottom| bottom.to_computed_value(context)),
-            left: self
-                .left
-                .as_ref()
-                .map(|left| left.to_computed_value(context)),
-        }
-    }
-
-    #[inline]
-    fn from_computed_value(computed: &super::computed::ClipRect) -> Self {
-        ClipRect {
-            top: computed
-                .top
-                .map(|top| ToComputedValue::from_computed_value(&top)),
-            right: computed
-                .right
-                .map(|right| ToComputedValue::from_computed_value(&right)),
-            bottom: computed
-                .bottom
-                .map(|bottom| ToComputedValue::from_computed_value(&bottom)),
-            left: computed
-                .left
-                .map(|left| ToComputedValue::from_computed_value(&left)),
-        }
-    }
-}
+/// rect(...)
+pub type ClipRect = generics::ClipRect<LengthOrAuto>;
 
 impl Parse for ClipRect {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         Self::parse_quirky(context, input, AllowQuirks::No)
     }
@@ -710,35 +619,26 @@ impl Parse for ClipRect {
 
 impl ClipRect {
     /// Parses a rect(<top>, <left>, <bottom>, <right>), allowing quirks.
     pub fn parse_quirky<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         allow_quirks: AllowQuirks,
     ) -> Result<Self, ParseError<'i>> {
-        use crate::values::specified::Length;
+        input.expect_function_matching("rect")?;
 
         fn parse_argument<'i, 't>(
             context: &ParserContext,
             input: &mut Parser<'i, 't>,
             allow_quirks: AllowQuirks,
-        ) -> Result<Option<Length>, ParseError<'i>> {
-            if input
-                .try(|input| input.expect_ident_matching("auto"))
-                .is_ok()
-            {
-                Ok(None)
-            } else {
-                Length::parse_quirky(context, input, allow_quirks).map(Some)
-            }
+        ) -> Result<LengthOrAuto, ParseError<'i>> {
+            LengthOrAuto::parse_quirky(context, input, allow_quirks)
         }
 
-        input.expect_function_matching("rect")?;
-
         input.parse_nested_block(|input| {
             let top = parse_argument(context, input, allow_quirks)?;
             let right;
             let bottom;
             let left;
 
             if input.try(|input| input.expect_comma()).is_ok() {
                 right = parse_argument(context, input, allow_quirks)?;
@@ -746,22 +646,18 @@ impl ClipRect {
                 bottom = parse_argument(context, input, allow_quirks)?;
                 input.expect_comma()?;
                 left = parse_argument(context, input, allow_quirks)?;
             } else {
                 right = parse_argument(context, input, allow_quirks)?;
                 bottom = parse_argument(context, input, allow_quirks)?;
                 left = parse_argument(context, input, allow_quirks)?;
             }
-            Ok(ClipRect {
-                top: top,
-                right: right,
-                bottom: bottom,
-                left: left,
-            })
+
+            Ok(ClipRect { top, right, bottom, left })
         })
     }
 }
 
 /// rect(...) | auto
 pub type ClipRectOrAuto = Either<ClipRect, Auto>;
 
 impl ClipRectOrAuto {