layout: Stop exposing raw Element types to layout as well.

This should make layout memory-safe as is, though we will need to do
more stuff for incremental reflow and parallelism.

src/components/main/css/matching.rs

@@ -30,7 +30,7 @@ pub trait MatchMethods {
 impl<'self> MatchMethods for LayoutNode<'self> {
     fn match_node(&self, stylist: &Stylist) {
         let applicable_declarations = do self.with_element |element| {
-            let style_attribute = match element.style_attribute {
+            let style_attribute = match *element.style_attribute() {
                 None => None,
                 Some(ref style_attribute) => Some(style_attribute)
             };

src/components/main/layout/wrapper.rs

@@ -15,7 +15,8 @@
 //!     onto these objects and cause use-after-free.
 
 use extra::url::Url;
-use script::dom::element::Element;
+use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
+use script::dom::element::{HTMLLinkElementTypeId};
 use script::dom::htmliframeelement::HTMLIFrameElement;
 use script::dom::htmlimageelement::HTMLImageElement;
 use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, LayoutView, Node};
@@ -23,7 +24,7 @@ use script::dom::node::{NodeTypeId};
 use script::dom::text::Text;
 use servo_msg::constellation_msg::{PipelineId, SubpageId};
 use std::cast;
-use style::TNode;
+use style::{PropertyDeclarationBlock, TElement, TNode};
 
 /// A wrapper so that layout can access only the methods that it should have access to. Layout must
 /// only ever see these and must never see instances of `AbstractNode`.
@@ -227,7 +228,7 @@ impl<'self> LayoutNode<'self> {
     }
 }
 
-impl<'self> TNode<Element> for LayoutNode<'self> {
+impl<'self> TNode<LayoutElement<'self>> for LayoutNode<'self> {
     fn parent_node(&self) -> Option<LayoutNode<'self>> {
         unsafe {
             self.node.node().parent_node.map(|node| self.new_with_this_lifetime(node))
@@ -260,10 +261,18 @@ impl<'self> TNode<Element> for LayoutNode<'self> {
         }
     }
 
-    /// FIXME(pcwalton): Unsafe!
+    /// If this is an element, accesses the element data. Fails if this is not an element node.
     #[inline]
-    fn with_element<R>(&self, f: &fn(&Element) -> R) -> R {
-        self.node.with_imm_element(f)
+    fn with_element<R>(&self, f: &fn(&LayoutElement<'self>) -> R) -> R {
+        self.node.with_imm_element(|element| {
+            // FIXME(pcwalton): Workaround until Rust gets multiple lifetime parameters on
+            // implementations.
+            unsafe {
+                f(&LayoutElement {
+                    element: cast::transmute_region(element),
+                })
+            }
+        })
     }
 }
 
@@ -350,3 +359,40 @@ pub trait PostorderNodeMutTraversal {
     }
 }
 
+/// A wrapper around elements that ensures layout can only ever access safe properties.
+pub struct LayoutElement<'self> {
+    priv element: &'self Element,
+}
+
+impl<'self> LayoutElement<'self> {
+    pub fn style_attribute(&self) -> &'self Option<PropertyDeclarationBlock> {
+        &self.element.style_attribute
+    }
+}
+
+impl<'self> TElement for LayoutElement<'self> {
+    fn get_local_name<'a>(&'a self) -> &'a str {
+        self.element.tag_name.as_slice()
+    }
+
+    fn get_namespace_url<'a>(&'a self) -> &'a str {
+        self.element.namespace.to_str().unwrap_or("")
+    }
+
+    fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> {
+        self.element.get_attr(ns_url, name)
+    }
+
+    fn get_link(&self) -> Option<~str> {
+        // FIXME: This is HTML only.
+        match self.element.node.type_id {
+            // http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#
+            // selector-link
+            ElementNodeTypeId(HTMLAnchorElementTypeId) |
+            ElementNodeTypeId(HTMLAreaElementTypeId) |
+            ElementNodeTypeId(HTMLLinkElementTypeId) => self.get_attr(None, "href"),
+            _ => None,
+        }
+    }
+}
+

src/components/script/dom/document.rs

@@ -29,7 +29,6 @@ use std::cast;
 use std::hashmap::HashMap;
 use std::str::eq_slice;
 use std::unstable::raw::Box;
-use style::{TElement, TNode};
 
 #[deriving(Eq)]
 pub enum DocumentTypeId {

src/components/script/dom/element.rs

@@ -20,7 +20,6 @@ use dom::namespace::Namespace;
 use layout_interface::{ContentBoxQuery, ContentBoxResponse, ContentBoxesQuery};
 use layout_interface::{ContentBoxesResponse, ContentChangedDocumentDamage};
 use layout_interface::{MatchSelectorsDocumentDamage};
-use style::{TElement, TNode};
 use style;
 
 use std::comm;
@@ -125,31 +124,6 @@ pub enum ElementTypeId {
 // Element methods
 //
 
-impl TElement for Element {
-    fn get_local_name<'a>(&'a self) -> &'a str {
-        self.tag_name.as_slice()
-    }
-
-    fn get_namespace_url<'a>(&'a self) -> &'a str {
-        self.namespace.to_str().unwrap_or("")
-    }
-
-    fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> {
-        self.get_attribute(ns_url, name).map(|attr| attr.value.clone())
-    }
-
-    fn get_link(&self) -> Option<~str>{
-        // FIXME: This is HTML only.
-        match self.node.type_id {
-            // http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#selector-link
-            ElementNodeTypeId(HTMLAnchorElementTypeId) |
-            ElementNodeTypeId(HTMLAreaElementTypeId) |
-            ElementNodeTypeId(HTMLLinkElementTypeId)
-            => self.get_attr(None, "href"),
-            _ => None,
-        }
-    }
-}
 
 impl<'self> Element {
     pub fn new_inherited(type_id: ElementTypeId, tag_name: ~str, namespace: Namespace, document: AbstractDocument) -> Element {
@@ -187,6 +161,11 @@ impl<'self> Element {
         })
     }
 
+    // FIXME(pcwalton): This is kind of confusingly named relative to the above...
+    pub fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> {
+        self.get_attribute(ns_url, name).map(|attr| attr.value.clone())
+    }
+
     pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
                     -> ErrorResult {
         self.set_attribute(abstract_self, namespace::Null, name, value)

src/components/script/dom/node.rs

@@ -25,7 +25,6 @@ use std::cast::transmute;
 use std::cast;
 use std::unstable::raw::Box;
 use std::util;
-use style::TNode;
 
 //
 // The basic Node structure

src/components/script/script_task.rs

@@ -52,7 +52,6 @@ use std::ptr;
 use std::str::eq_slice;
 use std::task::{spawn_sched, SingleThreaded};
 use std::util::replace;
-use style::{TElement, TNode};
 
 /// Messages used to control the script task.
 pub enum ScriptMsg {

src/components/style/node.rs

@@ -2,11 +2,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-//! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between script and
+//! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and
 //! style.
 
-/// FIXME(pcwalton): Should not require `Clone` and should instead return references. When this
-/// happens this will need to only be implemented for `AbstractNode<LayoutView>`.
 pub trait TNode<E:TElement> : Clone {
     fn parent_node(&self) -> Option<Self>;
     fn prev_sibling(&self) -> Option<Self>;