Introduce <LayoutDom<Element>>::attrs()

This safe method is the basic block to access element attributes from layout.
We reuse it in the other attr-related layout methods to remove a pretty big
source of rampant unsafe code between script and layout.

components/layout_thread/dom_wrapper.rs

@@ -489,14 +489,12 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     where
         F: FnMut(&Atom),
     {
-        unsafe {
         if let Some(ref classes) = self.element.get_classes_for_layout() {
             for class in *classes {
                 callback(class)
             }
         }
     }
-    }
 
     fn has_dirty_descendants(&self) -> bool {
         unsafe {
@@ -698,12 +696,12 @@ impl<'le> ServoLayoutElement<'le> {
 
     #[inline]
     fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> {
-        unsafe { self.element.get_attr_for_layout(namespace, name) }
+        self.element.get_attr_for_layout(namespace, name)
     }
 
     #[inline]
     fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> {
-        unsafe { self.element.get_attr_val_for_layout(namespace, name) }
+        self.element.get_attr_val_for_layout(namespace, name)
     }
 
     fn get_style_data(&self) -> Option<&StyleData> {
@@ -805,10 +803,11 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
             NamespaceConstraint::Specific(ref ns) => self
                 .get_attr_enum(ns, local_name)
                 .map_or(false, |value| value.eval_selector(operation)),
-            NamespaceConstraint::Any => {
+            NamespaceConstraint::Any => self
-                let values = unsafe { self.element.get_attr_vals_for_layout(local_name) };
+                .element
-                values.iter().any(|value| value.eval_selector(operation))
+                .get_attr_vals_for_layout(local_name)
-            },
+                .iter()
+                .any(|value| value.eval_selector(operation)),
         }
     }
 
@@ -878,7 +877,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
             NonTSPseudoClass::Lang(ref lang) => self.match_element_lang(None, &*lang),
 
-            NonTSPseudoClass::ServoNonZeroBorder => unsafe {
+            NonTSPseudoClass::ServoNonZeroBorder => {
                 match self
                     .element
                     .get_attr_for_layout(&ns!(), &local_name!("border"))
@@ -912,25 +911,20 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
     #[inline]
     fn is_link(&self) -> bool {
-        unsafe {
         match self.as_node().script_type_id() {
             // https://html.spec.whatwg.org/multipage/#selector-link
             NodeTypeId::Element(ElementTypeId::HTMLElement(
                 HTMLElementTypeId::HTMLAnchorElement,
             )) |
-                NodeTypeId::Element(ElementTypeId::HTMLElement(
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAreaElement)) |
-                    HTMLElementTypeId::HTMLAreaElement,
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) => {
-                )) |
+                self.element
-                NodeTypeId::Element(ElementTypeId::HTMLElement(
-                    HTMLElementTypeId::HTMLLinkElement,
-                )) => self
-                    .element
                     .get_attr_val_for_layout(&ns!(), &local_name!("href"))
-                    .is_some(),
+                    .is_some()
+            },
             _ => false,
         }
     }
-    }
 
     #[inline]
     fn has_id(&self, id: &Atom, case_sensitivity: CaseSensitivity) -> bool {
@@ -956,7 +950,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
     #[inline]
     fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
-        unsafe { self.element.has_class_for_layout(name, case_sensitivity) }
+        self.element.has_class_for_layout(name, case_sensitivity)
     }
 
     fn is_html_slot_element(&self) -> bool {
@@ -1439,10 +1433,12 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
             NamespaceConstraint::Specific(ref ns) => self
                 .get_attr_enum(ns, local_name)
                 .map_or(false, |value| value.eval_selector(operation)),
-            NamespaceConstraint::Any => {
+            NamespaceConstraint::Any => self
-                let values = unsafe { self.element.element.get_attr_vals_for_layout(local_name) };
+                .element
-                values.iter().any(|v| v.eval_selector(operation))
+                .element
-            },
+                .get_attr_vals_for_layout(local_name)
+                .iter()
+                .any(|v| v.eval_selector(operation)),
         }
     }
 

components/layout_thread_2020/dom_wrapper.rs

