Bug 1507309 - Simplify background-repeat. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Nov 2018 03:23:39 +0000
changeset 502981 8ee9454879c874c0bd1a3b8788707adb52193eb0
parent 502980 488a0a90ca916e094358cf9766a8aef648e711a5
child 502982 6c54e79a66b51815649c5c8488050f6e12206bcb
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1507309
milestone65.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 1507309 - Simplify background-repeat. r=heycam This way we always serialize in the shortest form, and take less space. This is useful because when serializing uncomputed values we'd like to compare to the initial value to avoid serializing parts of a shorthand, but with the existing implementation we would generate always a second keyword, which means that we'll never match it. This also matches Chrome and WebKit, incidentally, so I'm pretty confident the behavior change when serializing specified style is web-compatible. Differential Revision: https://phabricator.services.mozilla.com/D11941
servo/components/style/values/computed/background.rs
servo/components/style/values/specified/background.rs
--- a/servo/components/style/values/computed/background.rs
+++ b/servo/components/style/values/computed/background.rs
@@ -1,104 +1,23 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Computed types for CSS values related to backgrounds.
 
 use crate::values::computed::length::NonNegativeLengthOrPercentageOrAuto;
-use crate::values::computed::{Context, ToComputedValue};
 use crate::values::generics::background::BackgroundSize as GenericBackgroundSize;
-use crate::values::specified::background::BackgroundRepeat as SpecifiedBackgroundRepeat;
-use crate::values::specified::background::BackgroundRepeatKeyword;
-use std::fmt::{self, Write};
-use style_traits::{CssWriter, ToCss};
+
+pub use crate::values::specified::background::BackgroundRepeat;
 
 /// A computed value for the `background-size` property.
 pub type BackgroundSize = GenericBackgroundSize<NonNegativeLengthOrPercentageOrAuto>;
 
 impl BackgroundSize {
     /// Returns `auto auto`.
     pub fn auto() -> Self {
         GenericBackgroundSize::Explicit {
             width: NonNegativeLengthOrPercentageOrAuto::auto(),
             height: NonNegativeLengthOrPercentageOrAuto::auto(),
         }
     }
 }
