auto merge of #1605 : pcwalton/servo/harden-layout-more, r=metajack

Closes #1584.

r? @metajack

src/components/main/css/node_style.rs

@@ -6,7 +6,7 @@
 
 use css::node_util::NodeUtil;
 use layout::incremental::RestyleDamage;
-use layout::wrapper::LayoutNode;
+use layout::wrapper::ThreadSafeLayoutNode;
 
 use extra::arc::Arc;
 use style::ComputedValues;
@@ -17,7 +17,7 @@ pub trait StyledNode {
     fn restyle_damage(&self) -> RestyleDamage;
 }
 
-impl<'ln> StyledNode for LayoutNode<'ln> {
+impl<'ln> StyledNode for ThreadSafeLayoutNode<'ln> {
     #[inline]
     fn style<'a>(&'a self) -> &'a Arc<ComputedValues> {
         self.get_css_select_results()
@@ -27,3 +27,4 @@ impl<'ln> StyledNode for LayoutNode<'ln> {
         self.get_restyle_damage()
     }
 }
+

src/components/main/css/node_util.rs

@@ -4,11 +4,11 @@
 
 use layout::incremental::RestyleDamage;
 use layout::util::LayoutDataAccess;
-use layout::wrapper::LayoutNode;
+use layout::wrapper::{TLayoutNode, ThreadSafeLayoutNode};
 
 use extra::arc::Arc;
 use std::cast;
