From feff32c4ab97abb33b0c0ad83aa85af75ee378f0 Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Mon, 29 Feb 2016 09:19:03 -0500
Subject: [PATCH 01/14] For Shorthand impl, added to_name method and copied
 serialize_shorthand from cssstyledeclaration.rs file

For PropertyDeclaration impl, added shorthands method to get the shorthands associated
with the current longhand

For PropertyDeclarationBlock impl, added serialize algorithm
---
 .../style/properties/properties.mako.rs       | 186 ++++++++++++++++++
 1 file changed, 186 insertions(+)

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index ec6fe6562e05..922735894136 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -255,6 +255,10 @@ mod property_bit_field {
 
 /// Declarations are stored in reverse order.
 /// Overridden declarations are skipped.
+// TODO: Because normal & important are stored in seperate lists, it is impossible to know their
+// exact declaration order. They should be changed into one list, adding an important/normal
+// flag to PropertyDeclaration
+
 #[derive(Debug, PartialEq, HeapSizeOf)]
 pub struct PropertyDeclarationBlock {
     #[ignore_heap_size_of = "#7038"]
@@ -263,6 +267,121 @@ pub struct PropertyDeclarationBlock {
     pub normal: Arc<Vec<PropertyDeclaration>>,
 }
 
+impl PropertyDeclarationBlock {
+    // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
+    pub fn serialize(&self) -> String {
+        // Step 1
+        let mut result_list = String::new();
+
+        // Step 2
+        let mut already_serialized = HashSet::new();
+
+        // Step 3
+        // restore order of declarations since PropertyDeclarationBlock is stored in reverse order
+        let declarations = self.normal.iter().rev().chain(self.important.iter().rev()).collect::<Vec<_>>();
+
+
+        for declaration in &declarations {
+            // Step 3.1
+            let property = declaration.name().to_string();
+
+            // Step 3.2
+            if already_serialized.contains(&property) {
+                continue;
+            }
+
+            // Step 3.3
+            let mut shorthands = Vec::from(declaration.shorthands());
+            if shorthands.len() > 0 {
+                shorthands.sort_by(|a,b| a.longhands().len().cmp(&b.longhands().len()));
+
+                // Step 3.3.1
+                let mut longhands = declarations.iter().cloned()
+                .filter(|d| !already_serialized.contains(&d.name().to_string()))
+                    .collect::<Vec<_>>();
+
+                // Step 3.3.2
+                for shorthand in shorthands {
+                    let properties = shorthand.longhands();
+
+                    // Substep 2 & 3
+                    let mut current_longhands = Vec::new();
+                    let mut missing_properties: HashSet<_> = HashSet::from_iter(
+                        properties.iter().map(|&s| s.to_owned())
+                    );
+
+                    for longhand in longhands.iter().cloned() {
+                        let longhand_string = longhand.name().to_string();
+                        if !properties.contains(&&*longhand_string) {
+                            continue;
+                        }
+
+                        missing_properties.remove(&longhand_string);
+                        current_longhands.push(longhand);
+                    }
+
+                    // Substep 1
+                    if current_longhands.len() == 0 || missing_properties.len() > 0 {
+                        continue;
+                    }
+
+                    // Substep 4
+                    let important_count = current_longhands.iter()
+                        .filter(|l| self.important.contains(l))
+                        .count();
+
+                    let is_important = important_count > 0;
+                    if is_important && important_count != current_longhands.len() {
+                        continue;
+                    }
+
+                    // TODO: serialize shorthand does not take is_important into account currently
+                    // Substep 5
+                    let value = shorthand.serialize_shorthand(&current_longhands[..]);
+
+                    // Substep 6
+                    if value.is_empty() {
+                        continue;
+                    }
+
+                    // Substep 7 & 8
+                    result_list.push_str(&format!("{}: {}; ", &shorthand.to_name(), value));
+
+                    for current_longhand in current_longhands {
+                        // Substep 9
+                        already_serialized.insert(current_longhand.name().to_string());
+                        let index_to_remove = longhands.iter().position(|l| l == &current_longhand);
+                        if let Some(index) = index_to_remove {
+                            // Substep 10
+                            longhands.remove(index);
+                        }
+                     }
+                 }
+            }
+
+            // Step 3.3.4
+            if already_serialized.contains(&property) {
+                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));
+
+            // Step 3.3.8
+            already_serialized.insert(property);
+        }
+
+        result_list.pop(); // remove trailling whitespace
+        // Step 4
+        result_list
+    }
+}
+
 pub fn parse_style_attribute(input: &str, base_url: &Url, error_reporter: StdBox<ParseErrorReporter + Send>,
                              extra_data: ParserContextExtraData)
                              -> PropertyDeclarationBlock {
@@ -401,6 +520,10 @@ pub enum Shorthand {
     % endfor
 }
 
+use std::borrow::ToOwned;
+use std::iter::FromIterator;
+use util::str::str_join;
+
 impl Shorthand {
     pub fn from_name(name: &str) -> Option<Shorthand> {
         match_ignore_ascii_case! { name,
@@ -411,6 +534,14 @@ impl Shorthand {
         }
     }
 
+    pub fn to_name(&self) -> &str {
+        match *self {
+            % for property in SHORTHANDS:
+                Shorthand::${property.camel_case} => "${property.name}",
+            % endfor
+        }
+    }
+
     pub fn longhands(&self) -> &'static [&'static str] {
         % for property in data.shorthands:
             static ${property.ident.upper()}: &'static [&'static str] = &[
@@ -425,6 +556,29 @@ impl Shorthand {
             % endfor
         }
     }
+
+    pub fn serialize_shorthand(self, declarations: &[&PropertyDeclaration]) -> String {
+        // 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()
+            } else {
+                String::new()
+            }
+        } else {
+            if declarations.iter().any(|d| d.with_variables()) {
+                String::new()
+            } else {
+                let str_iter = declarations.iter().map(|d| d.value());
+                // 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, " ")
+            }
+        }
+    }
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, HeapSizeOf)]
@@ -680,6 +834,38 @@ impl PropertyDeclaration {
             _ => PropertyDeclarationParseResult::UnknownProperty
         }
     }