-
-/// The computed value of the `background-repeat` property:
-///
-/// https://drafts.csswg.org/css-backgrounds/#the-background-repeat
-#[derive(Clone, Debug, MallocSizeOf, PartialEq)]
-pub struct BackgroundRepeat(pub BackgroundRepeatKeyword, pub BackgroundRepeatKeyword);
-
-impl BackgroundRepeat {
-    /// Returns the `repeat repeat` value.
-    pub fn repeat() -> Self {
-        BackgroundRepeat(
-            BackgroundRepeatKeyword::Repeat,
-            BackgroundRepeatKeyword::Repeat,
-        )
-    }
-}
-
-impl ToCss for BackgroundRepeat {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        match (self.0, self.1) {
-            (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => {
-                dest.write_str("repeat-x")
-            },
-            (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => {
-                dest.write_str("repeat-y")
-            },
-            (horizontal, vertical) => {
-                horizontal.to_css(dest)?;
-                if horizontal != vertical {
-                    dest.write_str(" ")?;
-                    vertical.to_css(dest)?;
-                }
-                Ok(())
-            },
-        }
-    }
-}
-
-impl ToComputedValue for SpecifiedBackgroundRepeat {
-    type ComputedValue = BackgroundRepeat;
-
-    #[inline]
-    fn to_computed_value(&self, _: &Context) -> Self::ComputedValue {
-        match *self {
-            SpecifiedBackgroundRepeat::RepeatX => BackgroundRepeat(
-                BackgroundRepeatKeyword::Repeat,
-                BackgroundRepeatKeyword::NoRepeat,
-            ),
-            SpecifiedBackgroundRepeat::RepeatY => BackgroundRepeat(
-                BackgroundRepeatKeyword::NoRepeat,
-                BackgroundRepeatKeyword::Repeat,
-            ),
-            SpecifiedBackgroundRepeat::Keywords(horizontal, vertical) => {
-                BackgroundRepeat(horizontal, vertical.unwrap_or(horizontal))
-            },
-        }
-    }
-
-    #[inline]
-    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        // FIXME(emilio): Why can't this just be:
-        //   SpecifiedBackgroundRepeat::Keywords(computed.0, computed.1)
-        match (computed.0, computed.1) {
-            (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => {
-                SpecifiedBackgroundRepeat::RepeatX
-            },
-            (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => {
-                SpecifiedBackgroundRepeat::RepeatY
-            },
-            (horizontal, vertical) => {
-                SpecifiedBackgroundRepeat::Keywords(horizontal, Some(vertical))
-            },
-        }
-    }
-}
--- a/servo/components/style/values/specified/background.rs
+++ b/servo/components/style/values/specified/background.rs
@@ -4,17 +4,18 @@
 
 //! Specified types for CSS values related to backgrounds.
 
 use crate::parser::{Parse, ParserContext};
 use crate::values::generics::background::BackgroundSize as GenericBackgroundSize;
 use crate::values::specified::length::NonNegativeLengthOrPercentageOrAuto;
 use cssparser::Parser;
 use selectors::parser::SelectorParseErrorKind;
-use style_traits::ParseError;
+use std::fmt::{self, Write};
+use style_traits::{CssWriter, ParseError, ToCss};
 
 /// A specified value for the `background-size` property.
 pub type BackgroundSize = GenericBackgroundSize<NonNegativeLengthOrPercentageOrAuto>;
 
 impl Parse for BackgroundSize {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -51,62 +52,88 @@ impl BackgroundSize {
     MallocSizeOf,
     Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToComputedValue,
     ToCss,
 )]
 #[allow(missing_docs)]
+#[value_info(other_values = "repeat-x,repeat-y")]
 pub enum BackgroundRepeatKeyword {
     Repeat,
     Space,
     Round,
     NoRepeat,
 }
 
-/// The specified value for the `background-repeat` property.
+/// The value of the `background-repeat` property, with `repeat-x` / `repeat-y`
+/// represented as the combination of `no-repeat` and `repeat` in the opposite
+/// axes.
 ///
 /// https://drafts.csswg.org/css-backgrounds/#the-background-repeat
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
-pub enum BackgroundRepeat {
-    /// `repeat-x`
-    RepeatX,
-    /// `repeat-y`
-    RepeatY,
-    /// `[repeat | space | round | no-repeat]{1,2}`
-    Keywords(BackgroundRepeatKeyword, Option<BackgroundRepeatKeyword>),
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue)]
+pub struct BackgroundRepeat(pub BackgroundRepeatKeyword, pub BackgroundRepeatKeyword);
+
+impl BackgroundRepeat {
+    /// Returns the `repeat repeat` value.
+    pub fn repeat() -> Self {
+        BackgroundRepeat(
+            BackgroundRepeatKeyword::Repeat,
+            BackgroundRepeatKeyword::Repeat,
+        )
+    }
 }
 
-impl BackgroundRepeat {
-    /// Returns the `repeat` value.
-    #[inline]
-    pub fn repeat() -> Self {
-        BackgroundRepeat::Keywords(BackgroundRepeatKeyword::Repeat, None)
+impl ToCss for BackgroundRepeat {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        match (self.0, self.1) {
+            (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => {
+                dest.write_str("repeat-x")
+            },
+            (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => {
+                dest.write_str("repeat-y")
+            },
+            (horizontal, vertical) => {
+                horizontal.to_css(dest)?;
+                if horizontal != vertical {
+                    dest.write_str(" ")?;
+                    vertical.to_css(dest)?;
+                }
+                Ok(())
+            },
+        }
     }
 }
 
 impl Parse for BackgroundRepeat {
     fn parse<'i, 't>(
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let ident = input.expect_ident_cloned()?;
 
         match_ignore_ascii_case! { &ident,
-            "repeat-x" => return Ok(BackgroundRepeat::RepeatX),
-            "repeat-y" => return Ok(BackgroundRepeat::RepeatY),
+            "repeat-x" => {
+                return Ok(BackgroundRepeat(BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat));
+            },
+            "repeat-y" => {
+                return Ok(BackgroundRepeat(BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat));
+            },
             _ => {},
         }
 
         let horizontal = match BackgroundRepeatKeyword::from_ident(&ident) {
             Ok(h) => h,
             Err(()) => {
                 return Err(
                     input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone()))
                 );
             },
         };
 
         let vertical = input.try(BackgroundRepeatKeyword::parse).ok();
-        Ok(BackgroundRepeat::Keywords(horizontal, vertical))
+        Ok(BackgroundRepeat(horizontal, vertical.unwrap_or(horizontal)))
     }
 }