-use style::{ComputedValues, TNode};
+use style::ComputedValues;
 
 pub trait NodeUtil {
     fn get_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues>;
@@ -18,17 +18,20 @@ pub trait NodeUtil {
     fn set_restyle_damage(self, damage: RestyleDamage);
 }
 
-impl<'ln> NodeUtil for LayoutNode<'ln> {
-    /** 
-     * 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.
-     */
+impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
+    /// Returns the style results for the given node. If CSS selector
+    /// matching has not yet been performed, fails.
     #[inline]
     fn get_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues> {
         unsafe {
             let layout_data_ref = self.borrow_layout_data();
-            cast::transmute_region(layout_data_ref.get().as_ref().unwrap().data.style.as_ref().unwrap())
+            cast::transmute_region(layout_data_ref.get()
+                                                  .as_ref()
+                                                  .unwrap()
+                                                  .data
+                                                  .style
+                                                  .as_ref()
+                                                  .unwrap())
         }
     }
 
@@ -43,7 +46,7 @@ impl<'ln> NodeUtil for LayoutNode<'ln> {
     fn get_restyle_damage(self) -> RestyleDamage {
         // For DOM elements, if we haven't computed damage yet, assume the worst.
         // Other nodes don't have styles.
-        let default = if self.is_element() {
+        let default = if self.node_is_element() {
             RestyleDamage::all()
         } else {
             RestyleDamage::none()

src/components/main/layout/box_.rs

@@ -40,7 +40,7 @@ use layout::flow::{Flow, FlowFlagsInfo};
 use layout::flow;
 use layout::model::{MaybeAuto, specified, Auto, Specified};
 use layout::util::OpaqueNode;
-use layout::wrapper::LayoutNode;
+use layout::wrapper::{TLayoutNode, ThreadSafeLayoutNode};
 
 /// Boxes (`struct Box`) are the leaves of the layout tree. They cannot position themselves. In
 /// general, boxes do not have a simple correspondence with CSS boxes in the specification:
@@ -123,10 +123,11 @@ impl ImageBoxInfo {
     ///
     /// FIXME(pcwalton): The fact that image boxes store the cache in the box makes little sense to
     /// me.
-    pub fn new(node: &LayoutNode, image_url: Url, local_image_cache: MutexArc<LocalImageCache>)
+    pub fn new(node: &ThreadSafeLayoutNode,
+               image_url: Url,
+               local_image_cache: MutexArc<LocalImageCache>)
                -> ImageBoxInfo {
-
-        fn convert_length(node: &LayoutNode, name: &str) -> Option<Au> {
+        fn convert_length(node: &ThreadSafeLayoutNode, name: &str) -> Option<Au> {
             node.with_element(|element| {
                 element.get_attr(&namespace::Null, name).and_then(|string| {
                     let n: Option<int> = FromStr::from_str(string);
@@ -207,7 +208,7 @@ pub struct IframeBoxInfo {
 
 impl IframeBoxInfo {
     /// Creates the information specific to an iframe box.
-    pub fn new(node: &LayoutNode) -> IframeBoxInfo {
+    pub fn new(node: &ThreadSafeLayoutNode) -> IframeBoxInfo {
         let (pipeline_id, subpage_id) = node.iframe_pipeline_and_subpage_ids();
         IframeBoxInfo {
             pipeline_id: pipeline_id,
@@ -249,7 +250,7 @@ pub struct UnscannedTextBoxInfo {
 
 impl UnscannedTextBoxInfo {
     /// Creates a new instance of `UnscannedTextBoxInfo` from the given DOM node.
-    pub fn new(node: &LayoutNode) -> UnscannedTextBoxInfo {
+    pub fn new(node: &ThreadSafeLayoutNode) -> UnscannedTextBoxInfo {
         // FIXME(pcwalton): Don't copy text; atomically reference count it instead.
         UnscannedTextBoxInfo {
             text: node.text(),
@@ -297,9 +298,9 @@ pub struct InlineParentInfo {
 
 impl Box {
     /// Constructs a new `Box` instance.
-    pub fn new(node: LayoutNode, specific: SpecificBoxInfo) -> Box {
+    pub fn new(node: ThreadSafeLayoutNode, specific: SpecificBoxInfo) -> Box {
         Box {
-            node: OpaqueNode::from_layout_node(&node),
+            node: OpaqueNode::from_thread_safe_layout_node(&node),
             style: node.style().clone(),
             position: RefCell::new(Au::zero_rect()),
             border: RefCell::new(Zero::zero()),

src/components/main/layout/construct.rs

@@ -30,7 +30,7 @@ use layout::flow::{BaseFlow, Flow, FlowLeafSet, ImmutableFlowUtils, MutableOwned
 use layout::inline::InlineFlow;
 use layout::text::TextRunScanner;
 use layout::util::{LayoutDataAccess, OpaqueNode};
-use layout::wrapper::{LayoutNode, PostorderNodeMutTraversal};
+use layout::wrapper::{PostorderNodeMutTraversal, TLayoutNode, ThreadSafeLayoutNode};
 
 use gfx::font_context::FontContext;
 use script::dom::element::{HTMLIframeElementTypeId, HTMLImageElementTypeId};
@@ -234,7 +234,7 @@ impl<'fc> FlowConstructor<'fc> {
     }
 
     /// Builds the `ImageBoxInfo` for the given image. This is out of line to guide inlining.
-    fn build_box_info_for_image(&mut self, node: LayoutNode) -> Option<ImageBoxInfo> {
+    fn build_box_info_for_image(&mut self, node: ThreadSafeLayoutNode) -> Option<ImageBoxInfo> {
         // FIXME(pcwalton): Don't copy URLs.
         match node.image_url() {
             None => None,
@@ -247,7 +247,7 @@ impl<'fc> FlowConstructor<'fc> {
     }
 
     /// Builds a `Box` for the given node.
-    fn build_box_for_node(&mut self, node: LayoutNode) -> Box {
+    fn build_box_for_node(&mut self, node: ThreadSafeLayoutNode) -> Box {
         let specific = match node.type_id() {
             ElementNodeTypeId(HTMLImageElementTypeId) => {
                 match self.build_box_info_for_image(node) {
@@ -267,7 +267,10 @@ impl<'fc> FlowConstructor<'fc> {
     /// `#[inline(always)]` because this is performance critical and LLVM will not inline it
     /// otherwise.
     #[inline(always)]
-    fn flush_inline_boxes_to_flow(&mut self, boxes: ~[Box], flow: &mut ~Flow, node: LayoutNode) {
+    fn flush_inline_boxes_to_flow(&mut self,
+                                  boxes: ~[Box],
+                                  flow: &mut ~Flow,
+                                  node: ThreadSafeLayoutNode) {
         if boxes.len() == 0 {
             return
         }
@@ -285,7 +288,7 @@ impl<'fc> FlowConstructor<'fc> {
     fn flush_inline_boxes_to_flow_if_necessary(&mut self,
                                                opt_boxes: &mut Option<~[Box]>,
                                                flow: &mut ~Flow,
-                                               node: LayoutNode) {
+                                               node: ThreadSafeLayoutNode) {
         let opt_boxes = util::replace(opt_boxes, None);
         if opt_boxes.len() > 0 {
             self.flush_inline_boxes_to_flow(opt_boxes.to_vec(), flow, node)
@@ -295,9 +298,7 @@ impl<'fc> FlowConstructor<'fc> {
     /// Builds the children flows underneath a node with `display: block`. After this call,
     /// other `BlockFlow`s or `InlineFlow`s will be populated underneath this node, depending on
     /// whether {ib} splits needed to happen.
-    fn build_children_of_block_flow(&mut self,
-                                    flow: &mut ~Flow,
-                                    node: LayoutNode) {
+    fn build_children_of_block_flow(&mut self, flow: &mut ~Flow, node: ThreadSafeLayoutNode) {
         // Gather up boxes for the inline flows we might need to create.
         let mut opt_boxes_for_inline_flow = None;
         let mut first_box = true;
@@ -389,7 +390,7 @@ impl<'fc> FlowConstructor<'fc> {
     /// Builds a flow for a node with `display: block`. This yields a `BlockFlow` with possibly
     /// other `BlockFlow`s or `InlineFlow`s underneath it, depending on whether {ib} splits needed
     /// to happen.
-    fn build_flow_for_block(&mut self, node: LayoutNode, is_fixed: bool) -> ~Flow {
+    fn build_flow_for_block(&mut self, node: ThreadSafeLayoutNode, is_fixed: bool) -> ~Flow {
         let base = BaseFlow::new(self.next_flow_id(), node);
         let box_ = self.build_box_for_node(node);
         let mut flow = ~BlockFlow::from_box(base, box_, is_fixed) as ~Flow;
@@ -399,7 +400,7 @@ impl<'fc> FlowConstructor<'fc> {
 
     /// Builds the flow for a node with `float: {left|right}`. This yields a float `BlockFlow` with
     /// a `BlockFlow` underneath it.
-    fn build_flow_for_floated_block(&mut self, node: LayoutNode, float_type: FloatType)
+    fn build_flow_for_floated_block(&mut self, node: ThreadSafeLayoutNode, float_type: FloatType)
                                     -> ~Flow {
         let base = BaseFlow::new(self.next_flow_id(), node);
         let box_ = self.build_box_for_node(node);
@@ -413,7 +414,7 @@ impl<'fc> FlowConstructor<'fc> {
     /// Concatenates the boxes of kids, adding in our own borders/padding/margins if necessary.
     /// Returns the `InlineBoxesConstructionResult`, if any. There will be no
     /// `InlineBoxesConstructionResult` if this node consisted entirely of ignorable whitespace.
-    fn build_boxes_for_nonreplaced_inline_content(&mut self, node: LayoutNode)
+    fn build_boxes_for_nonreplaced_inline_content(&mut self, node: ThreadSafeLayoutNode)
                                                   -> ConstructionResult {
         let mut opt_inline_block_splits = None;
         let mut opt_box_accumulator = None;
@@ -519,7 +520,9 @@ impl<'fc> FlowConstructor<'fc> {
         }
     }
 
-    fn set_inline_info_for_inline_child(&mut self, boxes: &~[&Box], parent_node: LayoutNode) {
+    fn set_inline_info_for_inline_child(&mut self,
+                                        boxes: &~[&Box],
+                                        parent_node: ThreadSafeLayoutNode) {
         let parent_box = self.build_box_for_node(parent_node);
         let font_style = parent_box.font_style();
         let font_group = self.font_context.get_resolved_font_for_style(&font_style);
@@ -557,7 +560,7 @@ impl<'fc> FlowConstructor<'fc> {
                             style: parent_box.style.clone(),
                             font_ascent: font_ascent,
                             font_descent: font_descent,
-                            node: OpaqueNode::from_layout_node(&parent_node),
+                            node: OpaqueNode::from_thread_safe_layout_node(&parent_node),
                         });
                 },
                 &None => {}
@@ -566,7 +569,8 @@ impl<'fc> FlowConstructor<'fc> {
     }
     /// Creates an `InlineBoxesConstructionResult` for replaced content. Replaced content doesn't
     /// render its children, so this just nukes a child's boxes and creates a `Box`.
-    fn build_boxes_for_replaced_inline_content(&mut self, node: LayoutNode) -> ConstructionResult {
+    fn build_boxes_for_replaced_inline_content(&mut self, node: ThreadSafeLayoutNode)
+                                               -> ConstructionResult {
         for kid in node.children() {
             kid.set_flow_construction_result(NoConstructionResult)
         }
@@ -582,7 +586,7 @@ impl<'fc> FlowConstructor<'fc> {
 
     /// Builds one or more boxes for a node with `display: inline`. This yields an
     /// `InlineBoxesConstructionResult`.
-    fn build_boxes_for_inline(&mut self, node: LayoutNode) -> ConstructionResult {
+    fn build_boxes_for_inline(&mut self, node: ThreadSafeLayoutNode) -> ConstructionResult {
         // Is this node replaced content?
         if !node.is_replaced_content() {
             // Go to a path that concatenates our kids' boxes.
@@ -598,7 +602,7 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
     // `#[inline(always)]` because this is always called from the traversal function and for some
     // reason LLVM's inlining heuristics go awry here.
     #[inline(always)]
-    fn process(&mut self, node: LayoutNode) -> bool {
+    fn process(&mut self, node: ThreadSafeLayoutNode) -> bool {
         // Get the `display` property for this node, and determine whether this node is floated.
         let (display, float, position) = match node.type_id() {
             ElementNodeTypeId(_) => {
@@ -671,7 +675,7 @@ trait NodeUtils {
     fn swap_out_construction_result(self) -> ConstructionResult;
 }
 
-impl<'ln> NodeUtils for LayoutNode<'ln> {
+impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
     fn is_replaced_content(self) -> bool {
         match self.type_id() {
             TextNodeTypeId |

src/components/main/layout/flow.rs

@@ -35,7 +35,7 @@ use layout::incremental::RestyleDamage;
 use layout::inline::InlineFlow;
 use layout::parallel::{FlowParallelInfo, UnsafeFlow};
 use layout::parallel;
-use layout::wrapper::LayoutNode;
+use layout::wrapper::ThreadSafeLayoutNode;
 
 use extra::dlist::{DList, DListIterator, MutDListIterator};
 use extra::container::Deque;
@@ -553,7 +553,7 @@ impl Iterator<@Box> for BoxIterator {
 
 impl BaseFlow {
     #[inline]
-    pub fn new(id: int, node: LayoutNode) -> BaseFlow {
+    pub fn new(id: int, node: ThreadSafeLayoutNode) -> BaseFlow {
         let style = node.style();
         BaseFlow {
             restyle_damage: node.restyle_damage(),

src/components/main/layout/layout_task.rs

@@ -19,7 +19,7 @@ use layout::parallel::{AssignHeightsAndStoreOverflowTraversalKind, BubbleWidthsT
 use layout::parallel::{UnsafeFlow};
 use layout::parallel;
 use layout::util::{LayoutDataAccess, OpaqueNode, LayoutDataWrapper};
-use layout::wrapper::{DomLeafSet, LayoutNode};
+use layout::wrapper::{DomLeafSet, LayoutNode, TLayoutNode, ThreadSafeLayoutNode};
 
 use extra::arc::{Arc, MutexArc};
 use geom::rect::Rect;
@@ -416,6 +416,7 @@ impl LayoutTask {
     /// marked `#[inline(never)]` to aid benchmarking in sampling profilers.
     #[inline(never)]
     fn construct_flow_tree(&self, layout_context: &mut LayoutContext, node: LayoutNode) -> ~Flow {
+        let node = ThreadSafeLayoutNode::new(node);
         node.traverse_postorder_mut(&mut FlowConstructor::init(layout_context));
 
         let mut layout_data_ref = node.mutate_layout_data();
@@ -623,13 +624,16 @@ 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()
-                                                    .get()
-                                                    .resolve_color(child.style()
-                                                                        .get()
-                                                                        .Background
-                                                                        .background_color)
-                                                    .to_gfx_color();
+                        let element_bg_color = {
+                            let thread_safe_child = ThreadSafeLayoutNode::new(child);
+                            thread_safe_child.style()
+                                             .get()
+                                             .resolve_color(thread_safe_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

@@ -5,7 +5,7 @@
 use layout::box_::Box;
 use layout::construct::{ConstructionResult, NoConstructionResult};
 use layout::parallel::DomParallelInfo;
-use layout::wrapper::LayoutNode;
+use layout::wrapper::{LayoutNode, TLayoutNode, ThreadSafeLayoutNode};
 
 use extra::arc::Arc;
 use script::dom::bindings::utils::Reflectable;
@@ -227,6 +227,15 @@ impl OpaqueNode {
         }
     }
 
+    /// Converts a thread-safe DOM node (layout view) to an `OpaqueNode`.
+    pub fn from_thread_safe_layout_node(node: &ThreadSafeLayoutNode) -> OpaqueNode {
+        unsafe {
+            let abstract_node = node.get_abstract();
+            let ptr: uintptr_t = cast::transmute(abstract_node.reflector().get_jsobject());
+            OpaqueNode(ptr)
+        }
+    }
+
     /// Converts a DOM node (script view) to an `OpaqueNode`.
     pub fn from_script_node(node: &AbstractNode) -> OpaqueNode {
         unsafe {