From 51e642e875ea25502b92967358e32a30e06ab3dd Mon Sep 17 00:00:00 2001 From: David Raifaizen Date: Sat, 26 Mar 2016 11:02:34 -0400 Subject: [PATCH] Converted serialization methods to implement the to_css trait, writing to string buffers to save string allocations for every result --- components/script/dom/cssstyledeclaration.rs | 5 +- components/script/dom/element.rs | 10 +- .../style/properties/properties.mako.rs | 158 ++++++++++++++---- tests/unit/style/properties.rs | 5 +- 4 files changed, 132 insertions(+), 46 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 6e1b4f79cb0..5b2207f56af 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -10,7 +10,7 @@ use dom::bindings::js::{JS, Root}; use dom::bindings::reflector::{Reflector, reflect_dom_object}; use dom::bindings::str::DOMString; use dom::element::{Element, StylePriority}; -use dom::node::{Node, NodeDamage, window_from_node}; +use dom::node::{Node, window_from_node}; use dom::window::Window; use std::ascii::AsciiExt; use string_cache::Atom; @@ -147,7 +147,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 2.3 let list = list.iter().map(|x| &*x).collect::>(); - return DOMString::from(shorthand.serialize_shorthand(&list)); + let serialized_value = shorthand.serialize_shorthand_to_string(&list); + return DOMString::from(serialized_value); } // Step 3 & 4 diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index a16bab623b5..336fff2e838 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -5,7 +5,7 @@ //! Element nodes. use app_units::Au; -use cssparser::Color; +use cssparser::{Color, ToCss}; use devtools_traits::AttrInfo; use dom::activation::Activatable; use dom::attr::AttrValue; @@ -699,12 +699,10 @@ impl Element { } fn sync_property_with_attrs_style(&self) { - let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() { - declarations.serialize() + let mut style_str = String::new(); + if let &Some(ref declarations) = &*self.style_attribute().borrow() { + declarations.to_css(&mut style_str).unwrap(); } - 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 c6daab0f598..c81be955540 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -14,6 +14,7 @@ use std::ascii::AsciiExt; use std::boxed::Box as StdBox; use std::collections::HashSet; use std::fmt; +use std::fmt::Write; use std::intrinsics; use std::mem; use std::sync::Arc; @@ -266,11 +267,12 @@ pub struct PropertyDeclarationBlock { pub normal: Arc>, } -impl PropertyDeclarationBlock { +impl ToCss for PropertyDeclarationBlock { // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block - pub fn serialize(&self) -> String { - // Step 1 - let mut result_list = String::new(); + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + let mut is_first_serialization = true; // trailing serializations should have a prepended space + + // Step 1 -> dest = result list // Step 2 let mut already_serialized = Vec::new(); @@ -326,16 +328,15 @@ impl PropertyDeclarationBlock { // TODO: serialize shorthand does not take is_important into account currently // Substep 5 - let value = shorthand.serialize_shorthand(¤t_longhands[..]); + let was_serialized = + try!(shorthand.serialize_shorthand_to_buffer(dest, ¤t_longhands[..], &mut is_first_serialization)); + // If serialization occured, Substep 7 & 8 will have been completed // Substep 6 - if value.is_empty() { + if !was_serialized { continue; } - // Substep 7 & 8 - result_list.push_str(&format!("{}: {}; ", &shorthand.name(), value)); - for current_longhand in current_longhands { // Substep 9 already_serialized.push(current_longhand.name()); @@ -353,24 +354,63 @@ impl PropertyDeclarationBlock { continue; } - // Step 3.3.5 - let mut value = declaration.value(); - if self.important.contains(declaration) { - value.push_str(" !important"); - } - // Steps 3.3.6 & 3.3.7 - result_list.push_str(&format!("{}: {}; ", &property, value)); + // Steps 3.3.5, 3.3.6 & 3.3.7 + let append_important = self.important.contains(declaration); + try!(append_serialization(dest, + &property.to_string(), + AppendableValue::Declaration(declaration), + append_important, + &mut is_first_serialization)); // Step 3.3.8 already_serialized.push(property); } - result_list.pop(); // remove trailling whitespace // Step 4 - result_list + Ok(()) } } +enum AppendableValue<'a> { + Declaration(&'a PropertyDeclaration), + 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 + + // after first serialization(key: value;) add whitespace between the pairs + if !*is_first_serialization { + try!(write!(dest, " ")); + } + else { + *is_first_serialization = false; + } + + try!(write!(dest, "{}:", property_name)); + + match appendable_value { + AppendableValue::Declaration(decl) => { + if !decl.value_is_unparsed() { + // for normal parsed values, add a space between key: and value + try!(write!(dest, " ")); + } + + try!(decl.to_css(dest)); + }, + AppendableValue::Css(css) => try!(write!(dest, "{}", css)) + }; + + if is_important { + try!(write!(dest, " !important")); + } + + write!(dest, ";") +} + pub fn parse_style_attribute(input: &str, base_url: &Url, error_reporter: StdBox, extra_data: ParserContextExtraData) -> PropertyDeclarationBlock { @@ -509,8 +549,6 @@ pub enum Shorthand { % endfor } -use util::str::str_join; - impl Shorthand { pub fn from_name(name: &str) -> Option { match_ignore_ascii_case! { name, @@ -544,25 +582,48 @@ impl Shorthand { } } - pub fn serialize_shorthand(self, declarations: &[&PropertyDeclaration]) -> String { + /// Serializes possible shorthand value to String. + pub fn serialize_shorthand_to_string(self, declarations: &[&PropertyDeclaration]) -> String { + let mut result = String::new(); + self.serialize_shorthand_to_buffer(&mut result, declarations, &mut true).unwrap(); + result + } + + /// 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: &[&PropertyDeclaration], + is_first_serialization: &mut bool) + -> Result where W: Write { + + let property_name = self.name(); + // https://drafts.csswg.org/css-variables/#variables-in-shorthands if let Some(css) = declarations[0].with_variables_from_shorthand(self) { if declarations[1..] .iter() .all(|d| d.with_variables_from_shorthand(self) == Some(css)) { - css.to_owned() + + append_serialization( + dest, property_name, AppendableValue::Css(css), false, is_first_serialization + ).and_then(|_| Ok(true)) } else { - String::new() + Ok(false) } } else { if declarations.iter().any(|d| d.with_variables()) { - String::new() + Ok(false) } else { - let str_iter = declarations.iter().map(|d| d.value()); + for declaration in declarations.iter() { + try!(append_serialization( + dest, property_name, AppendableValue::Declaration(declaration), false, is_first_serialization + )); + } // FIXME: this needs property-specific code, which probably should be in style/ // "as appropriate according to the grammar of shorthand " // https://drafts.csswg.org/cssom/#serialize-a-css-value - str_join(str_iter, " ") + Ok(true) } } } @@ -647,6 +708,22 @@ impl fmt::Display for PropertyDeclarationName { } } } +impl ToCss for PropertyDeclaration { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + % for property in LONGHANDS: + % if property.derived_from is None: + PropertyDeclaration::${property.camel_case}(ref value) => + value.to_css(dest), + % endif + % endfor + PropertyDeclaration::Custom(_, ref value) => value.to_css(dest), + % if any(property.derived_from for property in data.longhands): + _ => Err(fmt::Error), + % endif + } + } +} impl PropertyDeclaration { pub fn name(&self) -> PropertyDeclarationName { @@ -666,17 +743,12 @@ impl PropertyDeclaration { } pub fn value(&self) -> String { - match *self { - % for property in data.longhands: - PropertyDeclaration::${property.camel_case} - % if not property.derived_from: - (ref value) => value.to_css_string(), - % else: - (_) => panic!("unsupported property declaration: ${property.name}"), - % endif - % endfor - PropertyDeclaration::Custom(_, ref value) => value.to_css_string(), + let mut value = String::new(); + if let Err(_) = self.to_css(&mut value) { + panic!("unsupported property declaration: {}", self.name()); } + + value } /// If this is a pending-substitution value from the given shorthand, return that value @@ -714,6 +786,20 @@ impl PropertyDeclaration { } } + /// Return whether the value is stored as it was in the CSS source, preserving whitespace + /// (as opposed to being parsed into a more abstract data structure). + /// This is the case of custom properties and values that contain unsubstituted variables. + pub fn value_is_unparsed(&self) -> bool { + match *self { + % for property in LONGHANDS: + PropertyDeclaration::${property.camel_case}(ref value) => { + matches!(*value, DeclaredValue::WithVariables { .. }) + }, + % endfor + PropertyDeclaration::Custom(..) => true + } + } + pub fn matches(&self, name: &str) -> bool { match *self { % for property in data.longhands: diff --git a/tests/unit/style/properties.rs b/tests/unit/style/properties.rs index 7e745dd0993..3e8ef23a108 100644 --- a/tests/unit/style/properties.rs +++ b/tests/unit/style/properties.rs @@ -2,6 +2,7 @@ * 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::ToCss; use rustc_serialize::json::Json; use std::env; use std::fs::{File, remove_file}; @@ -83,10 +84,10 @@ fn property_declaration_block_should_serialize_correctly() { important: Arc::new(important) }; - let css_string = block.serialize(); + let css_string = block.to_css_string(); assert_eq!( css_string, - "width: 70px; min-height: 20px; display: inline-block; height: 20px ! important;" + "width: 70px; min-height: 20px; display: inline-block; height: 20px !important;" ); }