Remove some PropertyDeclaration cloning.

components/script/dom/cssstyledeclaration.rs

@@ -21,6 +21,7 @@ use style::properties::PropertyDeclaration;
 
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
+use std::cell::Ref;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
 #[dom_struct]
@@ -50,7 +51,7 @@ macro_rules! css_properties(
     );
 );
 
-fn serialize_list(list: &Vec<PropertyDeclaration>) -> DOMString {
+fn serialize_list(list: &[Ref<PropertyDeclaration>]) -> DOMString {
     list.iter().fold(String::new(), |accum, ref declaration| {
         accum + &declaration.value() + " "
     })
@@ -78,25 +79,24 @@ impl CSSStyleDeclaration {
 }
 
 trait PrivateCSSStyleDeclarationHelpers {
-    fn get_declaration(self, property: &Atom) -> Option<PropertyDeclaration>;
-    fn get_important_declaration(self, property: &Atom) -> Option<PropertyDeclaration>;
-    fn get_computed_style(self, property: &Atom) -> Option<DOMString>;
+    fn get_declaration(&self, property: &Atom) -> Option<Ref<PropertyDeclaration>>;
+    fn get_important_declaration(&self, property: &Atom) -> Option<Ref<PropertyDeclaration>>;
 }
 
-impl<'a> PrivateCSSStyleDeclarationHelpers for &'a CSSStyleDeclaration {
-    fn get_declaration(self, property: &Atom) -> Option<PropertyDeclaration> {
-        let owner = self.owner.root();
-        let element = ElementCast::from_ref(owner.r());
-        element.get_inline_style_declaration(property).map(|decl| decl.clone())
+impl PrivateCSSStyleDeclarationHelpers for HTMLElement {
+    fn get_declaration(&self, property: &Atom) -> Option<Ref<PropertyDeclaration>> {
+        let element = ElementCast::from_ref(self);
+        element.get_inline_style_declaration(property)
     }
 
-    fn get_important_declaration(self, property: &Atom) -> Option<PropertyDeclaration> {
-        let owner = self.owner.root();
-        let element = ElementCast::from_ref(owner.r());
-        element.get_important_inline_style_declaration(property).map(|decl| decl.clone())
+    fn get_important_declaration(&self, property: &Atom) -> Option<Ref<PropertyDeclaration>> {
+        let element = ElementCast::from_ref(self);
+        element.get_important_inline_style_declaration(property)
+    }
 }
 
-    fn get_computed_style(self, property: &Atom) -> Option<DOMString> {
+impl CSSStyleDeclaration {
+    fn get_computed_style(&self, property: &Atom) -> Option<DOMString> {
         let owner = self.owner.root();
         let node = NodeCast::from_ref(owner.r());
         if !node.is_in_doc() {
@@ -144,6 +144,8 @@ impl<'a> CSSStyleDeclarationMethods for &'a CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
     fn GetPropertyValue(self, property: DOMString) -> DOMString {
+        let owner = self.owner.root();
+
         // Step 1
         let property = Atom::from_slice(&property.to_ascii_lowercase());
 
@@ -161,7 +163,7 @@ impl<'a> CSSStyleDeclarationMethods for &'a CSSStyleDeclaration {
             // Step 2.2
             for longhand in longhand_properties.iter() {
                 // Step 2.2.1
-                let declaration = self.get_declaration(&Atom::from_slice(&longhand));
+                let declaration = owner.get_declaration(&Atom::from_slice(&longhand));
 
                 // Step 2.2.2 & 2.2.3
                 match declaration {
@@ -175,11 +177,12 @@ impl<'a> CSSStyleDeclarationMethods for &'a CSSStyleDeclaration {
         }
 
         // Step 3 & 4
-        if let Some(ref declaration) = self.get_declaration(&property) {
-            declaration.value()
-        } else {
-            "".to_owned()
-        }
+        // FIXME: redundant let binding https://github.com/rust-lang/rust/issues/22252
+        let result = match owner.get_declaration(&property) {
+            Some(declaration) => declaration.value(),
+            None => "".to_owned(),
+        };
+        result
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
@@ -198,9 +201,13 @@ impl<'a> CSSStyleDeclarationMethods for &'a CSSStyleDeclaration {
                 return "important".to_owned();
             }
         // Step 3
-        } else if self.get_important_declaration(&property).is_some() {
+        } else {
+            // FIXME: extra let binding https://github.com/rust-lang/rust/issues/22323
+            let owner = self.owner.root();
+            if owner.get_important_declaration(&property).is_some() {
                 return "important".to_owned();
             }
+        }
 
         // Step 4
         "".to_owned()

components/script/dom/element.rs

@@ -584,8 +584,8 @@ pub trait ElementHelpers<'a> {
     fn remove_inline_style_property(self, property: &str);
     fn update_inline_style(self, property_decl: PropertyDeclaration, style_priority: StylePriority);
     fn set_inline_style_property_priority(self, properties: &[&str], style_priority: StylePriority);
-    fn get_inline_style_declaration(self, property: &Atom) -> Option<PropertyDeclaration>;
-    fn get_important_inline_style_declaration(self, property: &Atom) -> Option<PropertyDeclaration>;
+    fn get_inline_style_declaration(self, property: &Atom) -> Option<Ref<'a, PropertyDeclaration>>;
+    fn get_important_inline_style_declaration(self, property: &Atom) -> Option<Ref<'a, PropertyDeclaration>>;
     fn serialize(self, traversal_scope: TraversalScope) -> Fallible<DOMString>;
     fn get_root_element(self) -> Root<Element>;
     fn lookup_prefix(self, namespace: Namespace) -> Option<DOMString>;
@@ -660,7 +660,7 @@ impl<'a> ElementHelpers<'a> for &'a Element {
                                     .iter()
                                     .position(|decl| decl.name() == property);
             if let Some(index) = index {
-                Arc::make_unique(&mut declarations.normal).remove(index);
+                Arc::get_mut(&mut declarations.normal).unwrap().remove(index);
                 return;
             }
 
@@ -668,7 +668,7 @@ impl<'a> ElementHelpers<'a> for &'a Element {
                                     .iter()
                                     .position(|decl| decl.name() == property);
             if let Some(index) = index {
-                Arc::make_unique(&mut declarations.important).remove(index);
+                Arc::get_mut(&mut declarations.important).unwrap().remove(index);
                 return;
             }
         }
@@ -678,10 +678,14 @@ impl<'a> ElementHelpers<'a> for &'a Element {
         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 {
-                Arc::make_unique(&mut declarations.important)
+                &mut declarations.important
             } else {
-                Arc::make_unique(&mut declarations.normal)
+                &mut declarations.normal
             };
+            // Element is the only owner of the Arc’s for its style attribute,
+            // except during selector matching.
+            // But selector matching does not run concurrently with script.
+            let existing_declarations = Arc::get_mut(existing_declarations).unwrap();
 
             for declaration in existing_declarations.iter_mut() {
                 if declaration.name() == property_decl.name() {
@@ -713,8 +717,11 @@ impl<'a> ElementHelpers<'a> for &'a Element {
             } else {
                 (&mut declarations.normal, &mut declarations.important)
             };
-            let from = Arc::make_unique(from);
-            let to = Arc::make_unique(to);
+            // Element is the only owner of the Arc’s for its style attribute,
+            // except during selector matching.
+            // But selector matching does not run concurrently with script.
+            let from = Arc::get_mut(from).unwrap();
+            let to = Arc::get_mut(to).unwrap();
             let mut new_from = Vec::new();
             for declaration in from.drain(..) {
                 if properties.contains(&declaration.name()) {
@@ -727,24 +734,24 @@ impl<'a> ElementHelpers<'a> for &'a Element {
         }
     }
 
-    fn get_inline_style_declaration(self, property: &Atom) -> Option<PropertyDeclaration> {
-        let inline_declarations = self.style_attribute.borrow();
+    fn get_inline_style_declaration(self, property: &Atom) -> Option<Ref<'a, PropertyDeclaration>> {
+        Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| {
             inline_declarations.as_ref().and_then(|declarations| {
                 declarations.normal
                             .iter()
                             .chain(declarations.important.iter())
                             .find(|decl| decl.matches(&property))
-                        .map(|decl| decl.clone())
+            })
         })
     }
 
-    fn get_important_inline_style_declaration(self, property: &Atom) -> Option<PropertyDeclaration> {
-        let inline_declarations = self.style_attribute.borrow();
+    fn get_important_inline_style_declaration(self, property: &Atom) -> Option<Ref<'a, PropertyDeclaration>> {
+        Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| {
             inline_declarations.as_ref().and_then(|declarations| {
                 declarations.important
                             .iter()
                             .find(|decl| decl.matches(&property))
-                        .map(|decl| decl.clone())
+            })
         })
     }
 

components/script/lib.rs

@@ -10,6 +10,7 @@
 #![feature(borrow_state)]
 #![feature(box_raw)]
 #![feature(box_syntax)]
+#![feature(cell_extras)]
 #![feature(core)]
 #![feature(core_intrinsics)]
 #![feature(custom_attribute)]

components/style/properties.mako.rs

@@ -5583,9 +5583,9 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
         match declaration {
             Ok((results, important)) => {
                 if important {
-                    important_declarations.push_all(&results);
+                    important_declarations.extend(results);
                 } else {
-                    normal_declarations.push_all(&results);
+                    normal_declarations.extend(results);
                 }
             }
             Err(range) => {
@@ -5669,7 +5669,7 @@ impl<T: ToCss> DeclaredValue<T> {
     }
 }
 
-#[derive(Clone, PartialEq)]
+#[derive(PartialEq)]
 pub enum PropertyDeclaration {
     % for property in LONGHANDS:
         ${property.camel_case}(DeclaredValue<longhands::${property.ident}::SpecifiedValue>),