Use PropertyDeclarationBlock in a DOMRefCell everywhere.

components/script/dom/cssstyledeclaration.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::ToCss;
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::{self, CSSStyleDeclarationMethods};
 use dom::bindings::error::{Error, ErrorResult, Fallible};
 use dom::bindings::global::GlobalRef;
@@ -14,13 +15,11 @@ use dom::element::Element;
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
-use std::slice;
 use std::sync::Arc;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
-use style::properties::{PropertyDeclaration, Shorthand, Importance};
+use style::properties::{Shorthand, Importance};
 use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute};
-use style::refcell::Ref;
 use style::selector_impl::PseudoElement;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
@@ -93,7 +92,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.declarations.len(),
+            Some(ref declarations) => declarations.borrow().declarations.len(),
             None => 0,
         };
         len as u32
@@ -119,43 +118,42 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         // Step 2
         if let Some(shorthand) = Shorthand::from_name(&property) {
+            let style_attribute = owner.style_attribute().borrow();
+            let style_attribute = if let Some(ref style_attribute) = *style_attribute {
+                style_attribute.borrow()
+            } else {
+                // shorthand.longhands() is never empty, so with no style attribute
+                // step 2.2.2 would do this:
+                return DOMString::new()
+            };
+
             // Step 2.1
             let mut list = vec![];
 
             // Step 2.2
             for longhand in shorthand.longhands() {
                 // Step 2.2.1
-                let declaration = owner.get_inline_style_declaration(&Atom::from(*longhand));
+                let declaration = style_attribute.get(longhand);
 
                 // Step 2.2.2 & 2.2.3
                 match declaration {
-                    Some(declaration) => list.push(declaration),
+                    Some(&(ref declaration, _importance)) => list.push(declaration),
                     None => return DOMString::new(),
                 }
             }
 
             // Step 2.3
-            // Work around closures not being Clone
-            #[derive(Clone)]
-            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, (PropertyDeclaration, Importance)>>);
-            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.0)
-                }
-            }
-
             // 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()), Importance::Normal);
+                list, Importance::Normal);
             return DOMString::from(serialized_value);
         }
 
         // Step 3 & 4
-        match owner.get_inline_style_declaration(&property) {
+        owner.get_inline_style_declaration(&property, |d| match d {
             Some(declaration) => DOMString::from(declaration.0.value()),
             None => DOMString::new(),
-        }
+        })
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
@@ -172,13 +170,18 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                                     .all(|priority| priority == "important") {
                 return DOMString::from("important");
             }
-        // Step 3
         } else {
-            if let Some(decl) = self.owner.get_inline_style_declaration(&property) {
-                if decl.1.important() {
-                    return DOMString::from("important");
+            // Step 3
+            return self.owner.get_inline_style_declaration(&property, |d| {
+                if let Some(decl) = d {
+                    if decl.1.important() {
+                        return DOMString::from("important");
+                    }
                 }
-            }
+
+                // Step 4
+                DOMString::new()
+            })
         }
 
         // Step 4
@@ -328,13 +331,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let elem = self.owner.upcast::<Element>();
         let style_attribute = elem.style_attribute().borrow();
         style_attribute.as_ref().and_then(|declarations| {
-            declarations.declarations.get(index)
-        }).map(|&(ref declaration, importance)| {
-            let mut css = declaration.to_css_string();
-            if importance.important() {
-                css += " !important";
-            }
-            DOMString::from(css)
+            declarations.borrow().declarations.get(index).map(|entry| {
+                let (ref declaration, importance) = *entry;
+                let mut css = declaration.to_css_string();
+                if importance.important() {
+                    css += " !important";
+                }
+                DOMString::from(css)
+            })
         })
     }
 
@@ -344,7 +348,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let style_attribute = elem.style_attribute().borrow();
 
         if let Some(declarations) = style_attribute.as_ref() {
-            DOMString::from(declarations.to_css_string())
+            DOMString::from(declarations.borrow().to_css_string())
         } else {
             DOMString::new()
         }
@@ -366,7 +370,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() {
             None // Step 2
         } else {
-            Some(Arc::new(decl_block))
+            Some(Arc::new(DOMRefCell::new(decl_block)))
         };
         element.sync_property_with_attrs_style();
         let node = element.upcast::<Node>();

components/script/dom/element.rs

