auto merge of #1867 : saneyuki/servo/1828, r=pcwalton

Fix #1828

src/components/main/css/matching.rs

@@ -216,7 +216,7 @@ impl StyleSharingCandidate {
 
         let mut style = Some(style);
         let mut parent_style = Some(parent_style);
-        node.with_element(|element| {
+        let element = node.as_element();
         if element.style_attribute().is_some() {
             return None
         }
@@ -228,7 +228,6 @@ impl StyleSharingCandidate {
             class: element.get_attr(&Null, "class")
                           .map(|string| string.to_str()),
         })
-        })
     }
 
     fn can_share_style_with(&self, element: &LayoutElement) -> bool {
@@ -401,7 +400,7 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
                 }
 
                 // Check tag names, classes, etc.
-                if !self.with_element(|element| candidate.can_share_style_with(element)) {
+                if !candidate.can_share_style_with(&self.as_element()) {
                     return None
                 }
 
@@ -419,9 +418,7 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
                   stylist: &Stylist,
                   applicable_declarations: &mut ApplicableDeclarations,
                   shareable: &mut bool) {
-        let style_attribute = self.with_element(|element| {
-            element.style_attribute().as_ref()
-        });
+        let style_attribute = self.as_element().style_attribute().as_ref();
 
         applicable_declarations.normal_shareable =
             stylist.push_applicable_declarations(self,
@@ -448,9 +445,10 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
         if !self.is_element() {
             return CannotShare(false)
         }
-        let ok = self.with_element(|element| {
+        let ok = {
+            let element = self.as_element();
             element.style_attribute().is_none() && element.get_attr(&Null, "id").is_none()
-        });
+        };
         if !ok {
             return CannotShare(false)
         }

src/components/main/layout/box_.rs

@@ -132,12 +132,11 @@ impl ImageBoxInfo {
                local_image_cache: MutexArc<LocalImageCache>)
                -> ImageBoxInfo {
         fn convert_length(node: &ThreadSafeLayoutNode, name: &str) -> Option<Au> {
-            node.with_element(|element| {
+            let element = node.as_element();
             element.get_attr(&namespace::Null, name).and_then(|string| {
                 let n: Option<int> = FromStr::from_str(string);
                 n
             }).and_then(|pixels| Some(Au::from_px(pixels)))
-            })
         }
 
         ImageBoxInfo {

src/components/main/layout/construct.rs

@@ -851,8 +851,8 @@ trait ObjectElement {
 
 impl<'ln> ObjectElement for ThreadSafeLayoutNode<'ln> {
     fn get_type_and_data(&self) -> (Option<&'static str>, Option<&'static str>) {
-        (self.with_element(|e| { e.get_attr(&namespace::Null, "type") } ),
-        self.with_element(|e| { e.get_attr(&namespace::Null, "data") } ))
+        let elem = self.as_element();
+        (elem.get_attr(&namespace::Null, "type"), elem.get_attr(&namespace::Null, "data"))
     }
 
     fn has_object_data(&self) -> bool {

src/components/main/layout/wrapper.rs

@@ -34,7 +34,7 @@
 //!     `html_element_in_html_document_for_layout()`.
 
 use extra::url::Url;
-use script::dom::bindings::codegen::InheritTypes::{ElementDerived, HTMLIFrameElementDerived};
+use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementDerived};
 use script::dom::bindings::codegen::InheritTypes::{HTMLImageElementDerived, TextDerived};
 use script::dom::bindings::js::JS;
 use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
@@ -228,16 +228,13 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
 
     /// If this is an element, accesses the element data. Fails if this is not an element node.
     #[inline]
-    fn with_element<R>(&self, f: |&LayoutElement<'ln>| -> R) -> R {
+    fn as_element(&self) -> LayoutElement<'ln> {
         unsafe {
-            if !self.node.is_element() {
-                fail!("not an element!")
-            }
             let elem: JS<Element> = self.node.transmute_copy();
             let element = elem.get();
-            f(&LayoutElement {
+            LayoutElement {
                 element: cast::transmute_region(element),
-            })
+            }
         }
     }
 
@@ -250,7 +247,7 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
     }
 
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool {
-        self.with_element(|element| {
+        let element = self.as_element();
         let name = unsafe {
             if element.element.html_element_in_html_document_for_layout() {
                 attr.lower_name.as_slice()
@@ -266,7 +263,6 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
             // FIXME: https://github.com/mozilla/servo/issues/1558
             AnyNamespace => false,
         }
-        })
     }
 }
 
@@ -433,18 +429,15 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
 
     /// If this is an element, accesses the element data. Fails if this is not an element node.
     #[inline]
-    pub fn with_element<R>(&self, f: |&ThreadSafeLayoutElement| -> R) -> R {
+    pub fn as_element(&self) -> ThreadSafeLayoutElement {
         unsafe {
-            if !self.node.is_element() {
-                fail!("not an element!")
-            }
             let elem: JS<Element> = self.node.transmute_copy();
             let element = elem.unsafe_get();
             // FIXME(pcwalton): Workaround until Rust gets multiple lifetime parameters on
             // implementations.
-            f(&ThreadSafeLayoutElement {
+            ThreadSafeLayoutElement {
                 element: cast::transmute::<*mut Element,&mut Element>(element),
-            })
+            }
         }
     }
 

src/components/style/node.rs

@@ -15,10 +15,7 @@ pub trait TNode<E:TElement> : Clone {
     fn next_sibling(&self) -> Option<Self>;
     fn is_document(&self) -> bool;
     fn is_element(&self) -> bool;
-
-    /// FIXME(pcwalton): This should not use the `with` pattern.
-    fn with_element<'a, R>(&self, f: |&E| -> R) -> R;
-
+    fn as_element(&self) -> E;
     fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool;
 }
 

src/components/style/selector_matching.rs

@@ -117,7 +117,7 @@ impl SelectorMap {
 
         // At the end, we're going to sort the rules that we added, so remember where we began.
         let init_len = matching_rules_list.len();
-        node.with_element(|element: &E| {
+        let element = node.as_element();
         match element.get_attr(&namespace::Null, "id") {
             Some(id) => {
                 SelectorMap::get_matching_rules_from_hash(node,
@@ -154,7 +154,6 @@ impl SelectorMap {
                                         self.universal_rules,
                                         matching_rules_list,
                                         shareable);
-        });
 
         // Sort only the rules we just added.
         sort::quicksort(matching_rules_list.mut_slice_from(init_len));
@@ -582,37 +581,33 @@ fn matches_simple_selector<E:TElement,
         // TODO: case-sensitivity depends on the document type
         // TODO: intern element names
         LocalNameSelector(ref name) => {
-            element.with_element(|element: &E| {
+            let element = element.as_element();
             element.get_local_name().eq_ignore_ascii_case(name.as_slice())
-            })
         }
 
         NamespaceSelector(ref namespace) => {
             *shareable = false;
-            element.with_element(|element: &E| {
+            let element = element.as_element();
             element.get_namespace() == namespace
-            })
         }
         // TODO: case-sensitivity depends on the document type and quirks mode
         // TODO: cache and intern IDs on elements.
         IDSelector(ref id) => {
             *shareable = false;
-            element.with_element(|element: &E| {
+            let element = element.as_element();
             element.get_attr(&namespace::Null, "id")
                     .map_or(false, |attr| {
                 attr == *id
             })
-            })
         }
         // TODO: cache and intern class names on elements.
         ClassSelector(ref class) => {
-            element.with_element(|element: &E| {
+            let element = element.as_element();
             element.get_attr(&namespace::Null, "class")
                     .map_or(false, |attr| {
                 // TODO: case-sensitivity depends on the document type and quirks mode
                 attr.split(SELECTOR_WHITESPACE).any(|c| c == class.as_slice())
             })
-            })
         }
 
         AttrExists(ref attr) => {
@@ -663,34 +658,30 @@ fn matches_simple_selector<E:TElement,
 
         AnyLink => {
             *shareable = false;
-            element.with_element(|element: &E| {
+            let element = element.as_element();
             element.get_link().is_some()
-            })
         }
         Link => {
             *shareable = false;
-            element.with_element(|element: &E| {
-                match element.get_link() {
+            let elem = element.as_element();
+            match elem.get_link() {
                 Some(url) => !url_is_visited(url),
                 None => false,
             }
-            })
         }
         Visited => {
             *shareable = false;
-            element.with_element(|element: &E| {
-                match element.get_link() {
+            let elem = element.as_element();
+            match elem.get_link() {
                 Some(url) => url_is_visited(url),
                 None => false,
             }
-            })
         }
 
         Hover => {
             *shareable = false;
-            element.with_element(|element: &E| {
-                element.get_hover_state()
-            })
+            let elem = element.as_element();
+            elem.get_hover_state()
         },
         FirstChild => {
             *shareable = false;
@@ -791,14 +782,12 @@ fn matches_generic_nth_child<'a,
 
         if node.is_element() {
             if is_of_type {
-                element.with_element(|element: &E| {
-                    node.with_element(|node: &E| {
+                let element = element.as_element();
+                let node = node.as_element();
                 if element.get_local_name() == node.get_local_name() &&
                     element.get_namespace() == node.get_namespace() {
                     index += 1;
                 }
-                    })
-                })
             } else {
               index += 1;
             }