servo: Merge #18783 - style: Introduce CustomPropertiesBuilder (from emilio:custom-props-builder); r=SimonSapin
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 08 Oct 2017 09:50:04 -0500
changeset 427677 acb806f1eaab0b2f1b1a49137f3f64f2f455c60d
parent 427676 0761ebb5bc7992cecc174a901c412bf3b80b0513
child 427678 6947cddff40c1473a088d3fb2b21bf9a4db808ec
push id97
push userfmarier@mozilla.com
push dateSat, 14 Oct 2017 01:12:59 +0000
reviewersSimonSapin
milestone58.0a1
servo: Merge #18783 - style: Introduce CustomPropertiesBuilder (from emilio:custom-props-builder); r=SimonSapin I'm about to introduce more state here to implement optimizations for custom property cascading, so this abstraction is useful to encapsulate that state. Source-Repo: https://github.com/servo/servo Source-Revision: 55a37930b218713fff4ba84b4fa1e43a0455e498
servo/components/style/custom_properties.rs
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
servo/tests/unit/style/custom_properties.rs
--- a/servo/components/style/custom_properties.rs
+++ b/servo/components/style/custom_properties.rs
@@ -452,19 +452,69 @@ fn parse_var_function<'i, 't>(
         })?;
     }
     if let Some(refs) = references {
         refs.insert(Atom::from(name));
     }
     Ok(())
 }
 
+/// A struct that takes care of encapsulating the cascade process for custom
+/// properties.
+pub struct CustomPropertiesBuilder<'a> {
+    seen: PrecomputedHashSet<&'a Name>,
+    custom_properties: Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
+    inherited: Option<&'a Arc<CustomPropertiesMap>>,
+}
+
+impl<'a> CustomPropertiesBuilder<'a> {
+    /// Create a new builder, inheriting from a given custom properties map.
+    pub fn new(inherited: Option<&'a Arc<CustomPropertiesMap>>) -> Self {
+        Self {
+            seen: PrecomputedHashSet::default(),
+            custom_properties: None,
+            inherited,
+        }
+    }
+
+    /// Cascade a given custom property declaration.
+    pub fn cascade(
+        &mut self,
+        name: &'a Name,
+        specified_value: DeclaredValue<'a, Box<SpecifiedValue>>,
+    ) {
+        cascade(
+            &mut self.custom_properties,
+            self.inherited,
+            &mut self.seen,
+            name,
+            specified_value,
+        )
+    }
+
+    /// Returns the final map of applicable custom properties.
+    ///
+    /// If there was any specified property, we've created a new map and now we need
+    /// to remove any potential cycles, and wrap it in an arc.
+    ///
+    /// Otherwise, just use the inherited custom properties map.
+    pub fn build(mut self) -> Option<Arc<CustomPropertiesMap>> {
+        let mut map = match self.custom_properties.take() {
+            Some(map) => map,
+            None => return self.inherited.cloned(),
+        };
+
+        remove_cycles(&mut map);
+        Some(Arc::new(substitute_all(map)))
+    }
+}
+
 /// Add one custom property declaration to a map, unless another with the same
 /// name was already there.
-pub fn cascade<'a>(
+fn cascade<'a>(
     custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
     inherited: Option<&'a Arc<CustomPropertiesMap>>,
     seen: &mut PrecomputedHashSet<&'a Name>,
     name: &'a Name,
     specified_value: DeclaredValue<'a, Box<SpecifiedValue>>
 ) {
     let was_already_present = !seen.insert(name);
     if was_already_present {
@@ -504,34 +554,16 @@ pub fn cascade<'a>(
                 map.remove(&name);
             }
             CSSWideKeyword::Unset | // Custom properties are inherited by default.
             CSSWideKeyword::Inherit => {} // The inherited value is what we already have.
         }
     }
 }
 
-/// Returns the final map of applicable custom properties.
-///
-/// If there was any specified property, we've created a new map and now we need
-/// to remove any potential cycles, and wrap it in an arc.
-///
-/// Otherwise, just use the inherited custom properties map.
-pub fn finish_cascade(
-    specified_values_map: Option<OrderedMap<&Name, BorrowedSpecifiedValue>>,
-    inherited: Option<&Arc<CustomPropertiesMap>>,
-) -> Option<Arc<CustomPropertiesMap>> {
-    if let Some(mut map) = specified_values_map {
-        remove_cycles(&mut map);
-        Some(Arc::new(substitute_all(map)))
-    } else {
-        inherited.cloned()
-    }
-}
-
 /// https://drafts.csswg.org/css-variables/#cycles
 ///
 /// The initial value of a custom property is represented by this property not
 /// being in the map.
 fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) {
     let mut to_remove = PrecomputedHashSet::default();
     {
         let mut visited = PrecomputedHashSet::default();
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -4,16 +4,17 @@
 
 //! A property declaration block.
 
 #![deny(missing_docs)]
 
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important, ParserInput, CowRcStr};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseError as CssParseError};
+use custom_properties::CustomPropertiesBuilder;
 use error_reporting::{ParseErrorReporter, ContextualParseError};
 use parser::{ParserContext, ParserErrorContext};
 use properties::animated_properties::AnimationValue;
 use selectors::parser::SelectorParseError;
 use shared_lock::Locked;
 use smallbitvec::{self, SmallBitVec};
 use smallvec::SmallVec;
 use std::fmt;