+
+    pub fn shorthands(&self) -> &[Shorthand] {
+        // first generate longhand to shorthands lookup map
+        <%
+            longhand_to_shorthand_map = {}
+            for shorthand in SHORTHANDS:
+                for sub_property in shorthand.sub_properties:
+                    if sub_property.ident not in longhand_to_shorthand_map:
+                        longhand_to_shorthand_map[sub_property.ident] = []
+
+                    longhand_to_shorthand_map[sub_property.ident].append(shorthand.camel_case)
+
+            for shorthand_list in longhand_to_shorthand_map.itervalues():
+                shorthand_list.sort()
+        %>
+
+        // based on lookup results for each longhand, create result arrays
+        % for property in LONGHANDS:
+            static ${property.ident.upper()}: &'static [Shorthand] = &[
+                % for shorthand in longhand_to_shorthand_map.get(property.ident, []):
+                    Shorthand::${shorthand},
+                % endfor
+            ];
+        % endfor
+
+        match *self {
+            % for property in LONGHANDS:
+                PropertyDeclaration::${property.camel_case}(_) => ${property.ident.upper()},
+            % endfor
+            _ => &[] // include outlet for Custom enum value
+        }
+    }
 }
 
 pub mod style_struct_traits {

From b4d8f3ba3ab8d971ed0b7943ca19b630ed2998ec Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Mon, 29 Feb 2016 09:22:40 -0500
Subject: [PATCH 02/14] Removed previously copied serialize_shorthand method as
 well as unnecessary calls to document.content_changed

---
 components/script/dom/cssstyledeclaration.rs | 38 ++------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index df9251f3888b..6e1b4f79cb07 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -13,14 +13,11 @@ use dom::element::{Element, StylePriority};
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
-use std::borrow::ToOwned;
-use std::cell::Ref;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
 use style::properties::{PropertyDeclaration, Shorthand};
 use style::properties::{is_supported_property, parse_one_declaration};
 use style::selector_impl::PseudoElement;
-use util::str::str_join;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
 #[dom_struct]
@@ -50,29 +47,6 @@ macro_rules! css_properties(
     );
 );
 
-fn serialize_shorthand(shorthand: Shorthand, declarations: &[Ref<PropertyDeclaration>]) -> String {
-    // https://drafts.csswg.org/css-variables/#variables-in-shorthands
-    if let Some(css) = declarations[0].with_variables_from_shorthand(shorthand) {
-        if declarations[1..]
-               .iter()
-               .all(|d| d.with_variables_from_shorthand(shorthand) == Some(css)) {
-            css.to_owned()
-        } else {
-            String::new()
-        }
-    } else {
-        if declarations.iter().any(|d| d.with_variables()) {
-            String::new()
-        } else {
-            let str_iter = declarations.iter().map(|d| d.value());
-            // 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, " ")
-        }
-    }
-}
-
 impl CSSStyleDeclaration {
     pub fn new_inherited(owner: &Element,
                          pseudo: Option<PseudoElement>,
@@ -166,13 +140,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
                 // Step 2.2.2 & 2.2.3
                 match declaration {
-                    Some(declaration) => list.push(declaration),
+                    Some(declaration) => list.push(declaration.clone()),
                     None => return DOMString::new(),
                 }
             }
 
             // Step 2.3
-            return DOMString::from(serialize_shorthand(shorthand, &list));
+            let list = list.iter().map(|x| &*x).collect::<Vec<_>>();
+            return DOMString::from(shorthand.serialize_shorthand(&list));
         }
 
         // Step 3 & 4
@@ -260,8 +235,6 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             element.update_inline_style(decl, priority);
         }
 
-        let node = element.upcast::<Node>();
-        node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }
 
@@ -294,8 +267,6 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             None => element.set_inline_style_property_priority(&[&*property], priority),
         }
 
-        let node = element.upcast::<Node>();
-        node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }
 
@@ -330,9 +301,6 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             None => elem.remove_inline_style_property(&property),
         }
 
-        let node = elem.upcast::<Node>();
-        node.dirty(NodeDamage::NodeStyleDamaged);
-
         // Step 6
         Ok(value)
     }

From 839a7559e78d5db014106dd3dbdb855d989aa054 Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Mon, 29 Feb 2016 09:24:03 -0500
Subject: [PATCH 03/14] Added sync_property_with_attrs_style method to
 serialize style string when inline style is changed

---
 components/script/dom/element.rs | 164 +++++++++++++++++++------------
 1 file changed, 102 insertions(+), 62 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index f58320892024..295a74f3b98c 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -698,89 +698,129 @@ impl Element {
         }
     }
 
