Bug 1718281 - Make get_shorthand_appendable_value less generic. r=layout-reviewers,AlaskanEmily

This will allow to save some codesize from longhands_to_css.

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

servo/components/style/properties/declaration_block.rs

@@ -386,7 +386,7 @@ impl PropertyDeclarationBlock {
         // Step 1.2.3
         // We don't print !important when serializing individual properties,
         // so we treat this as a normal-importance property
-        match shorthand.get_shorthand_appendable_value(list.iter().cloned()) {
+        match shorthand.get_shorthand_appendable_value(&list) {
             Some(appendable_value) => append_declaration_value(dest, appendable_value),
             None => return Ok(()),
         }
@@ -915,9 +915,6 @@ impl PropertyDeclarationBlock {
     ///
     /// https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
     pub fn to_css(&self, dest: &mut CssStringWriter) -> fmt::Result {
-        use std::iter::Cloned;
-        use std::slice;
-
         let mut is_first_serialization = true; // trailing serializations should have a prepended space
 
         // Step 1 -> dest = result list
@@ -942,7 +939,7 @@ impl PropertyDeclarationBlock {
                     // properties in a declaration block, and that custom
                     // properties can't be part of a shorthand, we can just care
                     // about them here.
-                    append_serialization::<Cloned<slice::Iter<_>>, _>(
+                    append_serialization(
                         dest,
                         &property,
                         AppendableValue::Declaration(declaration),
@@ -996,7 +993,7 @@ impl PropertyDeclarationBlock {
 
                 // Step 3.4.3:
                 //     Let current longhands be an empty array.
-                let mut current_longhands = SmallVec::<[_; 10]>::new();
+                let mut current_longhands = SmallVec::<[&_; 10]>::new();
                 let mut logical_groups = LogicalGroupSet::new();
                 let mut saw_one = false;
                 let mut logical_mismatch = false;
@@ -1070,7 +1067,7 @@ impl PropertyDeclarationBlock {
                 //    Let value be the result of invoking serialize a CSS value
                 //    of current longhands.
                 let appendable_value = match shorthand
-                    .get_shorthand_appendable_value(current_longhands.iter().cloned())
+                    .get_shorthand_appendable_value(&current_longhands)
                 {
                     None => continue,
                     Some(appendable_value) => appendable_value,
@@ -1113,7 +1110,7 @@ impl PropertyDeclarationBlock {
                 //
                 // 3.4.10:
                 //     Append serialized declaration to list.
-                append_serialization::<Cloned<slice::Iter<_>>, _>(
+                append_serialization(
                     dest,
                     &shorthand,
                     value,
@@ -1149,7 +1146,7 @@ impl PropertyDeclarationBlock {
             //     its important flag set.
             //
             //     Append serialized declaration to list.
-            append_serialization::<Cloned<slice::Iter<_>>, _>(
+            append_serialization(
                 dest,
                 &property,
                 AppendableValue::Declaration(declaration),
@@ -1169,17 +1166,14 @@ impl PropertyDeclarationBlock {
 
 /// A convenient enum to represent different kinds of stuff that can represent a
 /// _value_ in the serialization of a property declaration.
-pub enum AppendableValue<'a, I>
-where
-    I: Iterator<Item = &'a PropertyDeclaration>,
-{
+pub enum AppendableValue<'a, 'b: 'a> {
     /// A given declaration, of which we'll serialize just the value.
     Declaration(&'a PropertyDeclaration),
     /// A set of declarations for a given shorthand.
     ///
     /// FIXME: This needs more docs, where are the shorthands expanded? We print
     /// the property name before-hand, don't we?
-    DeclarationsForShorthand(ShorthandId, I),
+    DeclarationsForShorthand(ShorthandId, &'a [&'b PropertyDeclaration]),
     /// A raw CSS string, coming for example from a property with CSS variables,
     /// or when storing a serialized shorthand value before appending directly.
     Css(&'a str),
@@ -1199,13 +1193,10 @@ where
 }
 
 /// Append a given kind of appendable value to a serialization.
-pub fn append_declaration_value<'a, I>(
+pub fn append_declaration_value<'a, 'b: 'a>(
     dest: &mut CssStringWriter,
-    appendable_value: AppendableValue<'a, I>,
-) -> fmt::Result
-where
-    I: Iterator<Item = &'a PropertyDeclaration>,
-{
+    appendable_value: AppendableValue<'a, 'b>,
+) -> fmt::Result {
     match appendable_value {
         AppendableValue::Css(css) => dest.write_str(css),
         AppendableValue::Declaration(decl) => decl.to_css(dest),
@@ -1216,15 +1207,14 @@ where
 }
 
 /// Append a given property and value pair to a serialization.
-pub fn append_serialization<'a, I, N>(
+pub fn append_serialization<'a, 'b: 'a, N>(
     dest: &mut CssStringWriter,
     property_name: &N,
-    appendable_value: AppendableValue<'a, I>,
+    appendable_value: AppendableValue<'a, 'b>,
     importance: Importance,
     is_first_serialization: &mut bool,
 ) -> fmt::Result
 where
-    I: Iterator<Item = &'a PropertyDeclaration>,
     N: ToCss,
 {
     handle_first_serialization(dest, is_first_serialization)?;

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

@@ -1471,15 +1471,17 @@ impl ShorthandId {
     ///
     /// Returns an error if writing to the stream fails, or if the declarations
     /// do not map to a shorthand.
-    pub fn longhands_to_css<'a, W, I>(
+    pub fn longhands_to_css<W>(
         &self,
-        declarations: I,
+        declarations: &[&PropertyDeclaration],
         dest: &mut CssWriter<W>,
     ) -> fmt::Result
     where
         W: Write,
-        I: Iterator<Item=&'a PropertyDeclaration>,
     {
+        // TODO(emilio): Save codesize here by using a lookup table on
+        // ShorthandId instead.
+        let declarations = declarations.iter().cloned();
         match *self {
             ShorthandId::All => {
                 // No need to try to serialize the declarations as the 'all'
@@ -1502,25 +1504,16 @@ impl ShorthandId {
     /// Finds and returns an appendable value for the given declarations.
     ///
     /// Returns the optional appendable value.
-    pub fn get_shorthand_appendable_value<'a, I>(
+    pub fn get_shorthand_appendable_value<'a, 'b: 'a>(
         self,
-        declarations: I,
-    ) -> Option<AppendableValue<'a, I::IntoIter>>
-    where
-        I: IntoIterator<Item=&'a PropertyDeclaration>,
-        I::IntoIter: Clone,
-    {
-        let declarations = declarations.into_iter();
-
-        // Only cloning iterators (a few pointers each) not declarations.
-        let mut declarations2 = declarations.clone();
-        let mut declarations3 = declarations.clone();
-
-        let first_declaration = declarations2.next()?;
+        declarations: &'a [&'b PropertyDeclaration],
+    ) -> Option<AppendableValue<'a, 'b>> {
+        let first_declaration = declarations.get(0)?;
+        let rest = || declarations.iter().skip(1);
 
         // https://drafts.csswg.org/css-variables/#variables-in-shorthands
         if let Some(css) = first_declaration.with_variables_from_shorthand(self) {
-            if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
+            if rest().all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
                return Some(AppendableValue::Css(css));
             }
             return None;
@@ -1528,7 +1521,7 @@ impl ShorthandId {
 
         // Check whether they are all the same CSS-wide keyword.
         if let Some(keyword) = first_declaration.get_css_wide_keyword() {
-            if declarations2.all(|d| d.get_css_wide_keyword() == Some(keyword)) {
+            if rest().all(|d| d.get_css_wide_keyword() == Some(keyword)) {
                 return Some(AppendableValue::Css(keyword.to_str()))
             }
             return None;
@@ -1540,7 +1533,7 @@ impl ShorthandId {
         }
 
         // Check whether all declarations can be serialized as part of shorthand.
-        if declarations3.all(|d| d.may_serialize_as_part_of_shorthand()) {
+        if declarations.iter().all(|d| d.may_serialize_as_part_of_shorthand()) {
             return Some(AppendableValue::DeclarationsForShorthand(self, declarations));
         }