@@ -686,34 +687,25 @@ impl PropertyDeclarationBlock {
 
     /// Returns a custom properties map which is the result of cascading custom
     /// properties in this declaration block along with the given custom
     /// properties.
     pub fn cascade_custom_properties(
         &self,
         inherited_custom_properties: Option<&Arc<::custom_properties::CustomPropertiesMap>>,
     ) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
-        let mut custom_properties = None;
-        let mut seen_custom = PrecomputedHashSet::default();
+        let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties);
 
         for declaration in self.normal_declaration_iter() {
             if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
-                ::custom_properties::cascade(
-                    &mut custom_properties,
-                    inherited_custom_properties,
-                    &mut seen_custom,
-                    name,
-                    value.borrow(),
-                );
+                builder.cascade(name, value.borrow());
             }
         }
-        ::custom_properties::finish_cascade(
-            custom_properties,
-            inherited_custom_properties,
-        )
+
+        builder.build()
     }
 }
 
 impl ToCss for PropertyDeclarationBlock {
     // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -7,16 +7,17 @@
 // Please note that valid Rust syntax may be mangled by the Mako parser.
 // For example, Vec<&Foo> will be mangled as Vec&Foo>. To work around these issues, the code
 // can be escaped. In the above example, Vec<<&Foo> or Vec< &Foo> achieves the desired result of Vec<&Foo>.
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 #[cfg(feature = "servo")]
 use app_units::Au;
+use custom_properties::CustomPropertiesBuilder;
 use servo_arc::{Arc, UniqueArc};
 use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
 use std::{fmt, mem, ops};
 use std::cell::RefCell;
 
 #[cfg(feature = "servo")] use cssparser::RGBA;
 use cssparser::{CowRcStr, Parser, TokenSerializationType, serialize_identifier};
@@ -28,17 +29,16 @@ use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")] use gecko_bindings::bindings;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::{self, nsCSSPropertyID};
 #[cfg(feature = "servo")] use logical_geometry::LogicalMargin;
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use parser::ParserContext;
 #[cfg(feature = "gecko")] use properties::longhands::system_font::SystemFont;
 use rule_cache::{RuleCache, RuleCacheConditions};
-use selector_map::PrecomputedHashSet;
 use selector_parser::PseudoElement;
 use selectors::parser::SelectorParseError;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
 use shared_lock::StylesheetGuards;
 use style_traits::{PARSING_MODE_DEFAULT, ToCss, ParseError};
 use style_traits::{PropertyDeclarationParseError, StyleParseError, ValueParseError};
 use stylesheets::{CssRuleType, Origin, UrlExtraData};
 #[cfg(feature = "servo")] use values::Either;
@@ -3223,36 +3223,28 @@ where
              layout_parent_style.unwrap_or(parent_style))
         },
         None => {
             (device.default_computed_values(),
              device.default_computed_values())
         }
     };
 
-    let inherited_custom_properties = inherited_style.custom_properties();
-    let mut custom_properties = None;
-    let mut seen_custom = PrecomputedHashSet::default();
-    for (declaration, _cascade_level) in iter_declarations() {
-        if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
-            ::custom_properties::cascade(
-                &mut custom_properties,
-                inherited_custom_properties,
-                &mut seen_custom,
-                name,
-                value.borrow(),
-            );
+    let custom_properties = {
+        let mut builder =
+            CustomPropertiesBuilder::new(inherited_style.custom_properties());
+
+        for (declaration, _cascade_level) in iter_declarations() {
+            if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
+                builder.cascade(name, value.borrow());
+            }
         }
-    }
-
-    let custom_properties =
-        ::custom_properties::finish_cascade(
-            custom_properties,
-            inherited_custom_properties,
-        );
+
+        builder.build()
+    };
 
     let mut context = computed::Context {
         is_root_element: flags.contains(IS_ROOT_ELEMENT),
         // We'd really like to own the rules here to avoid refcount traffic, but
         // animation's usage of `apply_declarations` make this tricky. See bug
         // 1375525.
         builder: StyleBuilder::new(
             device,
--- a/servo/tests/unit/style/custom_properties.rs
+++ b/servo/tests/unit/style/custom_properties.rs
@@ -1,41 +1,35 @@
 /* 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/. */
 
 use cssparser::{Parser, ParserInput};
 use servo_arc::Arc;
-use style::custom_properties::{self, Name, SpecifiedValue, CustomPropertiesMap};
+use style::custom_properties::{Name, SpecifiedValue, CustomPropertiesMap, CustomPropertiesBuilder};
 use style::properties::DeclaredValue;
 use test::{self, Bencher};
 
 fn cascade(
     name_and_value: &[(&str, &str)],
     inherited: Option<&Arc<CustomPropertiesMap>>,
 ) -> Option<Arc<CustomPropertiesMap>> {
     let values = name_and_value.iter().map(|&(name, value)| {
         let mut input = ParserInput::new(value);
         let mut parser = Parser::new(&mut input);
         (Name::from(name), SpecifiedValue::parse(&mut parser).unwrap())
     }).collect::<Vec<_>>();
 
-    let mut custom_properties = None;
-    let mut seen = Default::default();
+    let mut builder = CustomPropertiesBuilder::new(inherited);
+
     for &(ref name, ref val) in &values {
-        custom_properties::cascade(
-            &mut custom_properties,
-            inherited,
-            &mut seen,
-            name,
-            DeclaredValue::Value(val)
-        )
+        builder.cascade(name, DeclaredValue::Value(val));
     }
 
-    custom_properties::finish_cascade(custom_properties, inherited)
+    builder.build()
 }
 
 #[bench]
 fn cascade_custom_simple(b: &mut Bencher) {
     b.iter(|| {
         let parent = cascade(&[
             ("foo", "10px"),
             ("bar", "100px"),