From dc829da07ef705f5b76315c9746362a7c18354c9 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 4 Apr 2016 21:14:57 +0200 Subject: [PATCH] Less cloning and dynamic dispatch. --- components/script/dom/cssstyledeclaration.rs | 16 +++- components/script/dom/element.rs | 9 ++- .../style/properties/properties.mako.rs | 78 +++++++++++-------- 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index bcc569cbf6d..410c7757f5b 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -13,6 +13,8 @@ use dom::element::{Element, StylePriority}; use dom::node::{Node, window_from_node}; use dom::window::Window; use std::ascii::AsciiExt; +use std::cell::Ref; +use std::slice; use string_cache::Atom; use style::parser::ParserContextExtraData; use style::properties::{PropertyDeclaration, Shorthand}; @@ -140,14 +142,22 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 2.2.2 & 2.2.3 match declaration { - Some(declaration) => list.push(declaration.clone()), + Some(declaration) => list.push(declaration), None => return DOMString::new(), } } // Step 2.3 - let mut list = list.iter().map(|x| &*x); - let serialized_value = shorthand.serialize_shorthand_to_string(&mut list); + // Work around closures not being Clone + #[derive(Clone)] + struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, PropertyDeclaration>>); + impl<'a, 'b> Iterator for Map<'a, 'b> { + type Item = &'a PropertyDeclaration; + fn next(&mut self) -> Option { + self.0.next().map(|r| &**r) + } + } + let serialized_value = shorthand.serialize_shorthand_to_string(Map(list.iter())); return DOMString::from(serialized_value); } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 336fff2e838..6dddec31108 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -699,10 +699,11 @@ impl Element { } fn sync_property_with_attrs_style(&self) { - let mut style_str = String::new(); - if let &Some(ref declarations) = &*self.style_attribute().borrow() { - declarations.to_css(&mut style_str).unwrap(); - } + let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() { + declarations.to_css_string() + } else { + String::new() + }; let new_style = AttrValue::String(style_str); diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 3927adc3dac..adb0054c4ab 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -318,11 +318,11 @@ impl ToCss for PropertyDeclarationBlock { let mut current_longhands = Vec::new(); let mut important_count = 0; - for longhand in longhands.iter() { - let longhand_name = longhand.0.name(); + for &(longhand, longhand_important) in longhands.iter() { + let longhand_name = longhand.name(); if properties.iter().any(|p| &longhand_name == *p) { - current_longhands.push(longhand.0); - if longhand.1 == true { + current_longhands.push(longhand); + if longhand_important { important_count += 1; } } @@ -348,7 +348,7 @@ impl ToCss for PropertyDeclarationBlock { try!( shorthand.serialize_shorthand_to_buffer( dest, - &mut current_longhands.iter().cloned(), + current_longhands.iter().cloned(), &mut is_first_serialization ) ); @@ -376,12 +376,20 @@ impl ToCss for PropertyDeclarationBlock { continue; } + use std::iter::Cloned; + use std::slice; + // Steps 3.3.5, 3.3.6 & 3.3.7 - try!(append_serialization(dest, - &property.to_string(), - AppendableValue::Declaration(declaration), - important, - &mut is_first_serialization)); + // Need to specify an iterator type here even though it’s unused to work around + // "error: unable to infer enough type information about `_`; + // type annotations or generic parameter binding required [E0282]" + // Use the same type as earlier call to reuse generated code. + try!(append_serialization::>>( + dest, + &property.to_string(), + AppendableValue::Declaration(declaration), + important, + &mut is_first_serialization)); // Step 3.3.8 already_serialized.push(property); @@ -392,17 +400,20 @@ impl ToCss for PropertyDeclarationBlock { } } -enum AppendableValue<'a> { +enum AppendableValue<'a, I> +where I: Iterator { Declaration(&'a PropertyDeclaration), - DeclarationsForShorthand(&'a mut Iterator), + DeclarationsForShorthand(I), Css(&'a str) } -fn append_serialization(dest: &mut W, - property_name: &str, - appendable_value: AppendableValue, - is_important: bool, - is_first_serialization: &mut bool) -> fmt::Result where W: fmt::Write { +fn append_serialization<'a, W, I>(dest: &mut W, + property_name: &str, + appendable_value: AppendableValue<'a, I>, + is_important: bool, + is_first_serialization: &mut bool) + -> fmt::Result + where W: fmt::Write, I: Iterator { // after first serialization(key: value;) add whitespace between the pairs if !*is_first_serialization { @@ -611,7 +622,8 @@ impl Shorthand { } /// Serializes possible shorthand value to String. - pub fn serialize_shorthand_to_string(self, declarations: &mut Iterator) -> String { + pub fn serialize_shorthand_to_string<'a, I>(self, declarations: I) -> String + where I: Iterator + Clone { let mut result = String::new(); self.serialize_shorthand_to_buffer(&mut result, declarations, &mut true).unwrap(); result @@ -620,28 +632,28 @@ impl Shorthand { /// Serializes possible shorthand value to input buffer given a list of longhand declarations. /// On success, returns true if shorthand value is written and false if no shorthand value is present. - pub fn serialize_shorthand_to_buffer(self, - dest: &mut W, - declarations: &mut Iterator, - is_first_serialization: &mut bool) - -> Result where W: Write { + pub fn serialize_shorthand_to_buffer<'a, W, I>(self, + dest: &mut W, + declarations: I, + is_first_serialization: &mut bool) + -> Result + where W: Write, I: Iterator + Clone { - // FIXME: I know that creating this list here is wrong, but I couldn't get it to compile otherwise - // using plain iterators, need advice! - let declaration_list: Vec<_> = declarations.cloned().collect(); - let mut declarations = declaration_list.iter(); + // Only cloning iterators (a few pointers each) not declarations. + let mut declarations2 = declarations.clone(); + let mut declarations3 = declarations.clone(); - let first_declaration = match declarations.next() { + let first_declaration = match declarations2.next() { Some(declaration) => declaration, None => return Ok(false) }; - let property_name = &self.name(); + let property_name = self.name(); // https://drafts.csswg.org/css-variables/#variables-in-shorthands if let Some(css) = first_declaration.with_variables_from_shorthand(self) { - if declarations.all(|d| d.with_variables_from_shorthand(self) == Some(css)) { - return append_serialization( + if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) { + return append_serialization::( dest, property_name, AppendableValue::Css(css), false, is_first_serialization ).and_then(|_| Ok(true)); } @@ -650,12 +662,12 @@ impl Shorthand { } } - if !declaration_list.iter().any(|d| d.with_variables()) { + if !declarations3.any(|d| d.with_variables()) { try!( append_serialization( dest, property_name, - AppendableValue::DeclarationsForShorthand(&mut declaration_list.iter()), + AppendableValue::DeclarationsForShorthand(declarations), false, is_first_serialization )