layout: Reference count ComputedValues structures like Gecko does.

This has no difference in CSS selector matching performance and results
in a 31% speedup in constraint solving on the rainbow page.

src/components/main/css/matching.rs

@@ -8,7 +8,7 @@ use std::cell::Cell;
 use std::comm;
 use std::task;
 use std::vec;
-use extra::arc::RWArc;
+use extra::arc::{Arc, RWArc};
 
 use css::node_style::StyledNode;
 use layout::incremental;
@@ -85,13 +85,16 @@ impl MatchMethods for AbstractNode<LayoutView> {
 
     fn cascade_node(&self, parent: Option<AbstractNode<LayoutView>>) {
         let parent_style = match parent {
-            Some(parent) => Some(parent.style()),
+            Some(ref parent) => Some(parent.style()),
             None => None
         };
 
         let computed_values = unsafe {
-            cascade(self.borrow_layout_data_unchecked().as_ref().unwrap().applicable_declarations,
+            Arc::new(cascade(self.borrow_layout_data_unchecked()
-                    parent_style)
+                                 .as_ref()
+                                 .unwrap()
+                                 .applicable_declarations,
+                             parent_style.map(|parent_style| parent_style.get())))
         };
 
         match *self.mutate_layout_data().ptr {
@@ -102,8 +105,8 @@ impl MatchMethods for AbstractNode<LayoutView> {
                     None => (),
                     Some(ref previous_style) => {
                         layout_data.restyle_damage =
-                            Some(incremental::compute_damage(previous_style,
+                            Some(incremental::compute_damage(previous_style.get(),
-                                                             &computed_values).to_int())
+                                                             computed_values.get()).to_int())
                     }
                 }
                 *style = Some(computed_values)

src/components/main/css/node_style.rs

@@ -7,22 +7,20 @@
 use css::node_util::NodeUtil;
 use layout::incremental::RestyleDamage;
 
+use extra::arc::Arc;
 use style::ComputedValues;
 use script::dom::node::{AbstractNode, LayoutView};
 
 /// Node mixin providing `style` method that returns a `NodeStyle`
 pub trait StyledNode {
-    fn style(&self) -> &ComputedValues;
+    fn style<'a>(&'a self) -> &'a Arc<ComputedValues>;
     fn restyle_damage(&self) -> RestyleDamage;
 }
 
 impl StyledNode for AbstractNode<LayoutView> {
     #[inline]
-    fn style(&self) -> &ComputedValues {
+    fn style<'a>(&'a self) -> &'a Arc<ComputedValues> {
-        // FIXME(pcwalton): Is this assertion needed for memory safety? It's slow.
+        self.get_css_select_results()
-        //assert!(self.is_element()); // Only elements can have styles
-        let results = self.get_css_select_results();
-        results
     }
 
     fn restyle_damage(&self) -> RestyleDamage {

src/components/main/css/node_util.rs

@@ -5,30 +5,28 @@
 use layout::incremental::RestyleDamage;
 use layout::util::LayoutDataAccess;
 
+use extra::arc::Arc;
 use std::cast;
 use style::ComputedValues;
 use script::dom::node::{AbstractNode, LayoutView};
 use servo_util::tree::TreeNodeRef;
 
-pub trait NodeUtil<'self> {
+pub trait NodeUtil {
-    fn get_css_select_results(self) -> &'self ComputedValues;
+    fn get_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues>;
-    fn set_css_select_results(self, decl: ComputedValues);
     fn have_css_select_results(self) -> bool;
 
     fn get_restyle_damage(self) -> RestyleDamage;
     fn set_restyle_damage(self, damage: RestyleDamage);
 }
 
-impl<'self> NodeUtil<'self> for AbstractNode<LayoutView> {
+impl NodeUtil for AbstractNode<LayoutView> {
     /** 
      * Provides the computed style for the given node. If CSS selector
      * Returns the style results for the given node. If CSS selector
      * matching has not yet been performed, fails.
-     * FIXME: This isn't completely memory safe since the style is
-     * stored in a box that can be overwritten
      */
     #[inline]
-    fn get_css_select_results(self) -> &'self ComputedValues {
+    fn get_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues> {
         unsafe {
             cast::transmute_region(self.borrow_layout_data_unchecked()
                                        .as_ref()
@@ -44,14 +42,6 @@ impl<'self> NodeUtil<'self> for AbstractNode<LayoutView> {
         self.borrow_layout_data().ptr.as_ref().unwrap().style.is_some()
     }
 
-    /// Update the computed style of an HTML element with a style specified by CSS.
-    fn set_css_select_results(self, decl: ComputedValues) {
-        match *self.mutate_layout_data().ptr {
-            Some(ref mut data) => data.style = Some(decl),
-            _ => fail!("no layout data for this node"),
-        }
-    }
-
     /// Get the description of how to account for recent style changes.
     /// This is a simple bitfield and fine to copy by value.
     fn get_restyle_damage(self) -> RestyleDamage {

src/components/main/layout/box.rs

@@ -60,6 +60,9 @@ pub struct Box {
     /// The DOM node that this `Box` originates from.
     node: AbstractNode<LayoutView>,
 
+    /// The CSS style of this box.
+    style: Arc<ComputedValues>,
+
     /// The position of this box relative to its owning flow.
     position: Slot<Rect<Au>>,
 
@@ -212,8 +215,27 @@ pub enum SplitBoxResult {
 impl Box {
     /// Constructs a new `Box` instance.
     pub fn new(node: AbstractNode<LayoutView>, specific: SpecificBoxInfo) -> Box {
+        // Find the nearest ancestor element and take its style. (It should be either that node or
+        // its immediate parent.)
+        //
+        // FIXME(pcwalton): This is incorrect for non-inherited properties on anonymous boxes. For
+        // example:
+        //
+        //     <div style="border: solid">
+        //         <p>Foo</p>
+        //         Bar
+        //         <p>Baz</p>
+        //     </div>
+        //
+        // An anonymous block box is generated around `Bar`, but it shouldn't inherit the border.
+        let mut nearest_ancestor_element = node;
+        while !nearest_ancestor_element.is_element() {
+            nearest_ancestor_element = node.parent_node().expect("no nearest element?!");
+        }
+
         Box {
             node: node,
+            style: (*nearest_ancestor_element.style()).clone(),
             position: Slot::init(Au::zero_rect()),
             border: Slot::init(Zero::zero()),
             padding: Slot::init(Zero::zero()),
@@ -236,6 +258,7 @@ impl Box {
     pub fn transform(&self, size: Size2D<Au>, specific: SpecificBoxInfo) -> Box {
         Box {
             node: self.node,
+            style: self.style.clone(),
             position: Slot::init(Rect(self.position.get().origin, size)),
             border: Slot::init(self.border.get()),
             padding: Slot::init(self.padding.get()),
@@ -376,7 +399,7 @@ impl Box {
     /// FIXME(pcwalton): Just replace with the clear type from the style module for speed?
     #[inline(always)]
     pub fn clear(&self) -> Option<ClearType> {
-        let style = self.node.style();
+        let style = self.style();
         match style.Box.clear {
             clear::none => None,
             clear::left => Some(ClearLeft),
@@ -390,7 +413,7 @@ impl Box {
     /// FIXME(pcwalton): This should not be necessary; just make the font part of style sharable
     /// with the display list somehow. (Perhaps we should use an ARC.)
     pub fn font_style(&self) -> FontStyle {
-        let my_style = self.nearest_ancestor_element().style();
+        let my_style = self.style();
 
         debug!("(font style) start: {:?}", self.nearest_ancestor_element().type_id());
 
@@ -421,24 +444,23 @@ impl Box {
         }
     }
 
-    // FIXME(pcwalton): Why &'static??? Isn't that wildly unsafe?
     #[inline(always)]
-    pub fn style(&self) -> &'static ComputedValues {
+    pub fn style<'a>(&'a self) -> &'a ComputedValues {
-        self.node.style()
+        self.style.get()
     }
 
     /// Returns the text alignment of the computed style of the nearest ancestor-or-self `Element`
     /// node.
     pub fn text_align(&self) -> text_align::T {
-        self.nearest_ancestor_element().style().Text.text_align
+        self.style().Text.text_align
     }
 
     pub fn line_height(&self) -> line_height::T {
-        self.nearest_ancestor_element().style().Box.line_height
+        self.style().Box.line_height
     }
 
     pub fn vertical_align(&self) -> vertical_align::T {
-        self.nearest_ancestor_element().style().Box.vertical_align
+        self.style().Box.vertical_align
     }
 
     /// Returns the text decoration of the computed style of the nearest `Element` node
@@ -458,13 +480,13 @@ impl Box {
             // FIXME: Implement correctly.
             let display_in_flow = true;
 
-            let position = element.style().Box.position;
+            let position = element.style().get().Box.position;
-            let float = element.style().Box.float;
+            let float = element.style().get().Box.float;
 
             let in_flow = (position == position::static_) && (float == float::none) &&
                 display_in_flow;
 
-            let text_decoration = element.style().Text.text_decoration;
+            let text_decoration = element.style().get().Text.text_decoration;
 
             if text_decoration == text_decoration::none && in_flow {
                 match element.parent_node() {
@@ -524,9 +546,7 @@ impl Box {
         // needed. We could use display list optimization to clean this up, but it still seems
         // inefficient. What we really want is something like "nearest ancestor element that
         // doesn't have a box".
-        let nearest_ancestor_element = self.nearest_ancestor_element();
+        let style = self.style();
-
-        let style = nearest_ancestor_element.style();
         let background_color = style.resolve_color(style.Background.background_color);
         if !background_color.alpha.approx_eq(&0.0) {
             list.with_mut_ref(|list| {
@@ -613,7 +633,7 @@ impl Box {
                box_bounds, absolute_box_bounds, self.debug_str());
         debug!("Box::build_display_list: dirty={}, offset={}", *dirty, *offset);
 
-        if self.nearest_ancestor_element().style().Box.visibility != visibility::visible {
+        if self.style().Box.visibility != visibility::visible {
             return;
         }
 
@@ -642,9 +662,7 @@ impl Box {
                     list.append_item(ClipDisplayItemClass(item));
                 }
 
-
+                let color = self.style().Color.color.to_gfx_color();
-                let nearest_ancestor_element = self.nearest_ancestor_element();
-                let color = nearest_ancestor_element.style().Color.color.to_gfx_color();
 
                 // Create the text box.
                 do list.with_mut_ref |list| {
@@ -991,7 +1009,7 @@ impl Box {
 
     /// Returns true if the contents should be clipped (i.e. if `overflow` is `hidden`).
     pub fn needs_clip(&self) -> bool {
-        self.node.style().Box.overflow == overflow::hidden
+        self.style().Box.overflow == overflow::hidden
     }
 
     /// Returns a debugging string describing this box.

src/components/main/layout/construct.rs

@@ -472,7 +472,10 @@ impl<'self> PostorderNodeMutTraversal for FlowConstructor<'self> {
     fn process(&mut self, node: AbstractNode<LayoutView>) -> bool {
         // Get the `display` property for this node, and determine whether this node is floated.
         let (display, float) = match node.type_id() {
-            ElementNodeTypeId(_) => (node.style().Box.display, node.style().Box.float),
+            ElementNodeTypeId(_) => {
+                let style = node.style().get();
+                (style.Box.display, style.Box.float)
+            }
             TextNodeTypeId => (display::inline, float::none),
             CommentNodeTypeId |
             DoctypeNodeTypeId |

src/components/main/layout/inline.rs

@@ -589,7 +589,7 @@ impl InlineFlow {
         // not from the style of the first box child.
         let linebox_align = if line.range.begin() < boxes.len() {
             let first_box = &boxes[line.range.begin()];
-            first_box.nearest_ancestor_element().style().Text.text_align
+            first_box.style().Text.text_align
         } else {
             // Nothing to lay out, so assume left alignment.
             text_align::left
@@ -804,8 +804,9 @@ impl Flow for InlineFlow {
                 // content area. But for now we assume it's zero.
                 let parent_text_bottom = Au::new(0);
 
-                let parent = cur_box.node.parent_node();
+                let parent = cur_box.node.parent_node().unwrap();
-                let font_size = parent.unwrap().style().Font.font_size;
+                let parent_style = parent.style();
+                let font_size = parent_style.get().Font.font_size;
                 parent_text_top = font_size;
 
                 // Calculate a relative offset from the baseline.

src/components/main/layout/layout_task.rs

@@ -477,8 +477,8 @@ impl LayoutTask {
                     for child in node.traverse_preorder() {
                       if child.type_id() == ElementNodeTypeId(HTMLHtmlElementTypeId) || 
                          child.type_id() == ElementNodeTypeId(HTMLBodyElementTypeId) {
-                             let element_bg_color = child.style().resolve_color(
+                             let element_bg_color = child.style().get().resolve_color(
-                                 child.style().Background.background_color
+                                 child.style().get().Background.background_color
                              ).to_gfx_color();
                              match element_bg_color {
                                  color::rgba(0., 0., 0., 0.) => {}

src/components/main/layout/util.rs

@@ -127,7 +127,7 @@ pub struct LayoutData {
     applicable_declarations: ~[Arc<~[PropertyDeclaration]>],
 
     /// The results of CSS styling for this node.
-    style: Option<ComputedValues>,
+    style: Option<Arc<ComputedValues>>,
 
     /// Description of how to account for recent style changes.
     restyle_damage: Option<int>,