Auto merge of #11377 - servo:style-attr-ser-rebase, r=SimonSapin

Update style attributes on CSSStyleDeclaration changes

Rebase of #9410. Fixes #9410, fixes #9307.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11377)
<!-- Reviewable:end -->

3 files changed, 146 insertions, 99 deletions

diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs
index 95ff73e2cb7..43c1226b295 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/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index df9251f3888..dac16d72bcf 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -13,14 +13,13 @@ 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 std::slice;
 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 +49,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>,
@@ -172,7 +148,19 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
 
             // Step 2.3
-            return DOMString::from(serialize_shorthand(shorthand, &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)
+                }
+            }
+
+            // 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);
         }
 
         // Step 3 & 4
@@ -255,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);
 
         let node = element.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
@@ -317,20 +303,20 @@ 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 = elem.upcast::<Node>();
+        let node = element.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
 
         // Step 6
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index f5832089202..cf40c038c81 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;
@@ -698,89 +698,145 @@ 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()
+        } else {
+            String::new()
+        };
+
+        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.swap_value(&mut new_style);
+            return;
+        }
+
+        // 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) {
-        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,
+                               declarations: Vec<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, mut declarations: Vec<PropertyDeclaration>, style_priority: StylePriority) {
+            let mut inline_declarations = element.style_attribute().borrow_mut();
+            if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
+                let existing_declarations = if style_priority == StylePriority::Important {
+                    &mut existing_declarations.important
+                } else {
+                    &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);
+
+                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);
+                    }
                 }
+
+                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 {
+                (declarations, vec![])
+            } else {
+                (vec![], declarations)
+            };
 
-        *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, declarations, 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,