@@ -496,14 +496,12 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     where
         F: FnMut(&Atom),
     {
-        unsafe {
         if let Some(ref classes) = self.element.get_classes_for_layout() {
             for class in *classes {
                 callback(class)
             }
         }
     }
-    }
 
     fn has_dirty_descendants(&self) -> bool {
         unsafe {
@@ -705,12 +703,12 @@ impl<'le> ServoLayoutElement<'le> {
 
     #[inline]
     fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> {
-        unsafe { self.element.get_attr_for_layout(namespace, name) }
+        self.element.get_attr_for_layout(namespace, name)
     }
 
     #[inline]
     fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> {
-        unsafe { self.element.get_attr_val_for_layout(namespace, name) }
+        self.element.get_attr_val_for_layout(namespace, name)
     }
 
     fn get_style_data(&self) -> Option<&StyleData> {
@@ -812,10 +810,11 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
             NamespaceConstraint::Specific(ref ns) => self
                 .get_attr_enum(ns, local_name)
                 .map_or(false, |value| value.eval_selector(operation)),
-            NamespaceConstraint::Any => {
+            NamespaceConstraint::Any => self
-                let values = unsafe { self.element.get_attr_vals_for_layout(local_name) };
+                .element
-                values.iter().any(|value| value.eval_selector(operation))
+                .get_attr_vals_for_layout(local_name)
-            },
+                .iter()
+                .any(|value| value.eval_selector(operation)),
         }
     }
 
@@ -885,7 +884,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
             NonTSPseudoClass::Lang(ref lang) => self.match_element_lang(None, &*lang),
 
-            NonTSPseudoClass::ServoNonZeroBorder => unsafe {
+            NonTSPseudoClass::ServoNonZeroBorder => {
                 match self
                     .element
                     .get_attr_for_layout(&ns!(), &local_name!("border"))
@@ -919,25 +918,20 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
     #[inline]
     fn is_link(&self) -> bool {
-        unsafe {
         match self.as_node().script_type_id() {
             // https://html.spec.whatwg.org/multipage/#selector-link
             NodeTypeId::Element(ElementTypeId::HTMLElement(
                 HTMLElementTypeId::HTMLAnchorElement,
             )) |
-                NodeTypeId::Element(ElementTypeId::HTMLElement(
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAreaElement)) |
-                    HTMLElementTypeId::HTMLAreaElement,
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) => {
-                )) |
+                self.element
-                NodeTypeId::Element(ElementTypeId::HTMLElement(
-                    HTMLElementTypeId::HTMLLinkElement,
-                )) => self
-                    .element
                     .get_attr_val_for_layout(&ns!(), &local_name!("href"))
-                    .is_some(),
+                    .is_some()
+            },
             _ => false,
         }
     }
-    }
 
     #[inline]
     fn has_id(&self, id: &Atom, case_sensitivity: CaseSensitivity) -> bool {
@@ -963,7 +957,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
     #[inline]
     fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
-        unsafe { self.element.has_class_for_layout(name, case_sensitivity) }
+        self.element.has_class_for_layout(name, case_sensitivity)
     }
 
     fn is_html_slot_element(&self) -> bool {
@@ -1447,7 +1441,7 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
                 .get_attr_enum(ns, local_name)
                 .map_or(false, |value| value.eval_selector(operation)),
             NamespaceConstraint::Any => {
-                let values = unsafe { self.element.element.get_attr_vals_for_layout(local_name) };
+                let values = self.element.element.get_attr_vals_for_layout(local_name);
                 values.iter().any(|v| v.eval_selector(operation))
             },
         }

components/script/dom/element.rs

@@ -551,28 +551,21 @@ impl Element {
 }
 
 #[inline]
-#[allow(unsafe_code)]
+pub fn get_attr_for_layout<'dom>(
-pub unsafe fn get_attr_for_layout<'dom>(
+    elem: LayoutDom<'dom, Element>,
-    elem: &'dom Element,
     namespace: &Namespace,
     name: &LocalName,
 ) -> Option<LayoutDom<'dom, Attr>> {
