Move (some) CSSStyleDeclaration.SetProperty logic to style

components/script/dom/cssstyledeclaration.rs

@@ -17,7 +17,7 @@ use std::ascii::AsciiExt;
 use std::sync::Arc;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
-use style::properties::{Shorthand, Importance};
+use style::properties::{Shorthand, Importance, PropertyDeclarationBlock};
 use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute};
 use style::selector_impl::PseudoElement;
 
@@ -91,7 +91,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     fn Length(&self) -> u32 {
         let elem = self.owner.upcast::<Element>();
         let len = match *elem.style_attribute().borrow() {
-            Some(ref declarations) => declarations.read().declarations.len(),
+            Some(ref lock) => lock.read().declarations.len(),
             None => 0,
         };
         len as u32
@@ -112,8 +112,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         }
 
         let style_attribute = self.owner.style_attribute().borrow();
-        let style_attribute = if let Some(ref style_attribute) = *style_attribute {
-            style_attribute.read()
+        let style_attribute = if let Some(ref lock) = *style_attribute {
+            lock.read()
         } else {
             // No style attribute is like an empty style attribute: no matching declaration.
             return DOMString::new()
@@ -127,8 +127,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
     fn GetPropertyPriority(&self, property: DOMString) -> DOMString {
         let style_attribute = self.owner.style_attribute().borrow();
-        let style_attribute = if let Some(ref style_attribute) = *style_attribute {
-            style_attribute.read()
+        let style_attribute = if let Some(ref lock) = *style_attribute {
+            lock.read()
         } else {
             // No style attribute is like an empty style attribute: no matching declaration.
             return DOMString::new()
@@ -144,7 +144,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty
     fn SetProperty(&self,
-                   mut property: DOMString,
+                   property: DOMString,
                    value: DOMString,
                    priority: DOMString)
                    -> ErrorResult {
@@ -153,46 +153,78 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             return Err(Error::NoModificationAllowed);
         }
 
-        // Step 2
-        property.make_ascii_lowercase();
-
         // Step 3
         if !is_supported_property(&property) {
             return Ok(());
         }
 
-        // Step 4
+        let mut style_attribute = self.owner.style_attribute().borrow_mut();
+
         if value.is_empty() {
-            return self.RemoveProperty(property).map(|_| ());
+            // Step 4
+            let empty;
+            {
+                let mut style_attribute = if let Some(ref lock) = *style_attribute {
+                    lock.write()
+                } else {
+                    // No style attribute is like an empty style attribute: nothing to remove.
+                    return Ok(())
+                };
+
+                style_attribute.remove_property(&property);
+                empty = style_attribute.declarations.is_empty()
+            }
+            if empty {
+                *style_attribute = None;
+            }
+        } else {
+            // Step 5
+            let importance = match &*priority {
+                "" => Importance::Normal,
+                p if p.eq_ignore_ascii_case("important") => Importance::Important,
+                _ => return Ok(()),
+            };
+
+            // Step 6
+            let window = window_from_node(&*self.owner);
+            let declarations =
+                parse_one_declaration(&property, &value, &window.get_url(), window.css_error_reporter(),
+                                      ParserContextExtraData::default());
+
+            // Step 7
+            let declarations = if let Ok(declarations) = declarations {
+                declarations
+            } else {
+                return Ok(());
+            };
+
+            // Step 8
+            // Step 9
+            match *style_attribute {
+                Some(ref lock) => {
+                    let mut style_attribute = lock.write();
+                    for declaration in declarations {
+                        style_attribute.set_parsed_declaration(declaration, importance);
+                    }
+                    self.owner.set_style_attr(style_attribute.to_css_string());
+                }
+                ref mut option @ None => {
+                    let important_count = if importance.important() {
+                        declarations.len() as u32
+                    } else {
+                        0
+                    };
+                    let block = PropertyDeclarationBlock {
+                        declarations: declarations.into_iter().map(|d| (d, importance)).collect(),
+                        important_count: important_count,
+                    };
+                    self.owner.set_style_attr(block.to_css_string());
+                    *option = Some(Arc::new(RwLock::new(block)));
+                }
+            }
         }
 
-        // Step 5
-        let priority = match &*priority {
-            "" => Importance::Normal,
-            p if p.eq_ignore_ascii_case("important") => Importance::Important,
-            _ => return Ok(()),
-        };
-
-        // Step 6
-        let window = window_from_node(&*self.owner);
-        let declarations =
-            parse_one_declaration(&property, &value, &window.get_url(), window.css_error_reporter(),
-                                  ParserContextExtraData::default());
-
-        // Step 7
-        let declarations = if let Ok(declarations) = declarations {
-            declarations
-        } else {
-            return Ok(());
-        };
-
-        let element = self.owner.upcast::<Element>();
-
-        // Step 8
-        // Step 9
-        element.update_inline_style(declarations, priority);
-
-        let node = element.upcast::<Node>();
+        let node = self.owner.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }
@@ -247,8 +279,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let mut string = String::new();
         let empty;
         {
-            let mut style_attribute = if let Some(ref mut style_attribute) = *style_attribute {
-                style_attribute.write()
+            let mut style_attribute = if let Some(ref lock) = *style_attribute {
+                lock.write()
             } else {
                 // No style attribute is like an empty style attribute: nothing to remove.
                 return Ok(DOMString::new())
@@ -288,8 +320,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let index = index as usize;
         let elem = self.owner.upcast::<Element>();
         let style_attribute = elem.style_attribute().borrow();
-        style_attribute.as_ref().and_then(|declarations| {
-            declarations.read().declarations.get(index).map(|entry| {
+        style_attribute.as_ref().and_then(|lock| {
+            lock.read().declarations.get(index).map(|entry| {
                 let (ref declaration, importance) = *entry;
                 let mut css = declaration.to_css_string();
                 if importance.important() {
@@ -305,8 +337,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let elem = self.owner.upcast::<Element>();
         let style_attribute = elem.style_attribute().borrow();
 
-        if let Some(declarations) = style_attribute.as_ref() {
-            DOMString::from(declarations.read().to_css_string())
+        if let Some(lock) = style_attribute.as_ref() {
+            DOMString::from(lock.read().to_css_string())
         } else {
             DOMString::new()
         }

components/script/dom/element.rs

@@ -767,59 +767,6 @@ impl Element {
          self.attrs.borrow_mut().push(JS::from_ref(&attr));
     }
 
-    pub fn update_inline_style(&self,
-                               declarations: Vec<PropertyDeclaration>,
-                               importance: Importance) {
-        fn update(element: &Element, declarations: Vec<PropertyDeclaration>,
-                  importance: Importance) {
-            let mut inline_declarations = element.style_attribute().borrow_mut();
-            if let &mut Some(ref mut declaration_block) = &mut *inline_declarations {
-                {
-                    let mut declaration_block = declaration_block.write();
-                    let declaration_block = &mut *declaration_block;
-                    let existing_declarations = &mut declaration_block.declarations;
-
-                    'outer: for incoming_declaration in declarations {
-                        for existing_declaration in &mut *existing_declarations {
-                            if existing_declaration.0.name() == incoming_declaration.name() {
-                                match (existing_declaration.1, importance) {
-                                    (Importance::Normal, Importance::Important) => {
-                                        declaration_block.important_count += 1;
-                                    }
-                                    (Importance::Important, Importance::Normal) => {
-                                        declaration_block.important_count -= 1;
-                                    }
-                                    _ => {}
-                                }
-                                *existing_declaration = (incoming_declaration, importance);
-                                continue 'outer;
-                            }
-                        }
-                        existing_declarations.push((incoming_declaration, importance));
-                        if importance.important() {
-                            declaration_block.important_count += 1;
-                        }
-                    }
-                }
-                return;
-            }
-
-            let important_count = if importance.important() {
-                declarations.len() as u32
-            } else {
-                0
-            };
-
-            *inline_declarations = Some(Arc::new(RwLock::new(PropertyDeclarationBlock {
-                declarations: declarations.into_iter().map(|d| (d, importance)).collect(),
-                important_count: important_count,
-            })));
-        }
-
-        update(self, declarations, importance);
-        self.sync_property_with_attrs_style();
-    }
-
     pub fn set_inline_style_property_priority(&self,
                                               properties: &[&str],
                                               new_importance: Importance) {

components/style/properties/declaration_block.rs

@@ -130,6 +130,30 @@ impl PropertyDeclarationBlock {
         }
     }
 
+    pub fn set_parsed_declaration(&mut self, declaration: PropertyDeclaration,
+                                  importance: Importance) {
+        for slot in &mut *self.declarations {
+            if slot.0.name() == declaration.name() {
+                match (slot.1, importance) {
+                    (Importance::Normal, Importance::Important) => {
+                        self.important_count += 1;
+                    }
+                    (Importance::Important, Importance::Normal) => {
+                        self.important_count -= 1;
+                    }
+                    _ => {}
+                }
+                *slot = (declaration, importance);
+                return
+            }
+        }
+
+        self.declarations.push((declaration, importance));
+        if importance.important() {
+            self.important_count += 1;
+        }
+    }
+
     /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty
     pub fn remove_property(&mut self, property_name: &str) {
         // Step 2

components/style/properties/properties.mako.rs

@@ -1974,6 +1974,8 @@ pub fn modify_style_for_inline_absolute_hypothetical_fragment(style: &mut Arc<Co
     }
 }
 
+
+// FIXME: https://github.com/w3c/csswg-drafts/issues/580
 pub fn is_supported_property(property: &str) -> bool {
     match_ignore_ascii_case! { property,
         % for property in data.shorthands + data.longhands: