From 03d98acfa88c6c340c4e94eebe8b0c285ef04a36 Mon Sep 17 00:00:00 2001 From: Shanavas M Date: Fri, 11 Jan 2019 00:57:02 +0100 Subject: [PATCH] Bug 1519269 - Remove OrderedMap in favor of IndexMap. r=emilio This cherry-picks https://github.com/servo/servo/pull/22656. --- Cargo.lock | 1 + servo/components/style/Cargo.toml | 1 + servo/components/style/custom_properties.rs | 141 ++------------------ servo/components/style/lib.rs | 1 + servo/ports/geckolib/glue.rs | 4 +- 5 files changed, 14 insertions(+), 134 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 173a75a300b2..ff6b24b7c944 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2425,6 +2425,7 @@ dependencies = [ "fallible 0.0.1", "fxhash 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", + "indexmap 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)", "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/servo/components/style/Cargo.toml b/servo/components/style/Cargo.toml index d1cc1c601660..9b0b64c375cd 100644 --- a/servo/components/style/Cargo.toml +++ b/servo/components/style/Cargo.toml @@ -38,6 +38,7 @@ fallible = { path = "../fallible" } fxhash = "0.2" hashglobe = { path = "../hashglobe" } html5ever = {version = "0.22", optional = true} +indexmap = "1.0" itertools = "0.7.6" itoa = "0.4" lazy_static = "1" diff --git a/servo/components/style/custom_properties.rs b/servo/components/style/custom_properties.rs index 6bc2b331e35a..f21929c70798 100644 --- a/servo/components/style/custom_properties.rs +++ b/servo/components/style/custom_properties.rs @@ -8,17 +8,17 @@ use crate::hash::map::Entry; use crate::properties::{CSSWideKeyword, CustomDeclarationValue}; -use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet}; +use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher}; use crate::Atom; use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; -use precomputed_hash::PrecomputedHash; +use indexmap::IndexMap; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; use smallvec::SmallVec; -use std::borrow::{Borrow, Cow}; +use std::borrow::Cow; use std::cmp; use std::fmt::{self, Write}; -use std::hash::Hash; +use std::hash::BuildHasherDefault; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; /// The environment from which to get `env` function values. @@ -131,7 +131,8 @@ impl ToCss for SpecifiedValue { /// /// The variable values are guaranteed to not have references to other /// properties. -pub type CustomPropertiesMap = OrderedMap>; +pub type CustomPropertiesMap = + IndexMap, BuildHasherDefault>; /// Both specified and computed values are VariableValues, the difference is /// whether var() functions are expanded. @@ -140,130 +141,6 @@ pub type SpecifiedValue = VariableValue; /// whether var() functions are expanded. pub type ComputedValue = VariableValue; -/// A map that preserves order for the keys, and that is easily indexable. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct OrderedMap -where - K: PrecomputedHash + Hash + Eq + Clone, -{ - /// Key index. - index: Vec, - /// Key-value map. - values: PrecomputedHashMap, -} - -impl OrderedMap -where - K: Eq + PrecomputedHash + Hash + Clone, -{ - /// Creates a new ordered map. - pub fn new() -> Self { - OrderedMap { - index: Vec::new(), - values: PrecomputedHashMap::default(), - } - } - - /// Insert a new key-value pair. - /// - /// TODO(emilio): Remove unused_mut when Gecko and Servo agree in whether - /// it's necessary. - #[allow(unused_mut)] - pub fn insert(&mut self, key: K, value: V) { - let OrderedMap { - ref mut index, - ref mut values, - } = *self; - match values.entry(key) { - Entry::Vacant(mut entry) => { - index.push(entry.key().clone()); - entry.insert(value); - }, - Entry::Occupied(mut entry) => { - entry.insert(value); - }, - } - } - - /// 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 whether there's a value on the map for `key`. - pub fn contains_key(&self, key: &K) -> bool { - self.values.contains_key(key) - } - - /// 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 ordered map iterator. - pub fn iter<'a>(&'a self) -> OrderedMapIterator<'a, K, V> { - OrderedMapIterator { - inner: self, - pos: 0, - } - } - - /// 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() - } - - /// Returns whether this map is empty. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Remove an item given its key. - fn remove(&mut self, key: &Q) -> Option - where - K: Borrow, - Q: PrecomputedHash + Hash + Eq, - { - let index = self.index.iter().position(|k| k.borrow() == key)?; - self.index.remove(index); - self.values.remove(key) - } -} - -/// 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 + PrecomputedHash + 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 + PrecomputedHash + Hash + Clone, -{ - type Item = (&'a K, &'a V); - - fn next(&mut self) -> Option { - let key = self.inner.index.get(self.pos)?; - - self.pos += 1; - let value = &self.inner.values[key]; - - Some((key, value)) - } -} - /// A struct holding information about the external references to that a custom /// property value may have. #[derive(Default)] @@ -648,7 +525,7 @@ impl<'a> CustomPropertiesBuilder<'a> { if self.custom_properties.is_none() { self.custom_properties = Some(match self.inherited { Some(inherited) => (**inherited).clone(), - None => CustomPropertiesMap::new(), + None => CustomPropertiesMap::default(), }); } @@ -933,7 +810,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: // We have to clone the names so that we can mutably borrow the map // in the context we create for traversal. - let names = custom_properties_map.index.clone(); + let names: Vec<_> = custom_properties_map.keys().cloned().collect(); for name in names.into_iter() { let mut context = Context { count: 0, @@ -1112,7 +989,7 @@ pub fn substitute<'i>( let mut input = ParserInput::new(input); let mut input = Parser::new(&mut input); let mut position = (input.position(), first_token_type); - let empty_map = CustomPropertiesMap::new(); + let empty_map = CustomPropertiesMap::default(); let custom_properties = match computed_values_map { Some(m) => &**m, None => &empty_map, diff --git a/servo/components/style/lib.rs b/servo/components/style/lib.rs index 5c4b47193a5f..79829081b39f 100644 --- a/servo/components/style/lib.rs +++ b/servo/components/style/lib.rs @@ -48,6 +48,7 @@ extern crate hashglobe; #[cfg(feature = "servo")] #[macro_use] extern crate html5ever; +extern crate indexmap; extern crate itertools; extern crate itoa; #[macro_use] diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 89c39bbc65b2..7e88c06ab5d8 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -5792,8 +5792,8 @@ pub extern "C" fn Servo_GetCustomPropertyNameAt( None => return false, }; - let property_name = match custom_properties.get_key_at(index) { - Some(n) => n, + let property_name = match custom_properties.get_index(index as usize) { + Some((key, _value)) => key, None => return false, };