Bug 1551991 - Cleanup a bit the counter style code. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 16 May 2019 23:05:00 +0000
changeset 474262 5bac84efbf6d16393e61789c8fb183fb7410a0a0
parent 474261 3c388ac5887ceec18ae5db997cc34065cad9ac2f
child 474263 336a076368ebcebbde75f4e6e468ebfe2b50ec94
push id113144
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:44:55 +0000
treeherdermozilla-inbound@f4c4b796f845 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1551991
milestone68.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 1551991 - Cleanup a bit the counter style code. r=jwatt Use more compact types, and remove some manual implementations that can be derived. Differential Revision: https://phabricator.services.mozilla.com/D31315
servo/components/style/counter_style/mod.rs
servo/components/style/gecko/values.rs
--- a/servo/components/style/counter_style/mod.rs
+++ b/servo/components/style/counter_style/mod.rs
@@ -14,17 +14,16 @@ use crate::values::specified::Integer;
 use crate::values::CustomIdent;
 use crate::Atom;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser};
 use cssparser::{CowRcStr, Parser, SourceLocation, Token};
 use selectors::parser::SelectorParseErrorKind;
 use std::fmt::{self, Write};
 use std::mem;
 use std::num::Wrapping;
-use std::ops::Range;
 use style_traits::{Comma, CssWriter, OneOrMoreSeparated, ParseError};
 use style_traits::{StyleParseErrorKind, ToCss};
 
 /// Parse a counter style name reference.
 ///
 /// This allows the reserved counter style names "decimal" and "disc".
 pub fn parse_counter_style_name<'i, 't>(
     input: &mut Parser<'i, 't>,
@@ -256,17 +255,17 @@ counter_style_descriptors! {
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-prefix>
     "prefix" prefix / set_prefix [_]: Symbol,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-suffix>
     "suffix" suffix / set_suffix [_]: Symbol,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-range>
-    "range" range / set_range [_]: Ranges,
+    "range" range / set_range [_]: CounterRanges,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-pad>
     "pad" pad / set_pad [_]: Pad,
 
     /// <https://drafts.csswg.org/css-counter-styles/#counter-style-fallback>
     "fallback" fallback / set_fallback [_]: Fallback,
 
     /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols>
@@ -366,17 +365,17 @@ impl Parse for System {
         try_match_ident_ignore_ascii_case! { input,
             "cyclic" => Ok(System::Cyclic),
             "numeric" => Ok(System::Numeric),
             "alphabetic" => Ok(System::Alphabetic),
             "symbolic" => Ok(System::Symbolic),
             "additive" => Ok(System::Additive),
             "fixed" => {
                 let first_symbol_value = input.try(|i| Integer::parse(context, i)).ok();
-                Ok(System::Fixed { first_symbol_value: first_symbol_value })
+                Ok(System::Fixed { first_symbol_value })
             }
             "extends" => {
                 let other = parse_counter_style_name(input)?;
                 Ok(System::Extends(other))
             }
         }
     }
 }
@@ -404,36 +403,35 @@ impl ToCss for System {
                 dest.write_str("extends ")?;
                 other.to_css(dest)
             },
         }
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#typedef-symbol>
-#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
-#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss, ToShmem)]
+#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss, ToShmem, MallocSizeOf)]
 pub enum Symbol {
     /// <string>
-    String(String),
+    String(crate::OwnedStr),
     /// <custom-ident>
     Ident(CustomIdent),
     // Not implemented:
     // /// <image>
     // Image(Image),
 }
 
 impl Parse for Symbol {
     fn parse<'i, 't>(
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let location = input.current_source_location();
         match *input.next()? {
-            Token::QuotedString(ref s) => Ok(Symbol::String(s.as_ref().to_owned())),
+            Token::QuotedString(ref s) => Ok(Symbol::String(s.as_ref().to_owned().into())),
             Token::Ident(ref s) => Ok(Symbol::Ident(CustomIdent::from_ident(location, s, &[])?)),
             ref t => Err(location.new_unexpected_token_error(t.clone())),
         }
     }
 }
 
 impl Symbol {
     /// Returns whether this symbol is allowed in symbols() function.
@@ -458,100 +456,85 @@ impl Parse for Negative {
         Ok(Negative(
             Symbol::parse(context, input)?,
             input.try(|input| Symbol::parse(context, input)).ok(),
         ))
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#counter-style-range>
+#[derive(Clone, Debug, ToShmem, ToCss)]
+pub struct CounterRange {
+    /// The start of the range.
+    pub start: CounterBound,
+    /// The end of the range.
+    pub end: CounterBound,
+}
+
+/// <https://drafts.csswg.org/css-counter-styles/#counter-style-range>
 ///
-/// Empty Vec represents 'auto'
-#[derive(Clone, Debug, ToShmem)]
-pub struct Ranges(pub Vec<Range<CounterBound>>);
+/// Empty represents 'auto'
+#[derive(Clone, Debug, ToShmem, ToCss)]
+#[css(comma)]
+pub struct CounterRanges(
+    #[css(iterable, if_empty = "auto")]
+    pub crate::OwnedSlice<CounterRange>,
+);
 
-/// A bound found in `Ranges`.
+/// A bound found in `CounterRanges`.
 #[derive(Clone, Copy, Debug, ToCss, ToShmem)]
 pub enum CounterBound {
     /// An integer bound.
     Integer(Integer),
     /// The infinite bound.
     Infinite,
 }
 
-impl Parse for Ranges {
+impl Parse for CounterRanges {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         if input
             .try(|input| input.expect_ident_matching("auto"))
             .is_ok()
         {
-            Ok(Ranges(Vec::new()))
-        } else {
-            input
-                .parse_comma_separated(|input| {
-                    let opt_start = parse_bound(context, input)?;
-                    let opt_end = parse_bound(context, input)?;
-                    if let (CounterBound::Integer(start), CounterBound::Integer(end)) =
-                        (opt_start, opt_end)
-                    {
-                        if start > end {
-                            return Err(
-                                input.new_custom_error(StyleParseErrorKind::UnspecifiedError)
-                            );
-                        }
-                    }
-                    Ok(opt_start..opt_end)
-                })
-                .map(Ranges)
+            return Ok(CounterRanges(Default::default()));
         }
+
+        let ranges = input.parse_comma_separated(|input| {
+            let start = parse_bound(context, input)?;
+            let end = parse_bound(context, input)?;
+            if let (CounterBound::Integer(start), CounterBound::Integer(end)) =
+                (start, end)
+            {
+                if start > end {
+                    return Err(
+                        input.new_custom_error(StyleParseErrorKind::UnspecifiedError)
+                    );
+                }
+            }
+            Ok(CounterRange { start, end })
+        })?;
+
+        Ok(CounterRanges(ranges.into()))
     }
 }
 
 fn parse_bound<'i, 't>(
     context: &ParserContext,
     input: &mut Parser<'i, 't>,
 ) -> Result<CounterBound, ParseError<'i>> {
     if let Ok(integer) = input.try(|input| Integer::parse(context, input)) {
         return Ok(CounterBound::Integer(integer));
     }
     input.expect_ident_matching("infinite")?;
     Ok(CounterBound::Infinite)
 }
 
-impl ToCss for Ranges {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        let mut iter = self.0.iter();
-        if let Some(first) = iter.next() {
-            range_to_css(first, dest)?;
-            for item in iter {
-                dest.write_str(", ")?;
-                range_to_css(item, dest)?;
-            }
-            Ok(())
-        } else {
-            dest.write_str("auto")
-        }
-    }
-}
-
-fn range_to_css<W>(range: &Range<CounterBound>, dest: &mut CssWriter<W>) -> fmt::Result
-where
-    W: Write,
-{
-    range.start.to_css(dest)?;
-    dest.write_char(' ')?;
-    range.end.to_css(dest)
-}
-
 /// <https://drafts.csswg.org/css-counter-styles/#counter-style-pad>
 #[derive(Clone, Debug, ToCss, ToShmem)]
 pub struct Pad(pub Integer, pub Symbol);
 
 impl Parse for Pad {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -567,63 +550,59 @@ impl Parse for Pad {
 #[derive(Clone, Debug, ToCss, ToShmem)]
 pub struct Fallback(pub CustomIdent);
 
 impl Parse for Fallback {
     fn parse<'i, 't>(
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        parse_counter_style_name(input).map(Fallback)
+        Ok(Fallback(parse_counter_style_name(input)?))
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-symbols>
-#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
-#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss, ToShmem)]
-pub struct Symbols(#[css(iterable)] pub Vec<Symbol>);
+#[derive(Clone, Debug, Eq, PartialEq, MallocSizeOf, ToComputedValue, ToCss, ToShmem)]
+pub struct Symbols(#[css(iterable)] pub crate::OwnedSlice<Symbol>);
 
 impl Parse for Symbols {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let mut symbols = Vec::new();
-        loop {
-            if let Ok(s) = input.try(|input| Symbol::parse(context, input)) {
-                symbols.push(s)
-            } else {
-                if symbols.is_empty() {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                } else {
-                    return Ok(Symbols(symbols));
-                }
-            }
+        while let Ok(s) = input.try(|input| Symbol::parse(context, input)) {
+            symbols.push(s);
         }
+        if symbols.is_empty() {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+        }
+        Ok(Symbols(symbols.into()))
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#descdef-counter-style-additive-symbols>
 #[derive(Clone, Debug, ToCss, ToShmem)]
-pub struct AdditiveSymbols(pub Vec<AdditiveTuple>);
+#[css(comma)]
+pub struct AdditiveSymbols(#[css(iterable)] pub crate::OwnedSlice<AdditiveTuple>);
 
 impl Parse for AdditiveSymbols {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let tuples = Vec::<AdditiveTuple>::parse(context, input)?;
         // FIXME maybe? https://github.com/w3c/csswg-drafts/issues/1220
         if tuples
             .windows(2)
             .any(|window| window[0].weight <= window[1].weight)
         {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
-        Ok(AdditiveSymbols(tuples))
+        Ok(AdditiveSymbols(tuples.into()))
     }
 }
 
 /// <integer> && <symbol>
 #[derive(Clone, Debug, ToCss, ToShmem)]
 pub struct AdditiveTuple {
     /// <integer>
     pub weight: Integer,
@@ -638,20 +617,17 @@ impl OneOrMoreSeparated for AdditiveTupl
 impl Parse for AdditiveTuple {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         let symbol = input.try(|input| Symbol::parse(context, input));
         let weight = Integer::parse_non_negative(context, input)?;
         let symbol = symbol.or_else(|_| Symbol::parse(context, input))?;
-        Ok(AdditiveTuple {
-            weight: weight,
-            symbol: symbol,
-        })
+        Ok(Self { weight, symbol })
     }
 }
 
 /// <https://drafts.csswg.org/css-counter-styles/#counter-style-speak-as>
 #[derive(Clone, Debug, ToCss, ToShmem)]
 pub enum SpeakAs {
     /// auto
     Auto,
--- a/servo/components/style/gecko/values.rs
+++ b/servo/components/style/gecko/values.rs
@@ -283,17 +283,17 @@ impl CounterStyleOrNone {
             CounterStyleOrNone::Name(name) => unsafe {
                 set_name(gecko_value, name.0.into_addrefed());
             },
             CounterStyleOrNone::Symbols(symbols_type, symbols) => {
                 let symbols: Vec<_> = symbols
                     .0
                     .iter()
                     .map(|symbol| match *symbol {
-                        Symbol::String(ref s) => nsCStr::from(s),
+                        Symbol::String(ref s) => nsCStr::from(&**s),
                         Symbol::Ident(_) => unreachable!("Should not have identifier in symbols()"),
                     })
                     .collect();
                 let symbols: Vec<_> = symbols
                     .iter()
                     .map(|symbol| symbol as &nsACString as *const _)
                     .collect();
                 unsafe {
@@ -328,15 +328,15 @@ impl CounterStyleOrNone {
             let symbols = &anonymous.mSymbols;
             if anonymous.mSingleString {
                 debug_assert_eq!(symbols.len(), 1);
                 Either::Second(symbols[0].to_string())
             } else {
                 let symbol_type = SymbolsType::from_gecko_keyword(anonymous.mSystem as u32);
                 let symbols = symbols
                     .iter()
-                    .map(|gecko_symbol| Symbol::String(gecko_symbol.to_string()))
+                    .map(|gecko_symbol| Symbol::String(gecko_symbol.to_string().into()))
                     .collect();
                 Either::First(CounterStyleOrNone::Symbols(symbol_type, Symbols(symbols)))
             }
         }
     }
 }