@@ -110,7 +110,7 @@ pub struct Element {
     attrs: DOMRefCell<Vec<JS<Attr>>>,
     id_attribute: DOMRefCell<Option<Atom>>,
     #[ignore_heap_size_of = "Arc"]
-    style_attribute: DOMRefCell<Option<Arc<PropertyDeclarationBlock>>>,
+    style_attribute: DOMRefCell<Option<Arc<DOMRefCell<PropertyDeclarationBlock>>>>,
     attr_list: MutNullableHeap<JS<NamedNodeMap>>,
     class_list: MutNullableHeap<JS<DOMTokenList>>,
     state: Cell<ElementState>,
@@ -298,7 +298,7 @@ pub trait LayoutElementHelpers {
     #[allow(unsafe_code)]
     unsafe fn html_element_in_html_document_for_layout(&self) -> bool;
     fn id_attribute(&self) -> *const Option<Atom>;
-    fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>>;
+    fn style_attribute(&self) -> *const Option<Arc<DOMRefCell<PropertyDeclarationBlock>>>;
     fn local_name(&self) -> &Atom;
     fn namespace(&self) -> &Namespace;
     fn get_checked_state_for_layout(&self) -> bool;
@@ -330,10 +330,10 @@ impl LayoutElementHelpers for LayoutJS<Element> {
         #[inline]
         fn from_declaration(rule: PropertyDeclaration) -> ApplicableDeclarationBlock {
             ApplicableDeclarationBlock::from_declarations(
-                Arc::new(PropertyDeclarationBlock {
+                Arc::new(DOMRefCell::new(PropertyDeclarationBlock {
                     declarations: vec![(rule, Importance::Normal)],
                     important_count: 0,
-                }),
+                })),
                 Importance::Normal)
         }
 
@@ -619,7 +619,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
     }
 
     #[allow(unsafe_code)]
-    fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>> {
+    fn style_attribute(&self) -> *const Option<Arc<DOMRefCell<PropertyDeclarationBlock>>> {
         unsafe {
             (*self.unsafe_get()).style_attribute.borrow_for_layout()
         }
@@ -708,7 +708,7 @@ impl Element {
         self.attrs.borrow()
     }
 
-    pub fn style_attribute(&self) -> &DOMRefCell<Option<Arc<PropertyDeclarationBlock>>> {
+    pub fn style_attribute(&self) -> &DOMRefCell<Option<Arc<DOMRefCell<PropertyDeclarationBlock>>>> {
         &self.style_attribute
     }
 
@@ -738,7 +738,7 @@ impl Element {
     // therefore, it should not trigger subsequent mutation events
     pub fn sync_property_with_attrs_style(&self) {
         let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() {
-            declarations.to_css_string()
+            declarations.borrow().to_css_string()
         } else {
             String::new()
         };
@@ -770,15 +770,15 @@ impl Element {
             let mut inline_declarations = element.style_attribute.borrow_mut();
             if let &mut Some(ref mut declarations) = &mut *inline_declarations {
                 let mut importance = None;
-                let index = declarations.declarations.iter().position(|&(ref decl, i)| {
+                let index = declarations.borrow().declarations.iter().position(|&(ref decl, i)| {
                     let matching = decl.matches(property);
                     if matching {
                         importance = Some(i)
                     }
                     matching
                 });
                 if let Some(index) = index {
-                    let declarations = Arc::make_mut(declarations);
+                    let mut declarations = Arc::make_mut(declarations).borrow_mut();
                     declarations.declarations.remove(index);
                     if importance.unwrap().important() {
                         declarations.important_count -= 1;
@@ -801,7 +801,8 @@ impl Element {
                 {
                     // Usually, the reference count will be 1 here. But transitions could make it greater
                     // than that.
-                    let declaration_block = Arc::make_mut(declaration_block);
+                    let mut declaration_block = Arc::make_mut(declaration_block).borrow_mut();
+                    let declaration_block = &mut *declaration_block;
                     let existing_declarations = &mut declaration_block.declarations;
 
                     'outer: for incoming_declaration in declarations {
@@ -835,10 +836,10 @@ impl Element {
                 0
             };
 
-            *inline_declarations = Some(Arc::new(PropertyDeclarationBlock {
+            *inline_declarations = Some(Arc::new(DOMRefCell::new(PropertyDeclarationBlock {
                 declarations: declarations.into_iter().map(|d| (d, importance)).collect(),
                 important_count: important_count,
-            }));
+            })));
         }
 
         update(self, declarations, importance);
@@ -853,7 +854,8 @@ impl Element {
             if let &mut Some(ref mut block) = &mut *inline_declarations {
                 // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
                 // could make them greater than that.
-                let block = Arc::make_mut(block);
+                let mut block = Arc::make_mut(block).borrow_mut();
+                let block = &mut *block;
                 let declarations = &mut block.declarations;
                 for &mut (ref declaration, ref mut importance) in declarations {
                     if properties.iter().any(|p| declaration.name() == **p) {
@@ -875,16 +877,15 @@ impl Element {
         self.sync_property_with_attrs_style();
     }
 
-    pub fn get_inline_style_declaration(&self,
-                                        property: &Atom)
-                                        -> Option<Ref<(PropertyDeclaration, Importance)>> {
-        Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| {
-            inline_declarations.as_ref().and_then(|declarations| {
-                declarations.declarations
-                            .iter()
-                            .find(|&&(ref decl, _)| decl.matches(&property))
-            })
-        })
+    pub fn get_inline_style_declaration<F, R>(&self, property: &str, f: F) -> R
+    where F: FnOnce(Option<&(PropertyDeclaration, Importance)>) -> R {
+        let style_attr = self.style_attribute.borrow();
+        if let Some(ref block) = *style_attr {
+            let block = block.borrow();
+            f(block.get(property))
+        } else {
+            f(None)
+        }
     }
 
     pub fn serialize(&self, traversal_scope: TraversalScope) -> Fallible<DOMString> {
@@ -2130,11 +2131,11 @@ impl VirtualMethods for Element {
                 *self.style_attribute.borrow_mut() =
                     mutation.new_value(attr).map(|value| {
                         let win = window_from_node(self);
-                        Arc::new(parse_style_attribute(
+                        Arc::new(DOMRefCell::new(parse_style_attribute(
                             &value,
                             &doc.base_url(),
                             win.css_error_reporter(),
-                            ParserContextExtraData::default()))
+                            ParserContextExtraData::default())))
                     });
                 if node.is_in_doc() {
                     node.dirty(NodeDamage::NodeStyleDamaged);

components/script/layout_wrapper.rs

@@ -30,6 +30,7 @@
 
 #![allow(unsafe_code)]
 
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::inheritance::{CharacterDataTypeId, ElementTypeId};
 use dom::bindings::inheritance::{HTMLElementTypeId, NodeTypeId};
 use dom::bindings::js::LayoutJS;
@@ -461,7 +462,7 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         ServoLayoutNode::from_layout_js(self.element.upcast())
     }
 
-    fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>> {
+    fn style_attribute(&self) -> Option<&Arc<DOMRefCell<PropertyDeclarationBlock>>> {
         unsafe {
             (*self.element.style_attribute()).as_ref()
         }

components/style/dom.rs

@@ -8,6 +8,7 @@
 
 use atomic_refcell::{AtomicRef, AtomicRefMut};
 use data::PersistentStyleData;
+use domrefcell::DOMRefCell;
 use element_state::ElementState;
 use properties::{ComputedValues, PropertyDeclarationBlock};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
@@ -202,7 +203,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
 
     fn as_node(&self) -> Self::ConcreteNode;
 
-    fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>>;
+    fn style_attribute(&self) -> Option<&Arc<DOMRefCell<PropertyDeclarationBlock>>>;
 
     fn get_state(&self) -> ElementState;
 

components/style/domrefcell.rs

@@ -11,12 +11,19 @@ use thread_state;
 ///
 /// This extends the API of `core::cell::RefCell` to allow unsafe access in
 /// certain situations, with dynamic checking in debug builds.
-#[derive(Clone)]
+#[derive(Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct DOMRefCell<T> {
     value: RefCell<T>,
 }
 
+// FIXME: These two impls make promises that are not quite true,
+// but maybe the debug_assert! makes it close enough.
+#[allow(unsafe_code)]
+unsafe impl<T: Send> Send for DOMRefCell<T> {}
+#[allow(unsafe_code)]
+unsafe impl<T: Sync> Sync for DOMRefCell<T> {}
+
 // Functionality specific to Servo's `DOMRefCell` type
 // ===================================================
 

components/style/gecko/wrapper.rs

@@ -46,6 +46,7 @@ use std::ptr;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, AtomicPtr};
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
+use style::domrefcell::DOMRefCell;
 use url::Url;
 
 pub struct NonOpaqueStyleData(AtomicRefCell<PersistentStyleData>);
@@ -468,7 +469,7 @@ impl<'le> TElement for GeckoElement<'le> {
         unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) }
     }
 
-    fn style_attribute(&self) -> Option<&Arc<PropertyDeclarationBlock>> {
+    fn style_attribute(&self) -> Option<&Arc<DOMRefCell<PropertyDeclarationBlock>>> {
         let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.0) };
         if declarations.is_null() {
             None