From d48fd2964da97d0d5c668938a1af7950179e2f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 Aug 2017 11:56:04 +0200 Subject: [PATCH 1/4] style: Make ComputedValuesMap a bit more generic. --- components/style/custom_properties.rs | 21 +++++++++++-------- .../style/properties/properties.mako.rs | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index cb9e3956184..7eca6b6eece 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -97,32 +97,35 @@ impl ToCss for ComputedValue { /// DOM. CSSDeclarations expose property names as indexed properties, which /// need to be stable. So we keep an array of property names which order is /// determined on the order that they are added to the name-value map. +pub type CustomPropertiesMap = OrderedMap; + +/// A map that preserves order for the keys, and that is easily indexable. #[derive(Clone, Debug, Eq, PartialEq)] -pub struct CustomPropertiesMap { +pub struct OrderedMap { /// Custom property name index. index: Vec, /// Computed values indexed by custom property name. - values: HashMap, + values: HashMap, } -impl CustomPropertiesMap { +impl OrderedMap { /// Creates a new custom properties map. pub fn new() -> Self { - CustomPropertiesMap { + OrderedMap { index: Vec::new(), values: HashMap::new(), } } /// Insert a computed value if it has not previously been inserted. - pub fn insert(&mut self, name: &Name, value: ComputedValue) { + pub fn insert(&mut self, name: &Name, value: T) { debug_assert!(!self.index.contains(name)); self.index.push(name.clone()); self.values.insert(name.clone(), value); } /// Custom property computed value getter by name. - pub fn get_computed_value(&self, name: &Name) -> Option<&ComputedValue> { + pub fn get(&self, name: &Name) -> Option<&T> { let value = self.values.get(name); debug_assert_eq!(value.is_some(), self.index.contains(name)); value @@ -134,7 +137,7 @@ impl CustomPropertiesMap { } /// Get an iterator for custom properties computed values. - pub fn iter(&self) -> hash_map::Iter { + pub fn iter(&self) -> hash_map::Iter { self.values.iter() } @@ -525,7 +528,7 @@ fn substitute_one(name: &Name, custom_properties_map: &mut CustomPropertiesMap, invalid: &mut HashSet) -> Result { - if let Some(computed_value) = custom_properties_map.get_computed_value(&name) { + if let Some(computed_value) = custom_properties_map.get(&name) { if let Some(partial_computed_value) = partial_computed_value { partial_computed_value.push_variable(computed_value) } @@ -679,7 +682,7 @@ pub fn substitute<'i>(input: &'i str, first_token_type: TokenSerializationType, let mut position = (input.position(), first_token_type); let last_token_type = substitute_block( &mut input, &mut position, &mut substituted, &mut |name, substituted| { - if let Some(value) = computed_values_map.as_ref().and_then(|map| map.get_computed_value(name)) { + if let Some(value) = computed_values_map.as_ref().and_then(|map| map.get(name)) { substituted.push_variable(value); Ok(value.last_token_type) } else { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 82b2f4cb73b..dd0049cb75b 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2314,7 +2314,7 @@ impl ComputedValuesInner { PropertyDeclarationId::Custom(name) => { self.custom_properties .as_ref() - .and_then(|map| map.get_computed_value(name)) + .and_then(|map| map.get(name)) .map(|value| value.to_css_string()) .unwrap_or(String::new()) } From 6d61d62bf486cc279b78b08a57b18a878eced094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 Aug 2017 12:08:58 +0200 Subject: [PATCH 2/4] style: Make the OrderedMap also generic on the value. --- components/style/custom_properties.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 7eca6b6eece..bc7f448cc6b 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -16,6 +16,7 @@ use std::ascii::AsciiExt; use std::borrow::Cow; use std::collections::{HashMap, hash_map, HashSet}; use std::fmt; +use std::hash::Hash; use style_traits::{HasViewportPercentage, ToCss, StyleParseError, ParseError}; /// A custom property name is just an `Atom`. @@ -97,18 +98,24 @@ impl ToCss for ComputedValue { /// DOM. CSSDeclarations expose property names as indexed properties, which /// need to be stable. So we keep an array of property names which order is /// determined on the order that they are added to the name-value map. -pub type CustomPropertiesMap = OrderedMap; +pub type CustomPropertiesMap = OrderedMap; /// A map that preserves order for the keys, and that is easily indexable. #[derive(Clone, Debug, Eq, PartialEq)] -pub struct OrderedMap { +pub struct OrderedMap +where + K: Eq + Hash + Clone, +{ /// Custom property name index. - index: Vec, + index: Vec, /// Computed values indexed by custom property name. - values: HashMap, + values: HashMap, } -impl OrderedMap { +impl OrderedMap +where + K: Eq + Hash + Clone, +{ /// Creates a new custom properties map. pub fn new() -> Self { OrderedMap { @@ -118,26 +125,26 @@ impl OrderedMap { } /// Insert a computed value if it has not previously been inserted. - pub fn insert(&mut self, name: &Name, value: T) { + pub fn insert(&mut self, name: &K, value: V) { debug_assert!(!self.index.contains(name)); self.index.push(name.clone()); self.values.insert(name.clone(), value); } /// Custom property computed value getter by name. - pub fn get(&self, name: &Name) -> Option<&T> { + pub fn get(&self, name: &K) -> Option<&V> { let value = self.values.get(name); debug_assert_eq!(value.is_some(), self.index.contains(name)); value } /// Get the name of a custom property given its list index. - pub fn get_name_at(&self, index: u32) -> Option<&Name> { + pub fn get_key_at(&self, index: u32) -> Option<&K> { self.index.get(index as usize) } /// Get an iterator for custom properties computed values. - pub fn iter(&self) -> hash_map::Iter { + pub fn iter(&self) -> hash_map::Iter { self.values.iter() } From fd1f14a41e2ed86089a06eb6ad8f9d69fca2862f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 Aug 2017 12:35:16 +0200 Subject: [PATCH 3/4] style: Use OrderedMap for specified values too. --- components/style/custom_properties.rs | 63 +++++++++++++++++---------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index bc7f448cc6b..953b9f58e15 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -13,7 +13,7 @@ use properties::{CSSWideKeyword, DeclaredValue}; use selectors::parser::SelectorParseError; use servo_arc::Arc; use std::ascii::AsciiExt; -use std::borrow::Cow; +use std::borrow::{Borrow, Cow}; use std::collections::{HashMap, hash_map, HashSet}; use std::fmt; use std::hash::Hash; @@ -125,10 +125,10 @@ where } /// Insert a computed value if it has not previously been inserted. - pub fn insert(&mut self, name: &K, value: V) { - debug_assert!(!self.index.contains(name)); + pub fn insert(&mut self, name: K, value: V) { + debug_assert!(!self.index.contains(&name)); self.index.push(name.clone()); - self.values.insert(name.clone(), value); + self.values.insert(name, value); } /// Custom property computed value getter by name. @@ -153,6 +153,19 @@ where debug_assert_eq!(self.values.len(), self.index.len()); self.values.len() } + + fn remove(&mut self, key: &Q) -> Option + where + K: Borrow, + Q: Hash + Eq, + { + let index = match self.index.iter().position(|k| k.borrow() == key) { + Some(p) => p, + None => return None, + }; + self.index.remove(index); + self.values.remove(key) + } } impl ComputedValue { @@ -397,7 +410,7 @@ fn parse_var_function<'i, 't>(input: &mut Parser<'i, 't>, /// Add one custom property declaration to a map, unless another with the same /// name was already there. -pub fn cascade<'a>(custom_properties: &mut Option>>, +pub fn cascade<'a>(custom_properties: &mut Option>>, inherited: &'a Option>, seen: &mut HashSet<&'a Name>, name: &'a Name, @@ -410,17 +423,19 @@ pub fn cascade<'a>(custom_properties: &mut Option map, None => { - *custom_properties = Some(match *inherited { - Some(ref inherited) => inherited.iter().map(|(key, inherited_value)| { - (key, BorrowedSpecifiedValue { + let mut map = OrderedMap::new(); + if let Some(ref inherited) = *inherited { + for name in &inherited.index { + let inherited_value = inherited.get(name).unwrap(); + map.insert(name, BorrowedSpecifiedValue { css: &inherited_value.css, first_token_type: inherited_value.first_token_type, last_token_type: inherited_value.last_token_type, references: None }) - }).collect(), - None => HashMap::new(), - }); + } + } + *custom_properties = Some(map); custom_properties.as_mut().unwrap() } }; @@ -450,7 +465,7 @@ pub fn cascade<'a>(custom_properties: &mut Option>, +pub fn finish_cascade(specified_values_map: Option>, inherited: &Option>) -> Option> { if let Some(mut map) = specified_values_map { @@ -465,15 +480,15 @@ pub fn finish_cascade(specified_values_map: Option) { +fn remove_cycles(map: &mut OrderedMap<&Name, BorrowedSpecifiedValue>) { let mut to_remove = HashSet::new(); { let mut visited = HashSet::new(); let mut stack = Vec::new(); - for name in map.keys() { + for name in &map.index { walk(map, name, &mut stack, &mut visited, &mut to_remove); - fn walk<'a>(map: &HashMap<&'a Name, BorrowedSpecifiedValue<'a>>, + fn walk<'a>(map: &OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>, name: &'a Name, stack: &mut Vec<&'a Name>, visited: &mut HashSet<&'a Name>, @@ -482,7 +497,7 @@ fn remove_cycles(map: &mut HashMap<&Name, BorrowedSpecifiedValue>) { if already_visited_before { return } - if let Some(value) = map.get(name) { + if let Some(value) = map.get(&name) { if let Some(references) = value.references { stack.push(name); for next in references { @@ -501,18 +516,20 @@ fn remove_cycles(map: &mut HashMap<&Name, BorrowedSpecifiedValue>) { } } } - for name in &to_remove { - map.remove(name); + for name in to_remove { + map.remove(&name); } } /// Replace `var()` functions for all custom properties. -fn substitute_all(specified_values_map: HashMap<&Name, BorrowedSpecifiedValue>, +fn substitute_all(specified_values_map: OrderedMap<&Name, BorrowedSpecifiedValue>, inherited: &Option>) -> CustomPropertiesMap { let mut custom_properties_map = CustomPropertiesMap::new(); let mut invalid = HashSet::new(); - for (&name, value) in &specified_values_map { + for name in &specified_values_map.index { + let value = specified_values_map.get(name).unwrap(); + // If this value is invalid at computed-time it won’t be inserted in computed_values_map. // Nothing else to do. let _ = substitute_one( @@ -529,7 +546,7 @@ fn substitute_all(specified_values_map: HashMap<&Name, BorrowedSpecifiedValue>, /// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise. fn substitute_one(name: &Name, specified_value: &BorrowedSpecifiedValue, - specified_values_map: &HashMap<&Name, BorrowedSpecifiedValue>, + specified_values_map: &OrderedMap<&Name, BorrowedSpecifiedValue>, inherited: &Option>, partial_computed_value: Option<&mut ComputedValue>, custom_properties_map: &mut CustomPropertiesMap, @@ -553,7 +570,7 @@ fn substitute_one(name: &Name, let result = substitute_block( &mut input, &mut position, &mut partial_computed_value, &mut |name, partial_computed_value| { - if let Some(other_specified_value) = specified_values_map.get(name) { + if let Some(other_specified_value) = specified_values_map.get(&name) { substitute_one(name, other_specified_value, specified_values_map, inherited, Some(partial_computed_value), custom_properties_map, invalid) } else { @@ -585,7 +602,7 @@ fn substitute_one(name: &Name, partial_computed_value.push_variable(&computed_value) } let last_token_type = computed_value.last_token_type; - custom_properties_map.insert(name, computed_value); + custom_properties_map.insert(name.clone(), computed_value); Ok(last_token_type) } From ff1e701fa69ccd37c06067c3bcfa22922955279c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 4 Aug 2017 14:16:43 +0200 Subject: [PATCH 4/4] style: Implement OrderedMapIterator --- components/style/custom_properties.rs | 74 ++++++++++++++++++++------- ports/geckolib/glue.rs | 4 +- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 953b9f58e15..c564aad4287 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -14,7 +14,7 @@ use selectors::parser::SelectorParseError; use servo_arc::Arc; use std::ascii::AsciiExt; use std::borrow::{Borrow, Cow}; -use std::collections::{HashMap, hash_map, HashSet}; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::hash::Hash; use style_traits::{HasViewportPercentage, ToCss, StyleParseError, ParseError}; @@ -106,9 +106,9 @@ pub struct OrderedMap where K: Eq + Hash + Clone, { - /// Custom property name index. + /// Key index. index: Vec, - /// Computed values indexed by custom property name. + /// Key-value map. values: HashMap, } @@ -116,7 +116,7 @@ impl OrderedMap where K: Eq + Hash + Clone, { - /// Creates a new custom properties map. + /// Creates a new ordered map. pub fn new() -> Self { OrderedMap { index: Vec::new(), @@ -124,36 +124,41 @@ where } } - /// Insert a computed value if it has not previously been inserted. - pub fn insert(&mut self, name: K, value: V) { - debug_assert!(!self.index.contains(&name)); - self.index.push(name.clone()); - self.values.insert(name, value); + /// Insert a new key-value pair. + pub fn insert(&mut self, key: K, value: V) { + if !self.values.contains_key(&key) { + self.index.push(key.clone()); + } + self.values.insert(key, value); } - /// Custom property computed value getter by name. - pub fn get(&self, name: &K) -> Option<&V> { - let value = self.values.get(name); - debug_assert_eq!(value.is_some(), self.index.contains(name)); + /// Get a value given its key. + pub fn get(&self, key: &K) -> Option<&V> { + let value = self.values.get(key); + debug_assert_eq!(value.is_some(), self.index.contains(key)); value } - /// Get the name of a custom property given its list index. + /// Get the key located at the given index. pub fn get_key_at(&self, index: u32) -> Option<&K> { self.index.get(index as usize) } - /// Get an iterator for custom properties computed values. - pub fn iter(&self) -> hash_map::Iter { - self.values.iter() + /// Get an ordered map iterator. + pub fn iter<'a>(&'a self) -> OrderedMapIterator<'a, K, V> { + OrderedMapIterator { + inner: self, + pos: 0, + } } - /// Get the count of custom properties computed values. + /// Get the count of items in the map. pub fn len(&self) -> usize { debug_assert_eq!(self.values.len(), self.index.len()); self.values.len() } + /// Remove an item given its key. fn remove(&mut self, key: &Q) -> Option where K: Borrow, @@ -168,6 +173,39 @@ where } } +/// An iterator for OrderedMap. +/// +/// The iteration order is determined by the order that the values are +/// added to the key-value map. +pub struct OrderedMapIterator<'a, K, V> +where + K: 'a + Eq + Hash + Clone, V: 'a, +{ + /// The OrderedMap itself. + inner: &'a OrderedMap, + /// The position of the iterator. + pos: usize, +} + +impl<'a, K, V> Iterator for OrderedMapIterator<'a, K, V> +where + K: Eq + Hash + Clone, +{ + type Item = (&'a K, &'a V); + + fn next(&mut self) -> Option { + let ref index = self.inner.index; + if self.pos >= index.len() { + return None; + } + + let ref key = index[index.len() - self.pos - 1]; + self.pos += 1; + let value = self.inner.values.get(key).unwrap(); + Some((key, value)) + } +} + impl ComputedValue { fn empty() -> ComputedValue { ComputedValue { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 852ef0242da..a8fadfc202c 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -3519,7 +3519,7 @@ pub extern "C" fn Servo_GetCustomPropertyValue(computed_values: ServoStyleContex }; let name = unsafe { Atom::from((&*name)) }; - let computed_value = match custom_properties.get_computed_value(&name) { + let computed_value = match custom_properties.get(&name) { Some(v) => v, None => return false, }; @@ -3545,7 +3545,7 @@ pub extern "C" fn Servo_GetCustomPropertyNameAt(computed_values: ServoStyleConte None => return false, }; - let property_name = match custom_properties.get_name_at(index) { + let property_name = match custom_properties.get_key_at(index) { Some(n) => n, None => return false, };