+    fn sync_property_with_attrs_style(&self) {
+        let mut style_str = String::new();
+
+        if let &Some(ref declarations) = &*self.style_attribute().borrow() {
+            style_str.push_str(&declarations.serialize());
+        }
+
+        let new_style = AttrValue::String(style_str);
+
+        if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &atom!("style")) {
+            style_attr.set_value(new_style, self);
+            return;
+        }
+
+        self.push_new_attribute(
+            atom!("style"),
+            new_style,
+            atom!("style"),
+            ns!(),
+            Some(atom!("style"))
+        );
+    }
+
     pub fn remove_inline_style_property(&self, property: &str) {
-        let mut inline_declarations = self.style_attribute.borrow_mut();
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-            let index = declarations.normal
-                                    .iter()
-                                    .position(|decl| decl.matches(property));
-            if let Some(index) = index {
-                Arc::make_mut(&mut declarations.normal).remove(index);
-                return;
-            }
+        fn remove(element: &Element, property: &str) {
+            let mut inline_declarations = element.style_attribute.borrow_mut();
+            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+                let index = declarations.normal
+                                        .iter()
+                                        .position(|decl| decl.matches(property));
+                if let Some(index) = index {
+                    Arc::make_mut(&mut declarations.normal).remove(index);
+                    return;
+                }
 
-            let index = declarations.important
-                                    .iter()
-                                    .position(|decl| decl.matches(property));
-            if let Some(index) = index {
-                Arc::make_mut(&mut declarations.important).remove(index);
-                return;
+                let index = declarations.important
+                                        .iter()
+                                        .position(|decl| decl.matches(property));
+                if let Some(index) = index {
+                    Arc::make_mut(&mut declarations.important).remove(index);
+                    return;
+                }
             }
         }
+
+        remove(self, property);
+        self.sync_property_with_attrs_style();
     }
 
     pub fn update_inline_style(&self,
                                property_decl: PropertyDeclaration,
                                style_priority: StylePriority) {
-        let mut inline_declarations = self.style_attribute().borrow_mut();
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-            let existing_declarations = if style_priority == StylePriority::Important {
-                &mut declarations.important
-            } else {
-                &mut declarations.normal
-            };
 
-            // Usually, the reference count will be 1 here. But transitions could make it greater
-            // than that.
-            let existing_declarations = Arc::make_mut(existing_declarations);
-            for declaration in &mut *existing_declarations {
-                if declaration.name() == property_decl.name() {
-                    *declaration = property_decl;
-                    return;
+        fn update(element: &Element, property_decl: PropertyDeclaration, style_priority: StylePriority) {
+            let mut inline_declarations = element.style_attribute().borrow_mut();
+            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+                let existing_declarations = if style_priority == StylePriority::Important {
+                    &mut declarations.important
+                } else {
+                    &mut declarations.normal
+                };
+
+                // Usually, the reference count will be 1 here. But transitions could make it greater
+                // than that.
+                let existing_declarations = Arc::make_mut(existing_declarations);
+                for declaration in &mut *existing_declarations {
+                    if declaration.name() == property_decl.name() {
+                        *declaration = property_decl;
+                        return;
+                    }
                 }
+
+                // inserting instead of pushing since the declarations are in reverse order
+                existing_declarations.insert(0, property_decl);
+                return;
             }
-            existing_declarations.push(property_decl);
-            return;
-        }
 
-        let (important, normal) = if style_priority == StylePriority::Important {
-            (vec![property_decl], vec![])
-        } else {
-            (vec![], vec![property_decl])
-        };
+            let (important, normal) = if style_priority == StylePriority::Important {
+                (vec![property_decl], vec![])
+            } else {
+                (vec![], vec![property_decl])
+            };
 
-        *inline_declarations = Some(PropertyDeclarationBlock {
-            important: Arc::new(important),
-            normal: Arc::new(normal),
-        });
+            *inline_declarations = Some(PropertyDeclarationBlock {
+                important: Arc::new(important),
+                normal: Arc::new(normal),
+            });
+        }
+
+        update(self, property_decl, style_priority);
+        self.sync_property_with_attrs_style();
     }
 
     pub fn set_inline_style_property_priority(&self,
                                               properties: &[&str],
                                               style_priority: StylePriority) {
-        let mut inline_declarations = self.style_attribute().borrow_mut();
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-            let (from, to) = if style_priority == StylePriority::Important {
-                (&mut declarations.normal, &mut declarations.important)
-            } else {
-                (&mut declarations.important, &mut declarations.normal)
-            };
-
-            // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
-            // could make them greater than that.
-            let from = Arc::make_mut(from);
-            let to = Arc::make_mut(to);
-            let mut new_from = Vec::new();
-            for declaration in from.drain(..) {
-                let name = declaration.name();
-                if properties.iter().any(|p| name == **p) {
-                    to.push(declaration)
-                } else {
-                    new_from.push(declaration)
-                }
+        {
+            let mut inline_declarations = self.style_attribute().borrow_mut();
+            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+              let (from, to) = if style_priority == StylePriority::Important {
+                  (&mut declarations.normal, &mut declarations.important)
+              } else {
+                  (&mut declarations.important, &mut declarations.normal)
+              };
+
+              // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
+              // could make them greater than that.
+              let from = Arc::make_mut(from);
+              let to = Arc::make_mut(to);
+              let mut new_from = Vec::new();
+              for declaration in from.drain(..) {
+                  let name = declaration.name();
+                  if properties.iter().any(|p| name == **p) {
+                      to.push(declaration)
+                  } else {
+                      new_from.push(declaration)
+                  }
+              }
+              mem::replace(from, new_from);
             }
-            mem::replace(from, new_from);
         }
+
+        self.sync_property_with_attrs_style();
     }
 
     pub fn get_inline_style_declaration(&self,

From 47279179fa045a3f0206304896d7bb4154cdc91c Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Mon, 29 Feb 2016 09:24:33 -0500
Subject: [PATCH 04/14] Added test for serialize method

---
 tests/unit/style/properties.rs | 39 ++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tests/unit/style/properties.rs b/tests/unit/style/properties.rs
index 9f4c3f08af01..7e745dd09936 100644
--- a/tests/unit/style/properties.rs
+++ b/tests/unit/style/properties.rs
@@ -7,6 +7,10 @@ use std::env;
 use std::fs::{File, remove_file};
 use std::path::Path;
 use std::process::Command;
+use std::sync::Arc;
+use style::computed_values::display::T::inline_block;
+use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue};
+use style::values::specified::{Length, LengthOrPercentageOrAuto, LengthOrPercentage};
 
 #[test]
 fn properties_list_json() {
@@ -51,3 +55,38 @@ fn find_python() -> String {
         "python"
     }.to_owned()
 }
+
+#[test]
+fn property_declaration_block_should_serialize_correctly() {
+    let mut normal = Vec::new();
+    let mut important = Vec::new();
+
+    let length = LengthOrPercentageOrAuto::Length(Length::from_px(70f32));
+    let value = DeclaredValue::Value(length);
+    normal.push(PropertyDeclaration::Width(value));
+
+    let min_height = LengthOrPercentage::Length(Length::from_px(20f32));
+    let value = DeclaredValue::Value(min_height);
+    normal.push(PropertyDeclaration::MinHeight(value));
+
+    let value = DeclaredValue::Value(inline_block);
+    normal.push(PropertyDeclaration::Display(value));
+
+    let height = LengthOrPercentageOrAuto::Length(Length::from_px(20f32));
+    let value = DeclaredValue::Value(height);
+    important.push(PropertyDeclaration::Height(value));
+
+    normal.reverse();
+    important.reverse();
+    let block = PropertyDeclarationBlock {
+        normal: Arc::new(normal),
+        important: Arc::new(important)
+    };
+
+    let css_string = block.serialize();
+
+    assert_eq!(
+        css_string,
+        "width: 70px; min-height: 20px; display: inline-block; height: 20px ! important;"
+    );
+}

From 0985d7563f2b310fd396693b196d55303ec25f1b Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Wed, 2 Mar 2016 23:46:09 -0500
Subject: [PATCH 05/14] Making fixes based on suggestions from nox and
 SimonSapin