-    // cast to point to T in RefCell<T> directly
+    elem.attrs()
-    let attrs = elem.attrs.borrow_for_layout();
-    attrs
         .iter()
-        .find(|attr| {
+        .find(|attr| name == attr.local_name() && namespace == attr.namespace())
-            let attr = attr.to_layout();
+        .cloned()
-            name == attr.local_name() && namespace == attr.namespace()
-        })
-        .map(|attr| attr.to_layout())
 }
 
 pub trait LayoutElementHelpers<'dom> {
-    #[allow(unsafe_code)]
+    fn attrs(self) -> &'dom [LayoutDom<'dom, Attr>];
-    unsafe fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool;
+    fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool;
-    #[allow(unsafe_code)]
+    fn get_classes_for_layout(self) -> Option<&'dom [Atom]>;
-    unsafe fn get_classes_for_layout(self) -> Option<&'dom [Atom]>;
 
     #[allow(unsafe_code)]
     unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(self, _: &mut V)
@@ -595,41 +588,42 @@ pub trait LayoutElementHelpers<'dom> {
     /// The shadow root this element is a host of.
     #[allow(unsafe_code)]
     unsafe fn get_shadow_root_for_layout(self) -> Option<LayoutDom<'dom, ShadowRoot>>;
-    #[allow(unsafe_code)]
+    fn get_attr_for_layout(
-    unsafe fn get_attr_for_layout(
         self,
         namespace: &Namespace,
         name: &LocalName,
     ) -> Option<&'dom AttrValue>;
-    #[allow(unsafe_code)]
+    fn get_attr_val_for_layout(self, namespace: &Namespace, name: &LocalName) -> Option<&'dom str>;
-    unsafe fn get_attr_val_for_layout(
+    fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue>;
-        self,
-        namespace: &Namespace,
-        name: &LocalName,
-    ) -> Option<&'dom str>;
-    #[allow(unsafe_code)]
-    unsafe fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue>;
 }
 
 impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
     #[allow(unsafe_code)]
     #[inline]
-    unsafe fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
+    fn attrs(self) -> &'dom [LayoutDom<'dom, Attr>] {
-        get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class")).map_or(
+        unsafe {
-            false,
+            // FIXME(nox): This should probably be done through a ToLayout trait.
-            |attr| {
+            let attrs: &[Dom<Attr>] = &self.unsafe_get().attrs.borrow_for_layout();
+            // This doesn't compile if Dom and LayoutDom don't have the same
+            // representation.
+            let _ = mem::transmute::<Dom<Attr>, LayoutDom<Attr>>;
+            &*(attrs as *const [Dom<Attr>] as *const [LayoutDom<Attr>])
+        }
+    }
+
+    #[inline]
+    fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
+        get_attr_for_layout(self, &ns!(), &local_name!("class")).map_or(false, |attr| {
             attr.as_tokens()
                 .unwrap()
                 .iter()
                 .any(|atom| case_sensitivity.eq_atom(atom, name))
-            },
+        })
-        )
     }
 
-    #[allow(unsafe_code)]
     #[inline]
-    unsafe fn get_classes_for_layout(self) -> Option<&'dom [Atom]> {
+    fn get_classes_for_layout(self) -> Option<&'dom [Atom]> {
-        get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class"))
+        get_attr_for_layout(self, &ns!(), &local_name!("class"))
             .map(|attr| attr.as_tokens().unwrap())
     }
 
@@ -1066,34 +1060,25 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
             .map(|sr| sr.to_layout())
     }
 
-    #[allow(unsafe_code)]
     #[inline]
-    unsafe fn get_attr_for_layout(
+    fn get_attr_for_layout(
         self,
         namespace: &Namespace,
         name: &LocalName,
     ) -> Option<&'dom AttrValue> {
-        get_attr_for_layout(self.unsafe_get(), namespace, name).map(|attr| attr.value())
+        get_attr_for_layout(self, namespace, name).map(|attr| attr.value())
     }
 
-    #[allow(unsafe_code)]
     #[inline]