---
 components/script/dom/element.rs              |  9 +--
 .../style/properties/properties.mako.rs       | 55 +++++++------------
 2 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 295a74f3b98c..a16bab623b5c 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -699,11 +699,12 @@ impl Element {
     }
 
     fn sync_property_with_attrs_style(&self) {
-        let mut style_str = String::new();
-
-        if let &Some(ref declarations) = &*self.style_attribute().borrow() {
-            style_str.push_str(&declarations.serialize());
+        let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() {
+            declarations.serialize()
         }
+        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 922735894136..c6daab0f5984 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -255,10 +255,9 @@ mod property_bit_field {
 
 /// Declarations are stored in reverse order.
 /// Overridden declarations are skipped.
-// TODO: Because normal & important are stored in seperate lists, it is impossible to know their
-// exact declaration order. They should be changed into one list, adding an important/normal
-// flag to PropertyDeclaration
 
+
+// FIXME (https://github.com/servo/servo/issues/3426)
 #[derive(Debug, PartialEq, HeapSizeOf)]
 pub struct PropertyDeclarationBlock {
     #[ignore_heap_size_of = "#7038"]
@@ -274,16 +273,13 @@ impl PropertyDeclarationBlock {
         let mut result_list = String::new();
 
         // Step 2
-        let mut already_serialized = HashSet::new();
+        let mut already_serialized = Vec::new();
 
         // Step 3
         // restore order of declarations since PropertyDeclarationBlock is stored in reverse order
-        let declarations = self.normal.iter().rev().chain(self.important.iter().rev()).collect::<Vec<_>>();
-
-
-        for declaration in &declarations {
+        for declaration in self.important.iter().chain(self.normal.iter()).rev() {
             // Step 3.1
-            let property = declaration.name().to_string();
+            let property = declaration.name();
 
             // Step 3.2
             if already_serialized.contains(&property) {
@@ -291,13 +287,12 @@ impl PropertyDeclarationBlock {
             }
 
             // Step 3.3
-            let mut shorthands = Vec::from(declaration.shorthands());
-            if shorthands.len() > 0 {
-                shorthands.sort_by(|a,b| a.longhands().len().cmp(&b.longhands().len()));
+            let shorthands = declaration.shorthands();
+            if !shorthands.is_empty() {
 
                 // Step 3.3.1
-                let mut longhands = declarations.iter().cloned()
-                .filter(|d| !already_serialized.contains(&d.name().to_string()))
+                let mut longhands = self.important.iter().chain(self.normal.iter()).rev()
+                    .filter(|d| !already_serialized.contains(&d.name()))
                     .collect::<Vec<_>>();
 
                 // Step 3.3.2
@@ -306,22 +301,16 @@ impl PropertyDeclarationBlock {
 
                     // Substep 2 & 3
                     let mut current_longhands = Vec::new();
-                    let mut missing_properties: HashSet<_> = HashSet::from_iter(
-                        properties.iter().map(|&s| s.to_owned())
-                    );
 
-                    for longhand in longhands.iter().cloned() {
-                        let longhand_string = longhand.name().to_string();
-                        if !properties.contains(&&*longhand_string) {
-                            continue;
+                    for longhand in longhands.iter() {
+                        let longhand_name = longhand.name();
+                        if properties.iter().any(|p| &longhand_name == *p) {
+                            current_longhands.push(*longhand);
                         }
-
-                        missing_properties.remove(&longhand_string);
-                        current_longhands.push(longhand);
                     }
 
                     // Substep 1
-                    if current_longhands.len() == 0 || missing_properties.len() > 0 {
+                    if current_longhands.is_empty() || current_longhands.len() != properties.len() {
                         continue;
                     }
 
@@ -345,11 +334,11 @@ impl PropertyDeclarationBlock {
                     }
 
                     // Substep 7 & 8
-                    result_list.push_str(&format!("{}: {}; ", &shorthand.to_name(), value));
+                    result_list.push_str(&format!("{}: {}; ", &shorthand.name(), value));
 
                     for current_longhand in current_longhands {
                         // Substep 9
-                        already_serialized.insert(current_longhand.name().to_string());
+                        already_serialized.push(current_longhand.name());
                         let index_to_remove = longhands.iter().position(|l| l == &current_longhand);
                         if let Some(index) = index_to_remove {
                             // Substep 10
@@ -367,13 +356,13 @@ impl PropertyDeclarationBlock {
             // Step 3.3.5
             let mut value = declaration.value();
             if self.important.contains(declaration) {
-                value.push_str(" ! important");
+                value.push_str(" !important");
             }
             // Steps 3.3.6 & 3.3.7
             result_list.push_str(&format!("{}: {}; ", &property, value));
 
             // Step 3.3.8
-            already_serialized.insert(property);
+            already_serialized.push(property);
         }
 
         result_list.pop(); // remove trailling whitespace
@@ -520,8 +509,6 @@ pub enum Shorthand {
     % endfor
 }
 
-use std::borrow::ToOwned;
-use std::iter::FromIterator;
 use util::str::str_join;
 
 impl Shorthand {
@@ -534,7 +521,7 @@ impl Shorthand {
         }
     }
 
-    pub fn to_name(&self) -> &str {
+    pub fn name(&self) -> &'static str {
         match *self {
             % for property in SHORTHANDS:
                 Shorthand::${property.camel_case} => "${property.name}",
@@ -835,7 +822,7 @@ impl PropertyDeclaration {
         }
     }
 
-    pub fn shorthands(&self) -> &[Shorthand] {
+    pub fn shorthands(&self) -> &'static [Shorthand] {
         // first generate longhand to shorthands lookup map
         <%
             longhand_to_shorthand_map = {}
@@ -863,7 +850,7 @@ impl PropertyDeclaration {
             % for property in LONGHANDS:
                 PropertyDeclaration::${property.camel_case}(_) => ${property.ident.upper()},
             % endfor
-            _ => &[] // include outlet for Custom enum value
+            PropertyDeclaration::Custom(_, _) => &[]
         }
     }
 }

From 51e642e875ea25502b92967358e32a30e06ab3dd Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Sat, 26 Mar 2016 11:02:34 -0400
Subject: [PATCH 06/14] 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 6e1b4f79cb07..5b2207f56af3 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::<Vec<_>>();
-            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 a16bab623b5c..336fff2e838a 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 c6daab0f5984..c81be9555406 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<Vec<PropertyDeclaration>>,
 }
 
-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<W>(&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(&current_longhands[..]);
+                    let was_serialized =
+                        try!(shorthand.serialize_shorthand_to_buffer(dest, &current_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,22 +354,61 @@ 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<W>(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<ParseErrorReporter + Send>,
@@ -509,8 +549,6 @@ pub enum Shorthand {
     % endfor
 }
 
-use util::str::str_join;
-
 impl Shorthand {
     pub fn from_name(name: &str) -> Option<Shorthand> {
         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<W>(self,
+                                            dest: &mut W,
+                                            declarations: &[&PropertyDeclaration],
+                                            is_first_serialization: &mut bool)
+         -> Result<bool, fmt::Error> 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<W>(&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 7e745dd09936..3e8ef23a108c 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;"
     );
 }

From be2f70393f858f8b4f3eccca0925cecb3d8ba47b Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Sat, 2 Apr 2016 12:20:25 -0400
Subject: [PATCH 07/14] Adding more efficient way of determining if
 PropertyDeclaration is important

---
 .../style/properties/properties.mako.rs       | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index c81be9555406..301e9a3026b1 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -254,9 +254,10 @@ mod property_bit_field {
     % endif
 % endfor
 
-/// Declarations are stored in reverse order.
-/// Overridden declarations are skipped.
 
+use std::iter::{Iterator, Chain, Zip, Rev, Repeat, repeat};
+use std::slice;
+/// Overridden declarations are skipped.
 
 // FIXME (https://github.com/servo/servo/issues/3426)
 #[derive(Debug, PartialEq, HeapSizeOf)]
@@ -267,6 +268,19 @@ pub struct PropertyDeclarationBlock {
     pub normal: Arc<Vec<PropertyDeclaration>>,
 }
 
+impl PropertyDeclarationBlock {
+    /// Provides an iterator of all declarations, with indication of !important value
+    pub fn declarations(&self) -> Chain<
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>,
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>
+    > {
+        // Declarations are stored in reverse order.
+        let normal = self.normal.iter().rev().zip(repeat(false));
+        let important = self.important.iter().rev().zip(repeat(true));
+        normal.chain(important)
+    }
+}
+
 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 {
@@ -278,8 +292,7 @@ impl ToCss for PropertyDeclarationBlock {
         let mut already_serialized = Vec::new();
 
         // Step 3
-        // restore order of declarations since PropertyDeclarationBlock is stored in reverse order
-        for declaration in self.important.iter().chain(self.normal.iter()).rev() {
+        for (declaration, important) in self.declarations() {
             // Step 3.1
             let property = declaration.name();
 
@@ -293,8 +306,8 @@ impl ToCss for PropertyDeclarationBlock {
             if !shorthands.is_empty() {
 
                 // Step 3.3.1
-                let mut longhands = self.important.iter().chain(self.normal.iter()).rev()
-                    .filter(|d| !already_serialized.contains(&d.name()))
+                let mut longhands = self.declarations()
+                    .filter(|d| !already_serialized.contains(&d.0.name()))
                     .collect::<Vec<_>>();
 
                 // Step 3.3.2
@@ -303,24 +316,27 @@ impl ToCss for PropertyDeclarationBlock {
 
                     // Substep 2 & 3
                     let mut current_longhands = Vec::new();
+                    let mut important_count = 0;
 
                     for longhand in longhands.iter() {
-                        let longhand_name = longhand.name();
+                        let longhand_name = longhand.0.name();
                         if properties.iter().any(|p| &longhand_name == *p) {
-                            current_longhands.push(*longhand);
+                            current_longhands.push(longhand.0);
+                            if longhand.1 == true {
+                                important_count += 1;
+                            }
                         }
                     }
 
                     // Substep 1
+                    /* Assuming that the PropertyDeclarationBlock contains no duplicate entries,
+                    if the current_longhands length is equal to the properties length, it means
+                    that the properties that map to shorthand are present in longhands */
                     if current_longhands.is_empty() || current_longhands.len() != properties.len() {
                         continue;
                     }
 
                     // Substep 4
-                    let important_count = current_longhands.iter()
-                        .filter(|l| self.important.contains(l))
-                        .count();
-
                     let is_important = important_count > 0;
                     if is_important && important_count != current_longhands.len() {
                         continue;
@@ -340,7 +356,7 @@ impl ToCss for PropertyDeclarationBlock {
                     for current_longhand in current_longhands {
                         // Substep 9
                         already_serialized.push(current_longhand.name());
-                        let index_to_remove = longhands.iter().position(|l| l == &current_longhand);
+                        let index_to_remove = longhands.iter().position(|l| l.0 == current_longhand);
                         if let Some(index) = index_to_remove {
                             // Substep 10
                             longhands.remove(index);
@@ -355,11 +371,10 @@ impl ToCss for PropertyDeclarationBlock {
             }
 
             // 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,
+                                 important,
                                  &mut is_first_serialization));
 
             // Step 3.3.8
@@ -380,7 +395,7 @@ fn append_serialization<W>(dest: &mut W,
                            property_name: &str,
                            appendable_value: AppendableValue,
                            is_important: bool,
-                           is_first_serialization: &mut bool) -> fmt::Result where W: fmt::Write
+                           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 {

From 7d7aac212b34d7875f1a179a2cd8ef08233bda6c Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Sat, 2 Apr 2016 12:22:03 -0400
Subject: [PATCH 08/14] Shorthand serialization now takes iterators instead of
 array slice as argument

---
 components/script/dom/cssstyledeclaration.rs  |  4 +-
 .../style/properties/properties.mako.rs       | 96 ++++++++++++-------
 2 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 5b2207f56af3..bcc569cbf6d1 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -146,8 +146,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
 
             // Step 2.3
-            let list = list.iter().map(|x| &*x).collect::<Vec<_>>();
-            let serialized_value = shorthand.serialize_shorthand_to_string(&list);
+            let mut list = list.iter().map(|x| &*x);
+            let serialized_value = shorthand.serialize_shorthand_to_string(&mut list);
             return DOMString::from(serialized_value);
         }
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 301e9a3026b1..3927adc3dacd 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -345,7 +345,13 @@ impl ToCss for PropertyDeclarationBlock {
                     // TODO: serialize shorthand does not take is_important into account currently
                     // Substep 5
                     let was_serialized =
-                        try!(shorthand.serialize_shorthand_to_buffer(dest, &current_longhands[..], &mut is_first_serialization));
+                        try!(
+                            shorthand.serialize_shorthand_to_buffer(
+                                dest,
+                                &mut current_longhands.iter().cloned(),
+                                &mut is_first_serialization
+                            )
+                        );
                     // If serialization occured, Substep 7 & 8 will have been completed
 
                     // Substep 6
@@ -372,10 +378,10 @@ impl ToCss for PropertyDeclarationBlock {
 
             // 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));
+                                      &property.to_string(),
+                                      AppendableValue::Declaration(declaration),
+                                      important,
+                                      &mut is_first_serialization));
 
             // Step 3.3.8
             already_serialized.push(property);
@@ -388,6 +394,7 @@ impl ToCss for PropertyDeclarationBlock {
 
 enum AppendableValue<'a> {
     Declaration(&'a PropertyDeclaration),
+    DeclarationsForShorthand(&'a mut Iterator<Item=&'a PropertyDeclaration>),
     Css(&'a str)
 }
 
@@ -408,6 +415,7 @@ fn append_serialization<W>(dest: &mut W,
     try!(write!(dest, "{}:", property_name));
 
     match appendable_value {
+        AppendableValue::Css(css) => try!(write!(dest, " {}", css)),
         AppendableValue::Declaration(decl) => {
             if !decl.value_is_unparsed() {
                 // for normal parsed values, add a space between key: and value
@@ -416,8 +424,13 @@ fn append_serialization<W>(dest: &mut W,
 
             try!(decl.to_css(dest));
          },
-        AppendableValue::Css(css) =>  try!(write!(dest, "{}", css))
-    };
+         AppendableValue::DeclarationsForShorthand(decls) => {
+             for decl in decls {
+                 try!(write!(dest, " "));
+                 try!(decl.to_css(dest));
+             }
+         }
+    }
 
     if is_important {
         try!(write!(dest, " !important"));
@@ -598,49 +611,62 @@ impl Shorthand {
     }
 
     /// Serializes possible shorthand value to String.
-    pub fn serialize_shorthand_to_string(self, declarations: &[&PropertyDeclaration]) -> String {
+    pub fn serialize_shorthand_to_string(self, declarations: &mut Iterator<Item=&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<W>(self,
                                             dest: &mut W,
-                                            declarations: &[&PropertyDeclaration],
+                                            declarations: &mut Iterator<Item=&PropertyDeclaration>,
                                             is_first_serialization: &mut bool)
          -> Result<bool, fmt::Error> where W: Write {
 
-        let property_name = self.name();
+        // 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();
+
+        let first_declaration = match declarations.next() {
+            Some(declaration) => declaration,
+            None => return Ok(false)
+        };
+
+        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)) {
-
-                       append_serialization(
-                           dest, property_name, AppendableValue::Css(css), false, is_first_serialization
-                       ).and_then(|_| Ok(true))
-            } else {
-                Ok(false)
-            }
-        } else {
-            if declarations.iter().any(|d| d.with_variables()) {
-                Ok(false)
-            } else {
-                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
-                Ok(true)
-            }
+        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(
+                   dest, property_name, AppendableValue::Css(css), false, is_first_serialization
+               ).and_then(|_| Ok(true));
+           }
+           else {
+               return Ok(false);
+           }
         }
+
+        if !declaration_list.iter().any(|d| d.with_variables()) {
+            try!(
+                append_serialization(
+                    dest,
+                    property_name,
+                    AppendableValue::DeclarationsForShorthand(&mut declaration_list.iter()),
+                    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
+            return Ok(true);
+        }
+
+        Ok(false)
     }
 }
 

From dc829da07ef705f5b76315c9746362a7c18354c9 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Mon, 4 Apr 2016 21:14:57 +0200
Subject: [PATCH 09/14] Less cloning and dynamic dispatch.

---
 components/script/dom/cssstyledeclaration.rs  | 16 +++-
 components/script/dom/element.rs              |  9 +-
 .../style/properties/properties.mako.rs       | 82 +++++++++++--------
 3 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index bcc569cbf6d1..410c7757f5ba 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::Item> {
+                    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 336fff2e838a..6dddec311083 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 3927adc3dacd..adb0054c4ab8 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::<W, Cloned<slice::Iter< &PropertyDeclaration>>>(
+                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<Item=&'a PropertyDeclaration> {
     Declaration(&'a PropertyDeclaration),
-    DeclarationsForShorthand(&'a mut Iterator<Item=&'a PropertyDeclaration>),
+    DeclarationsForShorthand(I),
     Css(&'a str)
 }
 
-fn append_serialization<W>(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<Item=&'a PropertyDeclaration> {
 
     // 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<Item=&PropertyDeclaration>) -> String {
+    pub fn serialize_shorthand_to_string<'a, I>(self, declarations: I) -> String
+    where I: Iterator<Item=&'a PropertyDeclaration> + 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<W>(self,
-                                            dest: &mut W,
-                                            declarations: &mut Iterator<Item=&PropertyDeclaration>,
-                                            is_first_serialization: &mut bool)
-         -> Result<bool, fmt::Error> where W: Write {
-
-        // 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();
-
-        let first_declaration = match declarations.next() {
+    pub fn serialize_shorthand_to_buffer<'a, W, I>(self,
+                                                   dest: &mut W,
+                                                   declarations: I,
+                                                   is_first_serialization: &mut bool)
+                                                   -> Result<bool, fmt::Error>
+    where W: Write, I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+
+        // Only cloning iterators (a few pointers each) not declarations.
+        let mut declarations2 = declarations.clone();
+        let mut declarations3 = declarations.clone();
+
+        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::<W, I>(
                    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
                 )

From f38607967e080e4feb2c4400edc6a258493f725d Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Mon, 18 Apr 2016 21:18:51 -0400
Subject: [PATCH 10/14] Fixing python to_css codegen error

---
 components/style/properties/properties.mako.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index adb0054c4ab8..af6aec68307a 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -600,7 +600,7 @@ impl Shorthand {
 
     pub fn name(&self) -> &'static str {
         match *self {
-            % for property in SHORTHANDS:
+            % for property in data.shorthands:
                 Shorthand::${property.camel_case} => "${property.name}",
             % endfor
         }
@@ -764,8 +764,8 @@ impl fmt::Display for PropertyDeclarationName {
 impl ToCss for PropertyDeclaration {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
-            % for property in LONGHANDS:
-                % if property.derived_from is None:
+            % for property in data.longhands:
+                % if not property.derived_from:
                     PropertyDeclaration::${property.camel_case}(ref value) =>
                         value.to_css(dest),
                 % endif
@@ -844,7 +844,7 @@ impl PropertyDeclaration {
     /// 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:
+          % for property in data.longhands:
               PropertyDeclaration::${property.camel_case}(ref value) => {
                   matches!(*value, DeclaredValue::WithVariables { .. })
               },
@@ -965,7 +965,7 @@ impl PropertyDeclaration {
         // first generate longhand to shorthands lookup map
         <%
             longhand_to_shorthand_map = {}
-            for shorthand in SHORTHANDS:
+            for shorthand in data.shorthands:
                 for sub_property in shorthand.sub_properties:
                     if sub_property.ident not in longhand_to_shorthand_map:
                         longhand_to_shorthand_map[sub_property.ident] = []
@@ -977,7 +977,7 @@ impl PropertyDeclaration {
         %>
 
         // based on lookup results for each longhand, create result arrays
-        % for property in LONGHANDS:
+        % for property in data.longhands:
             static ${property.ident.upper()}: &'static [Shorthand] = &[
                 % for shorthand in longhand_to_shorthand_map.get(property.ident, []):
                     Shorthand::${shorthand},
@@ -986,7 +986,7 @@ impl PropertyDeclaration {
         % endfor
 
         match *self {
-            % for property in LONGHANDS:
+            % for property in data.longhands:
                 PropertyDeclaration::${property.camel_case}(_) => ${property.ident.upper()},
             % endfor
             PropertyDeclaration::Custom(_, _) => &[]

From 3dafd558c529fb24bc51c2cb5ac16c532bdaa0af Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Sun, 24 Apr 2016 01:43:59 -0400
Subject: [PATCH 11/14] Changing update_inline_style to process multiple
 declarations at once to handle shorthand serialization better

---
 components/script/dom/cssstyledeclaration.rs  |  10 +-
 components/script/dom/element.rs              |  36 ++--
 .../style/properties/properties.mako.rs       | 164 +++++++++++-------
 3 files changed, 130 insertions(+), 80 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 410c7757f5ba..7b0c62dabd5b 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -157,7 +157,9 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                     self.0.next().map(|r| &**r)
                 }
             }
-            let serialized_value = shorthand.serialize_shorthand_to_string(Map(list.iter()));
+
+            // TODO: important is hardcoded to false because method does not implement it yet
+            let serialized_value = shorthand.serialize_shorthand_value_to_string(Map(list.iter()), false);
             return DOMString::from(serialized_value);
         }
 
@@ -241,10 +243,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let element = self.owner.upcast::<Element>();
 
         // Step 8
-        for decl in declarations {
-            // Step 9
-            element.update_inline_style(decl, priority);
-        }
+        // Step 9
+        element.update_inline_style(declarations, priority);
 
         Ok(())
     }
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 6dddec311083..a05587fffa77 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -748,37 +748,45 @@ impl Element {
     }
 
     pub fn update_inline_style(&self,
-                               property_decl: PropertyDeclaration,
+                               declarations: Vec<PropertyDeclaration>,
                                style_priority: StylePriority) {
 
-        fn update(element: &Element, property_decl: PropertyDeclaration, style_priority: StylePriority) {
+        fn update(element: &Element, mut declarations: Vec<PropertyDeclaration>, style_priority: StylePriority) {
             let mut inline_declarations = element.style_attribute().borrow_mut();
-            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+            if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
                 let existing_declarations = if style_priority == StylePriority::Important {
-                    &mut declarations.important
+                    &mut existing_declarations.important
                 } else {
-                    &mut declarations.normal
+                    &mut existing_declarations.normal
                 };
 
                 // Usually, the reference count will be 1 here. But transitions could make it greater
                 // than that.
                 let existing_declarations = Arc::make_mut(existing_declarations);
-                for declaration in &mut *existing_declarations {
-                    if declaration.name() == property_decl.name() {
-                        *declaration = property_decl;
-                        return;
+
+                while let Some(mut incoming_declaration) = declarations.pop() {
+                    let mut replaced = false;
+                    for existing_declaration in &mut *existing_declarations {
+                        if existing_declaration.name() == incoming_declaration.name() {
+                            mem::swap(existing_declaration, &mut incoming_declaration);
+                            replaced = true;
+                            break;
+                        }
+                    }
+
+                    if !replaced {
+                        // inserting instead of pushing since the declarations are in reverse order
+                        existing_declarations.insert(0, incoming_declaration);
                     }
                 }
 
-                // inserting instead of pushing since the declarations are in reverse order
-                existing_declarations.insert(0, property_decl);
                 return;
             }
 
             let (important, normal) = if style_priority == StylePriority::Important {
-                (vec![property_decl], vec![])
+                (declarations, vec![])
             } else {
-                (vec![], vec![property_decl])
+                (vec![], declarations)
             };
 
             *inline_declarations = Some(PropertyDeclarationBlock {
@@ -787,7 +795,7 @@ impl Element {
             });
         }
 
-        update(self, property_decl, style_priority);
+        update(self, declarations, style_priority);
         self.sync_property_with_attrs_style();
     }
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index af6aec68307a..c83c9f241b8a 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -404,16 +404,13 @@ enum AppendableValue<'a, I>
 where I: Iterator<Item=&'a PropertyDeclaration> {
     Declaration(&'a PropertyDeclaration),
     DeclarationsForShorthand(I),
-    Css(&'a str)
+    Css(&'a str, bool)
 }
 
-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<Item=&'a PropertyDeclaration> {
+fn append_property_name<W>(dest: &mut W,
+                           property_name: &str,
+                           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 {
@@ -423,30 +420,69 @@ fn append_serialization<'a, W, I>(dest: &mut W,
         *is_first_serialization = false;
     }
 
-    try!(write!(dest, "{}:", property_name));
+    write!(dest, "{}", property_name)
+}
+
+fn append_declaration_value<'a, W, I>
+                           (dest: &mut W,
+                            appendable_value: AppendableValue<'a, I>,
+                            is_important: bool)
+                            -> fmt::Result
+                            where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
+  match appendable_value {
+      AppendableValue::Css(css, _) => {
+          try!(write!(dest, "{}", css))
+      },
+      AppendableValue::Declaration(decl) => {
+          try!(decl.to_css(dest));
+       },
+       AppendableValue::DeclarationsForShorthand(decls) => {
+           let mut decls = decls.peekable();
+           while let Some(decl) = decls.next() {
+               try!(decl.to_css(dest));
+
+               if decls.peek().is_some() {
+                   try!(write!(dest, " "));
+               }
+           }
+       }
+  }
+
+  if is_important {
+      try!(write!(dest, " !important"));
+  }
+
+  Ok(())
+}
 
-    match appendable_value {
-        AppendableValue::Css(css) => try!(write!(dest, " {}", css)),
-        AppendableValue::Declaration(decl) => {
+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<Item=&'a PropertyDeclaration> {
+
+    try!(append_property_name(dest, property_name, is_first_serialization));
+    try!(write!(dest, ":"));
+
+    // for normal parsed values, add a space between key: and value
+    match &appendable_value {
+        &AppendableValue::Css(_, is_unparsed) => {
+            if !is_unparsed {
+                try!(write!(dest, " "))
+            }
+        },
+        &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::DeclarationsForShorthand(decls) => {
-             for decl in decls {
-                 try!(write!(dest, " "));
-                 try!(decl.to_css(dest));
-             }
-         }
-    }
-
-    if is_important {
-        try!(write!(dest, " !important"));
+         &AppendableValue::DeclarationsForShorthand(_) => try!(write!(dest, " "))
     }
 
+    try!(append_declaration_value(dest, appendable_value, is_important));
     write!(dest, ";")
 }
 
@@ -622,15 +658,15 @@ impl Shorthand {
     }
 
     /// Serializes possible shorthand value to String.
-    pub fn serialize_shorthand_to_string<'a, I>(self, declarations: I) -> String
+    pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, is_important: bool) -> String
     where I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+        let appendable_value = self.get_shorthand_appendable_value(declarations).unwrap();
         let mut result = String::new();
-        self.serialize_shorthand_to_buffer(&mut result, declarations, &mut true).unwrap();
+        append_declaration_value(&mut result, appendable_value, is_important).unwrap();
         result
     }
 
-
-    /// Serializes possible shorthand value to input buffer given a list of longhand declarations.
+    /// Serializes possible shorthand name with 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<'a, W, I>(self,
                                                    dest: &mut W,
@@ -638,47 +674,53 @@ impl Shorthand {
                                                    is_first_serialization: &mut bool)
                                                    -> Result<bool, fmt::Error>
     where W: Write, I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+        match self.get_shorthand_appendable_value(declarations) {
+            None => Ok(false),
+            Some(appendable_value) => {
+                let property_name = self.name();
 
-        // Only cloning iterators (a few pointers each) not declarations.
-        let mut declarations2 = declarations.clone();
-        let mut declarations3 = declarations.clone();
-
-        let first_declaration = match declarations2.next() {
-            Some(declaration) => declaration,
-            None => return Ok(false)
-        };
-
-        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 declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
-               return append_serialization::<W, I>(
-                   dest, property_name, AppendableValue::Css(css), false, is_first_serialization
-               ).and_then(|_| Ok(true));
-           }
-           else {
-               return Ok(false);
-           }
-        }
-
-        if !declarations3.any(|d| d.with_variables()) {
-            try!(
                 append_serialization(
                     dest,
                     property_name,
-                    AppendableValue::DeclarationsForShorthand(declarations),
+                    appendable_value,
                     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
-            return Ok(true);
+                ).and_then(|_| Ok(true))
+            }
         }
+    }
+
+    fn get_shorthand_appendable_value<'a, I>(self, declarations: I) -> Option<AppendableValue<'a, I>>
+        where I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+
+            // Only cloning iterators (a few pointers each) not declarations.
+            let mut declarations2 = declarations.clone();
+            let mut declarations3 = declarations.clone();
+
+            let first_declaration = match declarations2.next() {
+                Some(declaration) => declaration,
+                None => return None
+            };
+
+            // https://drafts.csswg.org/css-variables/#variables-in-shorthands
+            if let Some(css) = first_declaration.with_variables_from_shorthand(self) {
+                if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
+                   let is_unparsed = first_declaration.value_is_unparsed();
+                   return Some(AppendableValue::Css(css, is_unparsed));
+               }
+               else {
+                   return None;
+               }
+            }
+
+            if !declarations3.any(|d| d.with_variables()) {
+                return Some(AppendableValue::DeclarationsForShorthand(declarations));
+                // 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
+            }
 
-        Ok(false)
+            None
     }
 }
 

From f6bbeae67fa5c6db6b69dedaa9a373dbab17759f Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Sat, 30 Apr 2016 00:53:26 -0400
Subject: [PATCH 12/14] Removing unnecessary field from serialization css enum

---
 components/style/properties/properties.mako.rs | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index c83c9f241b8a..de31ea761c63 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -404,7 +404,7 @@ enum AppendableValue<'a, I>
 where I: Iterator<Item=&'a PropertyDeclaration> {
     Declaration(&'a PropertyDeclaration),
     DeclarationsForShorthand(I),
-    Css(&'a str, bool)
+    Css(&'a str)
 }
 
 fn append_property_name<W>(dest: &mut W,
@@ -430,7 +430,7 @@ fn append_declaration_value<'a, W, I>
                             -> fmt::Result
                             where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
   match appendable_value {
-      AppendableValue::Css(css, _) => {
+      AppendableValue::Css(css) => {
           try!(write!(dest, "{}", css))
       },
       AppendableValue::Declaration(decl) => {
@@ -468,10 +468,8 @@ fn append_serialization<'a, W, I>(dest: &mut W,
 
     // for normal parsed values, add a space between key: and value
     match &appendable_value {
-        &AppendableValue::Css(_, is_unparsed) => {
-            if !is_unparsed {
-                try!(write!(dest, " "))
-            }
+        &AppendableValue::Css(_) => {
+            try!(write!(dest, " "))
         },
         &AppendableValue::Declaration(decl) => {
             if !decl.value_is_unparsed() {
@@ -705,8 +703,7 @@ impl Shorthand {
             // https://drafts.csswg.org/css-variables/#variables-in-shorthands
             if let Some(css) = first_declaration.with_variables_from_shorthand(self) {
                 if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
-                   let is_unparsed = first_declaration.value_is_unparsed();
-                   return Some(AppendableValue::Css(css, is_unparsed));
+                   return Some(AppendableValue::Css(css));
                }
                else {
                    return None;

From 8b3926079374da8cbcdf50383293aad97e268d76 Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Tue, 3 May 2016 23:22:08 -0400
Subject: [PATCH 13/14] Removed mutation calls from
 sync_property_with_attrs_style method in order to avoid reparsing serialized
 output

---
 components/script/dom/attr.rs    |  7 ++++++-
 components/script/dom/element.rs | 26 +++++++++++++++++---------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs
index 95ff73e2cb75..43c1226b2952 100644
--- a/components/script/dom/attr.rs
+++ b/components/script/dom/attr.rs
@@ -173,13 +173,18 @@ impl Attr {
     pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
         assert!(Some(owner) == self.owner().r());
         owner.will_mutate_attr();
-        mem::swap(&mut *self.value.borrow_mut(), &mut value);
+        self.swap_value(&mut value);
         if self.identifier.namespace == ns!() {
             vtable_for(owner.upcast())
                 .attribute_mutated(self, AttributeMutation::Set(Some(&value)));
         }
     }
 
+    /// Used to swap the attribute's value without triggering mutation events
+    pub fn swap_value(&self, value: &mut AttrValue) {
+        mem::swap(&mut *self.value.borrow_mut(), value);
+    }
+
     pub fn identifier(&self) -> &AttrIdentifier {
         &self.identifier
     }
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index a05587fffa77..cf40c038c81e 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -698,6 +698,8 @@ impl Element {
         }
     }
 
+    // this sync method is called upon modification of the style_attribute property,
+    // therefore, it should not trigger subsequent mutation events
     fn sync_property_with_attrs_style(&self) {
         let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() {
             declarations.to_css_string()
@@ -705,20 +707,26 @@ impl Element {
             String::new()
         };
 
-        let new_style = AttrValue::String(style_str);
+        let mut new_style = AttrValue::String(style_str);
 
         if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &atom!("style")) {
-            style_attr.set_value(new_style, self);
+            style_attr.swap_value(&mut new_style);
             return;
         }
 
-        self.push_new_attribute(
-            atom!("style"),
-            new_style,
-            atom!("style"),
-            ns!(),
-            Some(atom!("style"))
-        );
+        // explicitly not calling the push_new_attribute convenience method
+        // in order to avoid triggering mutation events
+        let window = window_from_node(self);
+        let attr = Attr::new(&window,
+                             atom!("style"),
+                             new_style,
+                             atom!("style"),
+                             ns!(),
+                             Some(atom!("style")),
+                             Some(self));
+
+         assert!(attr.GetOwnerElement().r() == Some(self));
+         self.attrs.borrow_mut().push(JS::from_ref(&attr));
     }
 
     pub fn remove_inline_style_property(&self, property: &str) {

From ff5cfb12a0db9c68571f8d36d16d1325e3db5f79 Mon Sep 17 00:00:00 2001
From: David Raifaizen <d-raif@hotmail.com>
Date: Wed, 18 May 2016 22:23:06 -0400
Subject: [PATCH 14/14] Restoring node dirty calls after properties are set to
 trigger mutations

---
 components/script/dom/cssstyledeclaration.rs | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 7b0c62dabd5b..dac16d72bcf7 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, window_from_node};
+use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
 use std::cell::Ref;
@@ -246,6 +246,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         // Step 9
         element.update_inline_style(declarations, priority);
 
+        let node = element.upcast::<Node>();
+        node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }
 
@@ -278,6 +280,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             None => element.set_inline_style_property_priority(&[&*property], priority),
         }
 
+        let node = element.upcast::<Node>();
+        node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }
 
@@ -299,19 +303,22 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         // Step 3
         let value = self.GetPropertyValue(property.clone());
 
-        let elem = self.owner.upcast::<Element>();
+        let element = self.owner.upcast::<Element>();
 
         match Shorthand::from_name(&property) {
             // Step 4
             Some(shorthand) => {
                 for longhand in shorthand.longhands() {
-                    elem.remove_inline_style_property(longhand)
+                    element.remove_inline_style_property(longhand)
                 }
             }
             // Step 5
-            None => elem.remove_inline_style_property(&property),
+            None => element.remove_inline_style_property(&property),
         }
 
+        let node = element.upcast::<Node>();
+        node.dirty(NodeDamage::NodeStyleDamaged);
+
         // Step 6
         Ok(value)
     }