-    unsafe fn get_attr_val_for_layout(
+    fn get_attr_val_for_layout(self, namespace: &Namespace, name: &LocalName) -> Option<&'dom str> {
-        self,
+        get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str())
-        namespace: &Namespace,
-        name: &LocalName,
-    ) -> Option<&'dom str> {
-        get_attr_for_layout(self.unsafe_get(), namespace, name).map(|attr| attr.as_str())
     }
 
-    #[allow(unsafe_code)]
     #[inline]
-    unsafe fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue> {
+    fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue> {
-        let attrs = self.unsafe_get().attrs.borrow_for_layout();
+        self.attrs()
-        attrs
             .iter()
             .filter_map(|attr| {
-                let attr = attr.to_layout();
                 if name == attr.local_name() {
                     Some(attr.value())
                 } else {

components/script/dom/htmlbodyelement.rs

@@ -100,35 +100,26 @@ pub trait HTMLBodyElementLayoutHelpers {
 }
 
 impl HTMLBodyElementLayoutHelpers for LayoutDom<'_, HTMLBodyElement> {
-    #[allow(unsafe_code)]
     fn get_background_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("bgcolor"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("text"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_background(self) -> Option<ServoUrl> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("background"))
             .and_then(AttrValue::as_resolved_url)
             .cloned()
     }
-    }
 }
 
 impl VirtualMethods for HTMLBodyElement {

components/script/dom/htmlcanvaselement.rs

@@ -152,25 +152,19 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> {
         }
     }
 
-    #[allow(unsafe_code)]
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_uint_px_dimension)
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_height(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("height"))
             .map(AttrValue::as_uint_px_dimension)
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 
     #[allow(unsafe_code)]
     fn get_canvas_id_for_layout(self) -> CanvasId {

components/script/dom/htmlfontelement.rs

@@ -107,32 +107,24 @@ pub trait HTMLFontElementLayoutHelpers {
 }
 
 impl HTMLFontElementLayoutHelpers for LayoutDom<'_, HTMLFontElement> {
-    #[allow(unsafe_code)]
     fn get_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("color"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_face(self) -> Option<Atom> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("face"))
             .map(AttrValue::as_atom)
             .cloned()
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_size(self) -> Option<u32> {
-        let size = unsafe {
+        let size = self
-            self.upcast::<Element>()
+            .upcast::<Element>()
-                .get_attr_for_layout(&ns!(), &local_name!("size"))
+            .get_attr_for_layout(&ns!(), &local_name!("size"));
-        };
         match size {
             Some(&AttrValue::UInt(_, s)) => Some(s),
             _ => None,

components/script/dom/htmlhrelement.rs

@@ -71,26 +71,20 @@ pub trait HTMLHRLayoutHelpers {
 }
 
 impl HTMLHRLayoutHelpers for LayoutDom<'_, HTMLHRElement> {
-    #[allow(unsafe_code)]
     fn get_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("color"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 }
 
 impl VirtualMethods for HTMLHRElement {

components/script/dom/htmliframeelement.rs

@@ -499,27 +499,21 @@ impl HTMLIFrameElementLayoutMethods for LayoutDom<'_, HTMLIFrameElement> {
         unsafe { (*self.unsafe_get()).browsing_context_id.get() }
     }
 
-    #[allow(unsafe_code)]
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_height(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("height"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 }
 
 impl HTMLIFrameElementMethods for HTMLIFrameElement {

components/script/dom/htmlimageelement.rs

@@ -1415,27 +1415,21 @@ impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> {
             .clone()
     }
 
-    #[allow(unsafe_code)]
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_height(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("height"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 }
 
 //https://html.spec.whatwg.org/multipage/#parse-a-sizes-attribute

components/script/dom/htmlinputelement.rs

@@ -730,14 +730,12 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem
             input: LayoutDom<'dom, HTMLInputElement>,
             default: &'static str,
         ) -> Cow<'dom, str> {
-            unsafe {
             input
                 .upcast::<Element>()
                 .get_attr_val_for_layout(&ns!(), &local_name!("value"))
                 .unwrap_or(default)
                 .into()
         }
-        }
 
         let placeholder = unsafe { &**self.unsafe_get().placeholder.borrow_for_layout() };
         match unsafe { self.unsafe_get().input_type() } {

components/script/dom/htmltablecellelement.rs

@@ -104,42 +104,33 @@ pub trait HTMLTableCellElementLayoutHelpers {
     fn get_width(self) -> LengthOrPercentageOrAuto;
 }
 
-#[allow(unsafe_code)]
 impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> {
     fn get_background_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("bgcolor"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
     fn get_colspan(self) -> Option<u32> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("colspan"))
             .map(AttrValue::as_uint)
     }
-    }
 
     fn get_rowspan(self) -> Option<u32> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("rowspan"))
             .map(AttrValue::as_uint)
     }
-    }
 
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 }
 
 impl VirtualMethods for HTMLTableCellElement {

components/script/dom/htmltableelement.rs

@@ -413,15 +413,12 @@ pub trait HTMLTableElementLayoutHelpers {
 }
 
 impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> {
-    #[allow(unsafe_code)]
     fn get_background_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("bgcolor"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 
     #[allow(unsafe_code)]
     fn get_border(self) -> Option<u32> {
@@ -433,16 +430,13 @@ impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> {
         unsafe { (*self.unsafe_get()).cellspacing.get() }
     }
 
-    #[allow(unsafe_code)]
     fn get_width(self) -> LengthOrPercentageOrAuto {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"))
             .map(AttrValue::as_dimension)
             .cloned()
             .unwrap_or(LengthOrPercentageOrAuto::Auto)
     }
-    }
 }
 
 impl VirtualMethods for HTMLTableElement {

components/script/dom/htmltablerowelement.rs

@@ -149,16 +149,13 @@ pub trait HTMLTableRowElementLayoutHelpers {
     fn get_background_color(self) -> Option<RGBA>;
 }
 
-#[allow(unsafe_code)]
 impl HTMLTableRowElementLayoutHelpers for LayoutDom<'_, HTMLTableRowElement> {
     fn get_background_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("bgcolor"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 }
 
 impl VirtualMethods for HTMLTableRowElement {

components/script/dom/htmltablesectionelement.rs

@@ -87,16 +87,13 @@ pub trait HTMLTableSectionElementLayoutHelpers {
     fn get_background_color(self) -> Option<RGBA>;
 }
 
-#[allow(unsafe_code)]
 impl HTMLTableSectionElementLayoutHelpers for LayoutDom<'_, HTMLTableSectionElement> {
     fn get_background_color(self) -> Option<RGBA> {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("bgcolor"))
             .and_then(AttrValue::as_color)
             .cloned()
     }
-    }
 }
 
 impl VirtualMethods for HTMLTableSectionElement {

components/script/dom/htmltextareaelement.rs

@@ -96,23 +96,17 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> {
         ))
     }
 
-    #[allow(unsafe_code)]
     fn get_cols(self) -> u32 {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("cols"))
             .map_or(DEFAULT_COLS, AttrValue::as_uint)
     }
-    }
 
-    #[allow(unsafe_code)]
     fn get_rows(self) -> u32 {
-        unsafe {
         self.upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("rows"))
             .map_or(DEFAULT_ROWS, AttrValue::as_uint)
     }
-    }
 }
 
 // https://html.spec.whatwg.org/multipage/#attr-textarea-cols-value

components/script/dom/svgsvgelement.rs

@@ -53,9 +53,7 @@ pub trait LayoutSVGSVGElementHelpers {
 }
 
 impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> {
-    #[allow(unsafe_code, non_snake_case)]
     fn data(self) -> SVGSVGData {
-        unsafe {
         let width_attr = self
             .upcast::<Element>()
             .get_attr_for_layout(&ns!(), &local_name!("width"));
@@ -67,7 +65,6 @@ impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> {
             height: height_attr.map_or(DEFAULT_HEIGHT, |val| val.as_uint()),
         }
     }
-    }
 }
 
 impl VirtualMethods for SVGSVGElement {