From 3b504148d5d4fef9f651e73df90ad8c9e9fb09a5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 18:02:02 +0200 Subject: [PATCH 01/23] Move around some code in the element The intent is to merge the two layout helper traits together so I'm moving them around to make later diffs more readable. --- components/script/dom/element.rs | 104 +++++++++++++++---------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 7d167bf2574..3acbd647107 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -550,21 +550,6 @@ impl Element { } } -#[allow(unsafe_code)] -pub trait RawLayoutElementHelpers { - unsafe fn get_attr_for_layout<'a>( - &'a self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'a AttrValue>; - unsafe fn get_attr_val_for_layout<'a>( - &'a self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'a str>; - unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>; -} - #[inline] #[allow(unsafe_code)] pub unsafe fn get_attr_for_layout<'dom>( @@ -583,43 +568,6 @@ pub unsafe fn get_attr_for_layout<'dom>( .map(|attr| attr.to_layout()) } -#[allow(unsafe_code)] -impl RawLayoutElementHelpers for Element { - #[inline] - unsafe fn get_attr_for_layout<'a>( - &'a self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'a AttrValue> { - get_attr_for_layout(self, namespace, name).map(|attr| attr.value()) - } - - #[inline] - unsafe fn get_attr_val_for_layout<'a>( - &'a self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'a str> { - get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str()) - } - - #[inline] - unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> { - let attrs = self.attrs.borrow_for_layout(); - attrs - .iter() - .filter_map(|attr| { - let attr = attr.to_layout(); - if name == attr.local_name() { - Some(attr.value()) - } else { - None - } - }) - .collect() - } -} - pub trait LayoutElementHelpers<'dom> { #[allow(unsafe_code)] unsafe fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool; @@ -649,6 +597,21 @@ pub trait LayoutElementHelpers<'dom> { unsafe fn get_shadow_root_for_layout(self) -> Option>; } +#[allow(unsafe_code)] +pub trait RawLayoutElementHelpers { + unsafe fn get_attr_for_layout<'a>( + &'a self, + namespace: &Namespace, + name: &LocalName, + ) -> Option<&'a AttrValue>; + unsafe fn get_attr_val_for_layout<'a>( + &'a self, + namespace: &Namespace, + name: &LocalName, + ) -> Option<&'a str>; + unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>; +} + impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[allow(unsafe_code)] #[inline] @@ -1105,6 +1068,43 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { } } +#[allow(unsafe_code)] +impl RawLayoutElementHelpers for Element { + #[inline] + unsafe fn get_attr_for_layout<'a>( + &'a self, + namespace: &Namespace, + name: &LocalName, + ) -> Option<&'a AttrValue> { + get_attr_for_layout(self, namespace, name).map(|attr| attr.value()) + } + + #[inline] + unsafe fn get_attr_val_for_layout<'a>( + &'a self, + namespace: &Namespace, + name: &LocalName, + ) -> Option<&'a str> { + get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str()) + } + + #[inline] + unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> { + let attrs = self.attrs.borrow_for_layout(); + attrs + .iter() + .filter_map(|attr| { + let attr = attr.to_layout(); + if name == attr.local_name() { + Some(attr.value()) + } else { + None + } + }) + .collect() + } +} + impl Element { pub fn is_html_element(&self) -> bool { self.namespace == ns!(html) From 0bda1748230b4d54c274a014d770daf461fec50f Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 18:30:42 +0200 Subject: [PATCH 02/23] Merge RawLayoutElementHelpers into LayoutElementHelpers --- components/layout_thread/dom_wrapper.rs | 18 +++--- components/layout_thread_2020/dom_wrapper.rs | 18 +++--- components/script/dom/element.rs | 55 +++++++++---------- components/script/dom/htmlbodyelement.rs | 8 +-- components/script/dom/htmlcanvaselement.rs | 13 ++--- components/script/dom/htmlfontelement.rs | 8 +-- components/script/dom/htmlhrelement.rs | 6 +- components/script/dom/htmliframeelement.rs | 6 +- components/script/dom/htmlimageelement.rs | 6 +- components/script/dom/htmlinputelement.rs | 5 +- components/script/dom/htmltablecellelement.rs | 10 ++-- components/script/dom/htmltableelement.rs | 6 +- components/script/dom/htmltablerowelement.rs | 4 +- .../script/dom/htmltablesectionelement.rs | 4 +- components/script/dom/htmltextareaelement.rs | 6 +- components/script/dom/svgsvgelement.rs | 8 +-- components/script/lib.rs | 2 +- 17 files changed, 86 insertions(+), 97 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index e3197c8e78a..adb1a915d57 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -48,7 +48,6 @@ use script::layout_exports::{Document, Element, Node, Text}; use script::layout_exports::{LayoutCharacterDataHelpers, LayoutDocumentHelpers}; use script::layout_exports::{ LayoutDom, LayoutElementHelpers, LayoutNodeHelpers, LayoutShadowRootHelpers, - RawLayoutElementHelpers, }; use script_layout_interface::wrapper_traits::{ DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode, @@ -699,12 +698,12 @@ impl<'le> ServoLayoutElement<'le> { #[inline] fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> { - unsafe { (*self.element.unsafe_get()).get_attr_for_layout(namespace, name) } + unsafe { self.element.get_attr_for_layout(namespace, name) } } #[inline] fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> { - unsafe { (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name) } + unsafe { self.element.get_attr_val_for_layout(namespace, name) } } fn get_style_data(&self) -> Option<&StyleData> { @@ -807,8 +806,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { .get_attr_enum(ns, local_name) .map_or(false, |value| value.eval_selector(operation)), NamespaceConstraint::Any => { - let values = - unsafe { (*self.element.unsafe_get()).get_attr_vals_for_layout(local_name) }; + let values = unsafe { self.element.get_attr_vals_for_layout(local_name) }; values.iter().any(|value| value.eval_selector(operation)) }, } @@ -881,7 +879,8 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { NonTSPseudoClass::Lang(ref lang) => self.match_element_lang(None, &*lang), NonTSPseudoClass::ServoNonZeroBorder => unsafe { - match (*self.element.unsafe_get()) + match self + .element .get_attr_for_layout(&ns!(), &local_name!("border")) { None | Some(&AttrValue::UInt(_, 0)) => false, @@ -924,7 +923,8 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { )) | NodeTypeId::Element(ElementTypeId::HTMLElement( HTMLElementTypeId::HTMLLinkElement, - )) => (*self.element.unsafe_get()) + )) => self + .element .get_attr_val_for_layout(&ns!(), &local_name!("href")) .is_some(), _ => false, @@ -1440,9 +1440,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.unsafe_get()).get_attr_vals_for_layout(local_name) - }; + let values = unsafe { self.element.element.get_attr_vals_for_layout(local_name) }; values.iter().any(|v| v.eval_selector(operation)) }, } diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 7d57d7f511e..3bfc5abbfea 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -48,7 +48,6 @@ use script::layout_exports::{Document, Element, Node, Text}; use script::layout_exports::{LayoutCharacterDataHelpers, LayoutDocumentHelpers}; use script::layout_exports::{ LayoutDom, LayoutElementHelpers, LayoutNodeHelpers, LayoutShadowRootHelpers, - RawLayoutElementHelpers, }; use script_layout_interface::wrapper_traits::{ DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode, @@ -706,12 +705,12 @@ impl<'le> ServoLayoutElement<'le> { #[inline] fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> { - unsafe { (*self.element.unsafe_get()).get_attr_for_layout(namespace, name) } + unsafe { self.element.get_attr_for_layout(namespace, name) } } #[inline] fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> { - unsafe { (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name) } + unsafe { self.element.get_attr_val_for_layout(namespace, name) } } fn get_style_data(&self) -> Option<&StyleData> { @@ -814,8 +813,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { .get_attr_enum(ns, local_name) .map_or(false, |value| value.eval_selector(operation)), NamespaceConstraint::Any => { - let values = - unsafe { (*self.element.unsafe_get()).get_attr_vals_for_layout(local_name) }; + let values = unsafe { self.element.get_attr_vals_for_layout(local_name) }; values.iter().any(|value| value.eval_selector(operation)) }, } @@ -888,7 +886,8 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { NonTSPseudoClass::Lang(ref lang) => self.match_element_lang(None, &*lang), NonTSPseudoClass::ServoNonZeroBorder => unsafe { - match (*self.element.unsafe_get()) + match self + .element .get_attr_for_layout(&ns!(), &local_name!("border")) { None | Some(&AttrValue::UInt(_, 0)) => false, @@ -931,7 +930,8 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { )) | NodeTypeId::Element(ElementTypeId::HTMLElement( HTMLElementTypeId::HTMLLinkElement, - )) => (*self.element.unsafe_get()) + )) => self + .element .get_attr_val_for_layout(&ns!(), &local_name!("href")) .is_some(), _ => false, @@ -1447,9 +1447,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.unsafe_get()).get_attr_vals_for_layout(local_name) - }; + let values = unsafe { self.element.element.get_attr_vals_for_layout(local_name) }; values.iter().any(|v| v.eval_selector(operation)) }, } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 3acbd647107..add49150a27 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -595,21 +595,20 @@ 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>; -} - -#[allow(unsafe_code)] -pub trait RawLayoutElementHelpers { - unsafe fn get_attr_for_layout<'a>( - &'a self, + #[allow(unsafe_code)] + unsafe fn get_attr_for_layout( + self, namespace: &Namespace, name: &LocalName, - ) -> Option<&'a AttrValue>; - unsafe fn get_attr_val_for_layout<'a>( - &'a self, + ) -> Option<&'dom AttrValue>; + #[allow(unsafe_code)] + unsafe fn get_attr_val_for_layout( + self, namespace: &Namespace, name: &LocalName, - ) -> Option<&'a str>; - unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>; + ) -> 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> { @@ -765,7 +764,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { let size = if let Some(this) = self.downcast::() { // FIXME(pcwalton): More use of atoms, please! - match (*self.unsafe_get()).get_attr_val_for_layout(&ns!(), &local_name!("type")) { + match self.get_attr_val_for_layout(&ns!(), &local_name!("type")) { // Not text entry widget Some("hidden") | Some("date") | @@ -1012,15 +1011,15 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { let mut current_node = Some(self.upcast::()); while let Some(node) = current_node { current_node = node.composed_parent_node_ref(); - match node.downcast::().map(|el| el.unsafe_get()) { + match node.downcast::() { Some(elem) => { if let Some(attr) = - (*elem).get_attr_val_for_layout(&ns!(xml), &local_name!("lang")) + elem.get_attr_val_for_layout(&ns!(xml), &local_name!("lang")) { return attr.to_owned(); } if let Some(attr) = - (*elem).get_attr_val_for_layout(&ns!(), &local_name!("lang")) + elem.get_attr_val_for_layout(&ns!(), &local_name!("lang")) { return attr.to_owned(); } @@ -1066,31 +1065,31 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { .as_ref() .map(|sr| sr.to_layout()) } -} -#[allow(unsafe_code)] -impl RawLayoutElementHelpers for Element { + #[allow(unsafe_code)] #[inline] - unsafe fn get_attr_for_layout<'a>( - &'a self, + unsafe fn get_attr_for_layout( + self, namespace: &Namespace, name: &LocalName, - ) -> Option<&'a AttrValue> { - get_attr_for_layout(self, namespace, name).map(|attr| attr.value()) + ) -> Option<&'dom AttrValue> { + get_attr_for_layout(self.unsafe_get(), namespace, name).map(|attr| attr.value()) } + #[allow(unsafe_code)] #[inline] - unsafe fn get_attr_val_for_layout<'a>( - &'a self, + unsafe fn get_attr_val_for_layout( + self, namespace: &Namespace, name: &LocalName, - ) -> Option<&'a str> { - get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str()) + ) -> 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<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> { - let attrs = self.attrs.borrow_for_layout(); + unsafe fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue> { + let attrs = self.unsafe_get().attrs.borrow_for_layout(); attrs .iter() .filter_map(|attr| { diff --git a/components/script/dom/htmlbodyelement.rs b/components/script/dom/htmlbodyelement.rs index 1d9d2dc69b8..4bf835cbee9 100644 --- a/components/script/dom/htmlbodyelement.rs +++ b/components/script/dom/htmlbodyelement.rs @@ -9,7 +9,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::eventtarget::EventTarget; use crate::dom::htmlelement::HTMLElement; use crate::dom::node::{document_from_node, window_from_node, BindContext, Node}; @@ -103,7 +103,7 @@ impl HTMLBodyElementLayoutHelpers for LayoutDom<'_, HTMLBodyElement> { #[allow(unsafe_code)] fn get_background_color(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) .and_then(AttrValue::as_color) .cloned() @@ -113,7 +113,7 @@ impl HTMLBodyElementLayoutHelpers for LayoutDom<'_, HTMLBodyElement> { #[allow(unsafe_code)] fn get_color(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("text")) .and_then(AttrValue::as_color) .cloned() @@ -123,7 +123,7 @@ impl HTMLBodyElementLayoutHelpers for LayoutDom<'_, HTMLBodyElement> { #[allow(unsafe_code)] fn get_background(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("background")) .and_then(AttrValue::as_resolved_url) .cloned() diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index c230eb77337..dda0f0b1536 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -18,7 +18,7 @@ use crate::dom::canvasrenderingcontext2d::{ CanvasRenderingContext2D, LayoutCanvasRenderingContext2DHelpers, }; use crate::dom::document::Document; -use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::globalscope::GlobalScope; use crate::dom::htmlelement::HTMLElement; use crate::dom::node::{window_from_node, Node}; @@ -124,8 +124,7 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { #[allow(unsafe_code)] fn data(self) -> HTMLCanvasData { unsafe { - let canvas = &*self.unsafe_get(); - let source = match canvas.context.borrow_for_layout().as_ref() { + let source = match self.unsafe_get().context.borrow_for_layout().as_ref() { Some(&CanvasContext::Context2d(ref context)) => { HTMLCanvasDataSource::Image(Some(context.to_layout().get_ipc_renderer())) }, @@ -138,10 +137,10 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { None => HTMLCanvasDataSource::Image(None), }; - let width_attr = canvas + let width_attr = self .upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")); - let height_attr = canvas + let height_attr = self .upcast::() .get_attr_for_layout(&ns!(), &local_name!("height")); HTMLCanvasData { @@ -156,7 +155,7 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_uint_px_dimension) .unwrap_or(LengthOrPercentageOrAuto::Auto) @@ -166,7 +165,7 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { #[allow(unsafe_code)] fn get_height(self) -> LengthOrPercentageOrAuto { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("height")) .map(AttrValue::as_uint_px_dimension) .unwrap_or(LengthOrPercentageOrAuto::Auto) diff --git a/components/script/dom/htmlfontelement.rs b/components/script/dom/htmlfontelement.rs index 5dbb8fb1bff..fd40c6907f4 100644 --- a/components/script/dom/htmlfontelement.rs +++ b/components/script/dom/htmlfontelement.rs @@ -8,7 +8,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{Element, RawLayoutElementHelpers}; +use crate::dom::element::{Element, LayoutElementHelpers}; use crate::dom::htmlelement::HTMLElement; use crate::dom::node::Node; use crate::dom::virtualmethods::VirtualMethods; @@ -110,7 +110,7 @@ impl HTMLFontElementLayoutHelpers for LayoutDom<'_, HTMLFontElement> { #[allow(unsafe_code)] fn get_color(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("color")) .and_then(AttrValue::as_color) .cloned() @@ -120,7 +120,7 @@ impl HTMLFontElementLayoutHelpers for LayoutDom<'_, HTMLFontElement> { #[allow(unsafe_code)] fn get_face(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("face")) .map(AttrValue::as_atom) .cloned() @@ -130,7 +130,7 @@ impl HTMLFontElementLayoutHelpers for LayoutDom<'_, HTMLFontElement> { #[allow(unsafe_code)] fn get_size(self) -> Option { let size = unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("size")) }; match size { diff --git a/components/script/dom/htmlhrelement.rs b/components/script/dom/htmlhrelement.rs index 4f4e7fc1c13..9be680b9043 100644 --- a/components/script/dom/htmlhrelement.rs +++ b/components/script/dom/htmlhrelement.rs @@ -7,7 +7,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{Element, RawLayoutElementHelpers}; +use crate::dom::element::{Element, LayoutElementHelpers}; use crate::dom::htmlelement::HTMLElement; use crate::dom::node::Node; use crate::dom::virtualmethods::VirtualMethods; @@ -74,7 +74,7 @@ impl HTMLHRLayoutHelpers for LayoutDom<'_, HTMLHRElement> { #[allow(unsafe_code)] fn get_color(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("color")) .and_then(AttrValue::as_color) .cloned() @@ -84,7 +84,7 @@ impl HTMLHRLayoutHelpers for LayoutDom<'_, HTMLHRElement> { #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_dimension) .cloned() diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 7b8c89bebf9..7b98f57a524 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -14,7 +14,7 @@ use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::document::Document; use crate::dom::domtokenlist::DOMTokenList; -use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::eventtarget::EventTarget; use crate::dom::globalscope::GlobalScope; use crate::dom::htmlelement::HTMLElement; @@ -502,7 +502,7 @@ impl HTMLIFrameElementLayoutMethods for LayoutDom<'_, HTMLIFrameElement> { #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_dimension) .cloned() @@ -513,7 +513,7 @@ impl HTMLIFrameElementLayoutMethods for LayoutDom<'_, HTMLIFrameElement> { #[allow(unsafe_code)] fn get_height(self) -> LengthOrPercentageOrAuto { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("height")) .map(AttrValue::as_dimension) .cloned() diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 5afbb4f0535..626f23b69f8 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -22,7 +22,7 @@ use crate::dom::document::Document; use crate::dom::element::{cors_setting_for_element, referrer_policy_for_element}; use crate::dom::element::{reflect_cross_origin_attribute, set_cross_origin_attribute}; use crate::dom::element::{ - AttributeMutation, CustomElementCreationMode, Element, ElementCreator, RawLayoutElementHelpers, + AttributeMutation, CustomElementCreationMode, Element, ElementCreator, LayoutElementHelpers, }; use crate::dom::event::Event; use crate::dom::eventtarget::EventTarget; @@ -1418,7 +1418,7 @@ impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> { #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_dimension) .cloned() @@ -1429,7 +1429,7 @@ impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> { #[allow(unsafe_code)] fn get_height(self) -> LengthOrPercentageOrAuto { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("height")) .map(AttrValue::as_dimension) .cloned() diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 5d7438ecc4a..2517d8c94ca 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -18,9 +18,7 @@ use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::compositionevent::CompositionEvent; use crate::dom::document::Document; -use crate::dom::element::{ - AttributeMutation, Element, LayoutElementHelpers, RawLayoutElementHelpers, -}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::eventtarget::EventTarget; use crate::dom::file::File; @@ -735,7 +733,6 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem unsafe { input .upcast::() - .unsafe_get() .get_attr_val_for_layout(&ns!(), &local_name!("value")) .unwrap_or(default) .into() diff --git a/components/script/dom/htmltablecellelement.rs b/components/script/dom/htmltablecellelement.rs index 6aa9b466602..792c3c3aab0 100644 --- a/components/script/dom/htmltablecellelement.rs +++ b/components/script/dom/htmltablecellelement.rs @@ -9,7 +9,7 @@ use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::root::LayoutDom; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{Element, RawLayoutElementHelpers}; +use crate::dom::element::{Element, LayoutElementHelpers}; use crate::dom::htmlelement::HTMLElement; use crate::dom::htmltablerowelement::HTMLTableRowElement; use crate::dom::node::Node; @@ -108,7 +108,7 @@ pub trait HTMLTableCellElementLayoutHelpers { impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> { fn get_background_color(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) .and_then(AttrValue::as_color) .cloned() @@ -117,7 +117,7 @@ impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> { fn get_colspan(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("colspan")) .map(AttrValue::as_uint) } @@ -125,7 +125,7 @@ impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> { fn get_rowspan(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("rowspan")) .map(AttrValue::as_uint) } @@ -133,7 +133,7 @@ impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> { fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_dimension) .cloned() diff --git a/components/script/dom/htmltableelement.rs b/components/script/dom/htmltableelement.rs index 4473746367a..6f19fe0f909 100644 --- a/components/script/dom/htmltableelement.rs +++ b/components/script/dom/htmltableelement.rs @@ -11,7 +11,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{Dom, DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection}; use crate::dom::htmlelement::HTMLElement; use crate::dom::htmltablecaptionelement::HTMLTableCaptionElement; @@ -416,7 +416,7 @@ impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> { #[allow(unsafe_code)] fn get_background_color(self) -> Option { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) .and_then(AttrValue::as_color) .cloned() @@ -436,7 +436,7 @@ impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> { #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")) .map(AttrValue::as_dimension) .cloned() diff --git a/components/script/dom/htmltablerowelement.rs b/components/script/dom/htmltablerowelement.rs index 857ef5787c9..e9a0365f9cf 100644 --- a/components/script/dom/htmltablerowelement.rs +++ b/components/script/dom/htmltablerowelement.rs @@ -11,7 +11,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{Element, RawLayoutElementHelpers}; +use crate::dom::element::{Element, LayoutElementHelpers}; use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection}; use crate::dom::htmlelement::HTMLElement; use crate::dom::htmltablecellelement::HTMLTableCellElement; @@ -153,7 +153,7 @@ pub trait HTMLTableRowElementLayoutHelpers { impl HTMLTableRowElementLayoutHelpers for LayoutDom<'_, HTMLTableRowElement> { fn get_background_color(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) .and_then(AttrValue::as_color) .cloned() diff --git a/components/script/dom/htmltablesectionelement.rs b/components/script/dom/htmltablesectionelement.rs index 89e399c70c9..601e4da9c54 100644 --- a/components/script/dom/htmltablesectionelement.rs +++ b/components/script/dom/htmltablesectionelement.rs @@ -9,7 +9,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{Element, RawLayoutElementHelpers}; +use crate::dom::element::{Element, LayoutElementHelpers}; use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection}; use crate::dom::htmlelement::HTMLElement; use crate::dom::htmltablerowelement::HTMLTableRowElement; @@ -91,7 +91,7 @@ pub trait HTMLTableSectionElementLayoutHelpers { impl HTMLTableSectionElementLayoutHelpers for LayoutDom<'_, HTMLTableSectionElement> { fn get_background_color(self) -> Option { unsafe { - (&*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) .and_then(AttrValue::as_color) .cloned() diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index e3721a78213..8cade345f0f 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -14,7 +14,7 @@ use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom}; use crate::dom::bindings::str::DOMString; use crate::dom::compositionevent::CompositionEvent; use crate::dom::document::Document; -use crate::dom::element::RawLayoutElementHelpers; +use crate::dom::element::LayoutElementHelpers; use crate::dom::element::{AttributeMutation, Element}; use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::globalscope::GlobalScope; @@ -99,7 +99,7 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { #[allow(unsafe_code)] fn get_cols(self) -> u32 { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("cols")) .map_or(DEFAULT_COLS, AttrValue::as_uint) } @@ -108,7 +108,7 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { #[allow(unsafe_code)] fn get_rows(self) -> u32 { unsafe { - (*self.upcast::().unsafe_get()) + self.upcast::() .get_attr_for_layout(&ns!(), &local_name!("rows")) .map_or(DEFAULT_ROWS, AttrValue::as_uint) } diff --git a/components/script/dom/svgsvgelement.rs b/components/script/dom/svgsvgelement.rs index f4efe6e065b..a4f2401072f 100644 --- a/components/script/dom/svgsvgelement.rs +++ b/components/script/dom/svgsvgelement.rs @@ -7,7 +7,7 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, LayoutDom}; use crate::dom::bindings::str::DOMString; use crate::dom::document::Document; -use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers}; +use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers}; use crate::dom::node::Node; use crate::dom::svggraphicselement::SVGGraphicsElement; use crate::dom::virtualmethods::VirtualMethods; @@ -56,12 +56,10 @@ impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> { #[allow(unsafe_code, non_snake_case)] fn data(self) -> SVGSVGData { unsafe { - let SVG = &*self.unsafe_get(); - - let width_attr = SVG + let width_attr = self .upcast::() .get_attr_for_layout(&ns!(), &local_name!("width")); - let height_attr = SVG + let height_attr = self .upcast::() .get_attr_for_layout(&ns!(), &local_name!("height")); SVGSVGData { diff --git a/components/script/lib.rs b/components/script/lib.rs index 0df29eb6359..13791748931 100644 --- a/components/script/lib.rs +++ b/components/script/lib.rs @@ -131,7 +131,7 @@ pub mod layout_exports { pub use crate::dom::bindings::root::LayoutDom; pub use crate::dom::characterdata::LayoutCharacterDataHelpers; pub use crate::dom::document::{Document, LayoutDocumentHelpers}; - pub use crate::dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers}; + pub use crate::dom::element::{Element, LayoutElementHelpers}; pub use crate::dom::node::NodeFlags; pub use crate::dom::node::{LayoutNodeHelpers, Node}; pub use crate::dom::shadowroot::{LayoutShadowRootHelpers, ShadowRoot}; From 5ff931d1717ac0e7a6e5d9d63f0cfa47c1f3cd50 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 18:46:49 +0200 Subject: [PATCH 03/23] Introduce >::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 | 62 ++++++------- components/layout_thread_2020/dom_wrapper.rs | 54 +++++------ components/script/dom/element.rs | 91 ++++++++----------- components/script/dom/htmlbodyelement.rs | 33 +++---- components/script/dom/htmlcanvaselement.rs | 22 ++--- components/script/dom/htmlfontelement.rs | 30 +++--- components/script/dom/htmlhrelement.rs | 24 ++--- components/script/dom/htmliframeelement.rs | 26 ++---- components/script/dom/htmlimageelement.rs | 26 ++---- components/script/dom/htmlinputelement.rs | 12 +-- components/script/dom/htmltablecellelement.rs | 39 +++----- components/script/dom/htmltableelement.rs | 24 ++--- components/script/dom/htmltablerowelement.rs | 11 +-- .../script/dom/htmltablesectionelement.rs | 11 +-- components/script/dom/htmltextareaelement.rs | 18 ++-- components/script/dom/svgsvgelement.rs | 21 ++--- 16 files changed, 203 insertions(+), 301 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index adb1a915d57..4359f623acc 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -489,11 +489,9 @@ 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) - } + if let Some(ref classes) = self.element.get_classes_for_layout() { + for class in *classes { + callback(class) } } } @@ -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 => { - let values = unsafe { self.element.get_attr_vals_for_layout(local_name) }; - values.iter().any(|value| value.eval_selector(operation)) - }, + NamespaceConstraint::Any => self + .element + .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,23 +911,18 @@ 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( - HTMLElementTypeId::HTMLAreaElement, - )) | - NodeTypeId::Element(ElementTypeId::HTMLElement( - HTMLElementTypeId::HTMLLinkElement, - )) => self - .element + 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(HTMLElementTypeId::HTMLAreaElement)) | + NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) => { + self.element .get_attr_val_for_layout(&ns!(), &local_name!("href")) - .is_some(), - _ => false, - } + .is_some() + }, + _ => false, } } @@ -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 => { - let values = unsafe { self.element.element.get_attr_vals_for_layout(local_name) }; - values.iter().any(|v| v.eval_selector(operation)) - }, + NamespaceConstraint::Any => self + .element + .element + .get_attr_vals_for_layout(local_name) + .iter() + .any(|v| v.eval_selector(operation)), } } diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 3bfc5abbfea..c113a1bc49a 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -496,11 +496,9 @@ 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) - } + if let Some(ref classes) = self.element.get_classes_for_layout() { + for class in *classes { + callback(class) } } } @@ -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 => { - let values = unsafe { self.element.get_attr_vals_for_layout(local_name) }; - values.iter().any(|value| value.eval_selector(operation)) - }, + NamespaceConstraint::Any => self + .element + .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,23 +918,18 @@ 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( - HTMLElementTypeId::HTMLAreaElement, - )) | - NodeTypeId::Element(ElementTypeId::HTMLElement( - HTMLElementTypeId::HTMLLinkElement, - )) => self - .element + 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(HTMLElementTypeId::HTMLAreaElement)) | + NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) => { + self.element .get_attr_val_for_layout(&ns!(), &local_name!("href")) - .is_some(), - _ => false, - } + .is_some() + }, + _ => false, } } @@ -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)) }, } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index add49150a27..8eee9a64490 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -551,28 +551,21 @@ impl Element { } #[inline] -#[allow(unsafe_code)] -pub unsafe fn get_attr_for_layout<'dom>( - elem: &'dom Element, +pub fn get_attr_for_layout<'dom>( + elem: LayoutDom<'dom, Element>, namespace: &Namespace, name: &LocalName, ) -> Option> { - // cast to point to T in RefCell directly - let attrs = elem.attrs.borrow_for_layout(); - attrs + elem.attrs() .iter() - .find(|attr| { - let attr = attr.to_layout(); - name == attr.local_name() && namespace == attr.namespace() - }) - .map(|attr| attr.to_layout()) + .find(|attr| name == attr.local_name() && namespace == attr.namespace()) + .cloned() } pub trait LayoutElementHelpers<'dom> { - #[allow(unsafe_code)] - unsafe fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool; - #[allow(unsafe_code)] - unsafe fn get_classes_for_layout(self) -> Option<&'dom [Atom]>; + fn attrs(self) -> &'dom [LayoutDom<'dom, Attr>]; + fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool; + fn get_classes_for_layout(self) -> Option<&'dom [Atom]>; #[allow(unsafe_code)] unsafe fn synthesize_presentational_hints_for_legacy_attributes(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>; - #[allow(unsafe_code)] - unsafe fn get_attr_for_layout( + fn get_attr_for_layout( self, namespace: &Namespace, name: &LocalName, ) -> Option<&'dom AttrValue>; - #[allow(unsafe_code)] - unsafe fn get_attr_val_for_layout( - self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'dom str>; - #[allow(unsafe_code)] - unsafe fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue>; + fn get_attr_val_for_layout(self, namespace: &Namespace, name: &LocalName) -> Option<&'dom str>; + 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 { - get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class")).map_or( - false, - |attr| { - attr.as_tokens() - .unwrap() - .iter() - .any(|atom| case_sensitivity.eq_atom(atom, name)) - }, - ) + fn attrs(self) -> &'dom [LayoutDom<'dom, Attr>] { + unsafe { + // FIXME(nox): This should probably be done through a ToLayout trait. + let attrs: &[Dom] = &self.unsafe_get().attrs.borrow_for_layout(); + // This doesn't compile if Dom and LayoutDom don't have the same + // representation. + let _ = mem::transmute::, LayoutDom>; + &*(attrs as *const [Dom] as *const [LayoutDom]) + } } - #[allow(unsafe_code)] #[inline] - unsafe fn get_classes_for_layout(self) -> Option<&'dom [Atom]> { - get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class")) + 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)) + }) + } + + #[inline] + fn get_classes_for_layout(self) -> Option<&'dom [Atom]> { + 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( - self, - namespace: &Namespace, - name: &LocalName, - ) -> Option<&'dom str> { - get_attr_for_layout(self.unsafe_get(), namespace, name).map(|attr| attr.as_str()) + fn get_attr_val_for_layout(self, namespace: &Namespace, name: &LocalName) -> Option<&'dom str> { + get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str()) } - #[allow(unsafe_code)] #[inline] - unsafe fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue> { - let attrs = self.unsafe_get().attrs.borrow_for_layout(); - attrs + fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue> { + self.attrs() .iter() .filter_map(|attr| { - let attr = attr.to_layout(); if name == attr.local_name() { Some(attr.value()) } else { diff --git a/components/script/dom/htmlbodyelement.rs b/components/script/dom/htmlbodyelement.rs index 4bf835cbee9..775af6a686c 100644 --- a/components/script/dom/htmlbodyelement.rs +++ b/components/script/dom/htmlbodyelement.rs @@ -100,34 +100,25 @@ pub trait HTMLBodyElementLayoutHelpers { } impl HTMLBodyElementLayoutHelpers for LayoutDom<'_, HTMLBodyElement> { - #[allow(unsafe_code)] fn get_background_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) + .and_then(AttrValue::as_color) + .cloned() } - #[allow(unsafe_code)] fn get_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("text")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("text")) + .and_then(AttrValue::as_color) + .cloned() } - #[allow(unsafe_code)] fn get_background(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("background")) - .and_then(AttrValue::as_resolved_url) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("background")) + .and_then(AttrValue::as_resolved_url) + .cloned() } } diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index dda0f0b1536..c2958751952 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -152,24 +152,18 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { } } - #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_uint_px_dimension) - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .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::() - .get_attr_for_layout(&ns!(), &local_name!("height")) - .map(AttrValue::as_uint_px_dimension) - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("height")) + .map(AttrValue::as_uint_px_dimension) + .unwrap_or(LengthOrPercentageOrAuto::Auto) } #[allow(unsafe_code)] diff --git a/components/script/dom/htmlfontelement.rs b/components/script/dom/htmlfontelement.rs index fd40c6907f4..f9b1575577e 100644 --- a/components/script/dom/htmlfontelement.rs +++ b/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 { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("color")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("color")) + .and_then(AttrValue::as_color) + .cloned() } - #[allow(unsafe_code)] fn get_face(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("face")) - .map(AttrValue::as_atom) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("face")) + .map(AttrValue::as_atom) + .cloned() } - #[allow(unsafe_code)] fn get_size(self) -> Option { - let size = unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("size")) - }; + let size = self + .upcast::() + .get_attr_for_layout(&ns!(), &local_name!("size")); match size { Some(&AttrValue::UInt(_, s)) => Some(s), _ => None, diff --git a/components/script/dom/htmlhrelement.rs b/components/script/dom/htmlhrelement.rs index 9be680b9043..69fa8887bb1 100644 --- a/components/script/dom/htmlhrelement.rs +++ b/components/script/dom/htmlhrelement.rs @@ -71,25 +71,19 @@ pub trait HTMLHRLayoutHelpers { } impl HTMLHRLayoutHelpers for LayoutDom<'_, HTMLHRElement> { - #[allow(unsafe_code)] fn get_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("color")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .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::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("width")) + .map(AttrValue::as_dimension) + .cloned() + .unwrap_or(LengthOrPercentageOrAuto::Auto) } } diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 7b98f57a524..b36d3641764 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -499,26 +499,20 @@ impl HTMLIFrameElementLayoutMethods for LayoutDom<'_, HTMLIFrameElement> { unsafe { (*self.unsafe_get()).browsing_context_id.get() } } - #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .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::() - .get_attr_for_layout(&ns!(), &local_name!("height")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("height")) + .map(AttrValue::as_dimension) + .cloned() + .unwrap_or(LengthOrPercentageOrAuto::Auto) } } diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 626f23b69f8..cd32e6c95de 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -1415,26 +1415,20 @@ impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> { .clone() } - #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .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::() - .get_attr_for_layout(&ns!(), &local_name!("height")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("height")) + .map(AttrValue::as_dimension) + .cloned() + .unwrap_or(LengthOrPercentageOrAuto::Auto) } } diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 2517d8c94ca..ef4c6e0d7d9 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -730,13 +730,11 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem input: LayoutDom<'dom, HTMLInputElement>, default: &'static str, ) -> Cow<'dom, str> { - unsafe { - input - .upcast::() - .get_attr_val_for_layout(&ns!(), &local_name!("value")) - .unwrap_or(default) - .into() - } + input + .upcast::() + .get_attr_val_for_layout(&ns!(), &local_name!("value")) + .unwrap_or(default) + .into() } let placeholder = unsafe { &**self.unsafe_get().placeholder.borrow_for_layout() }; diff --git a/components/script/dom/htmltablecellelement.rs b/components/script/dom/htmltablecellelement.rs index 792c3c3aab0..81e9dafb1cc 100644 --- a/components/script/dom/htmltablecellelement.rs +++ b/components/script/dom/htmltablecellelement.rs @@ -104,41 +104,32 @@ pub trait HTMLTableCellElementLayoutHelpers { fn get_width(self) -> LengthOrPercentageOrAuto; } -#[allow(unsafe_code)] impl HTMLTableCellElementLayoutHelpers for LayoutDom<'_, HTMLTableCellElement> { fn get_background_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) + .and_then(AttrValue::as_color) + .cloned() } fn get_colspan(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("colspan")) - .map(AttrValue::as_uint) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("colspan")) + .map(AttrValue::as_uint) } fn get_rowspan(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("rowspan")) - .map(AttrValue::as_uint) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("rowspan")) + .map(AttrValue::as_uint) } fn get_width(self) -> LengthOrPercentageOrAuto { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("width")) + .map(AttrValue::as_dimension) + .cloned() + .unwrap_or(LengthOrPercentageOrAuto::Auto) } } diff --git a/components/script/dom/htmltableelement.rs b/components/script/dom/htmltableelement.rs index 6f19fe0f909..f42e72b5247 100644 --- a/components/script/dom/htmltableelement.rs +++ b/components/script/dom/htmltableelement.rs @@ -413,14 +413,11 @@ pub trait HTMLTableElementLayoutHelpers { } impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> { - #[allow(unsafe_code)] fn get_background_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) + .and_then(AttrValue::as_color) + .cloned() } #[allow(unsafe_code)] @@ -433,15 +430,12 @@ impl HTMLTableElementLayoutHelpers for LayoutDom<'_, HTMLTableElement> { unsafe { (*self.unsafe_get()).cellspacing.get() } } - #[allow(unsafe_code)] fn get_width(self) -> LengthOrPercentageOrAuto { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")) - .map(AttrValue::as_dimension) - .cloned() - .unwrap_or(LengthOrPercentageOrAuto::Auto) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("width")) + .map(AttrValue::as_dimension) + .cloned() + .unwrap_or(LengthOrPercentageOrAuto::Auto) } } diff --git a/components/script/dom/htmltablerowelement.rs b/components/script/dom/htmltablerowelement.rs index e9a0365f9cf..1bece55857b 100644 --- a/components/script/dom/htmltablerowelement.rs +++ b/components/script/dom/htmltablerowelement.rs @@ -149,15 +149,12 @@ pub trait HTMLTableRowElementLayoutHelpers { fn get_background_color(self) -> Option; } -#[allow(unsafe_code)] impl HTMLTableRowElementLayoutHelpers for LayoutDom<'_, HTMLTableRowElement> { fn get_background_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) + .and_then(AttrValue::as_color) + .cloned() } } diff --git a/components/script/dom/htmltablesectionelement.rs b/components/script/dom/htmltablesectionelement.rs index 601e4da9c54..fa0f3b93ab5 100644 --- a/components/script/dom/htmltablesectionelement.rs +++ b/components/script/dom/htmltablesectionelement.rs @@ -87,15 +87,12 @@ pub trait HTMLTableSectionElementLayoutHelpers { fn get_background_color(self) -> Option; } -#[allow(unsafe_code)] impl HTMLTableSectionElementLayoutHelpers for LayoutDom<'_, HTMLTableSectionElement> { fn get_background_color(self) -> Option { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) - .and_then(AttrValue::as_color) - .cloned() - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("bgcolor")) + .and_then(AttrValue::as_color) + .cloned() } } diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 8cade345f0f..248dfa33047 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -96,22 +96,16 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { )) } - #[allow(unsafe_code)] fn get_cols(self) -> u32 { - unsafe { - self.upcast::() - .get_attr_for_layout(&ns!(), &local_name!("cols")) - .map_or(DEFAULT_COLS, AttrValue::as_uint) - } + self.upcast::() + .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::() - .get_attr_for_layout(&ns!(), &local_name!("rows")) - .map_or(DEFAULT_ROWS, AttrValue::as_uint) - } + self.upcast::() + .get_attr_for_layout(&ns!(), &local_name!("rows")) + .map_or(DEFAULT_ROWS, AttrValue::as_uint) } } diff --git a/components/script/dom/svgsvgelement.rs b/components/script/dom/svgsvgelement.rs index a4f2401072f..583bdb0eb44 100644 --- a/components/script/dom/svgsvgelement.rs +++ b/components/script/dom/svgsvgelement.rs @@ -53,19 +53,16 @@ pub trait LayoutSVGSVGElementHelpers { } impl LayoutSVGSVGElementHelpers for LayoutDom<'_, SVGSVGElement> { - #[allow(unsafe_code, non_snake_case)] fn data(self) -> SVGSVGData { - unsafe { - let width_attr = self - .upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")); - let height_attr = self - .upcast::() - .get_attr_for_layout(&ns!(), &local_name!("height")); - SVGSVGData { - width: width_attr.map_or(DEFAULT_WIDTH, |val| val.as_uint()), - height: height_attr.map_or(DEFAULT_HEIGHT, |val| val.as_uint()), - } + let width_attr = self + .upcast::() + .get_attr_for_layout(&ns!(), &local_name!("width")); + let height_attr = self + .upcast::() + .get_attr_for_layout(&ns!(), &local_name!("height")); + SVGSVGData { + width: width_attr.map_or(DEFAULT_WIDTH, |val| val.as_uint()), + height: height_attr.map_or(DEFAULT_HEIGHT, |val| val.as_uint()), } } } From a913c6650da2c402863db4e6bf3185a5810d920b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 19:06:25 +0200 Subject: [PATCH 04/23] Reduce scope of unsafe block in LayoutHTMLCanvasElementHelpers::data --- components/script/dom/htmlcanvaselement.rs | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index c2958751952..5864321cb13 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -123,8 +123,8 @@ pub trait LayoutHTMLCanvasElementHelpers { impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { #[allow(unsafe_code)] fn data(self) -> HTMLCanvasData { - unsafe { - let source = match self.unsafe_get().context.borrow_for_layout().as_ref() { + let source = unsafe { + match self.unsafe_get().context.borrow_for_layout().as_ref() { Some(&CanvasContext::Context2d(ref context)) => { HTMLCanvasDataSource::Image(Some(context.to_layout().get_ipc_renderer())) }, @@ -135,20 +135,20 @@ impl LayoutHTMLCanvasElementHelpers for LayoutDom<'_, HTMLCanvasElement> { context.to_layout().canvas_data_source() }, None => HTMLCanvasDataSource::Image(None), - }; - - let width_attr = self - .upcast::() - .get_attr_for_layout(&ns!(), &local_name!("width")); - let height_attr = self - .upcast::() - .get_attr_for_layout(&ns!(), &local_name!("height")); - HTMLCanvasData { - source: source, - width: width_attr.map_or(DEFAULT_WIDTH, |val| val.as_uint()), - height: height_attr.map_or(DEFAULT_HEIGHT, |val| val.as_uint()), - canvas_id: self.get_canvas_id_for_layout(), } + }; + + let width_attr = self + .upcast::() + .get_attr_for_layout(&ns!(), &local_name!("width")); + let height_attr = self + .upcast::() + .get_attr_for_layout(&ns!(), &local_name!("height")); + HTMLCanvasData { + source: source, + width: width_attr.map_or(DEFAULT_WIDTH, |val| val.as_uint()), + height: height_attr.map_or(DEFAULT_HEIGHT, |val| val.as_uint()), + canvas_id: self.get_canvas_id_for_layout(), } } From fbc3e430ab40ab12336be90665d81eaec0f4fef3 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 20:37:57 +0200 Subject: [PATCH 05/23] Make a bunch or trivial LayoutElementHelpers safe --- components/layout_thread/dom_wrapper.rs | 10 ++++------ components/layout_thread_2020/dom_wrapper.rs | 10 ++++------ components/script/dom/element.rs | 20 +++++++------------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 4359f623acc..2528a474211 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -443,7 +443,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { } fn is_html_element(&self) -> bool { - unsafe { self.element.is_html_element() } + self.element.is_html_element() } fn is_mathml_element(&self) -> bool { @@ -954,14 +954,12 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn is_html_slot_element(&self) -> bool { - unsafe { self.element.is_html_element() && self.local_name() == &local_name!("slot") } + self.element.is_html_element() && self.local_name() == &local_name!("slot") } fn is_html_element_in_html_document(&self) -> bool { - unsafe { - if !self.element.is_html_element() { - return false; - } + if !self.element.is_html_element() { + return false; } self.as_node().owner_doc().is_html_document() diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index c113a1bc49a..dea364d1723 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -450,7 +450,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { } fn is_html_element(&self) -> bool { - unsafe { self.element.is_html_element() } + self.element.is_html_element() } fn is_mathml_element(&self) -> bool { @@ -961,14 +961,12 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn is_html_slot_element(&self) -> bool { - unsafe { self.element.is_html_element() && self.local_name() == &local_name!("slot") } + self.element.is_html_element() && self.local_name() == &local_name!("slot") } fn is_html_element_in_html_document(&self) -> bool { - unsafe { - if !self.element.is_html_element() { - return false; - } + if !self.element.is_html_element() { + return false; } self.as_node().owner_doc().is_html_document() diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 8eee9a64490..446fffea01a 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -571,12 +571,9 @@ pub trait LayoutElementHelpers<'dom> { unsafe fn synthesize_presentational_hints_for_legacy_attributes(self, _: &mut V) where V: Push; - #[allow(unsafe_code)] - unsafe fn get_colspan(self) -> u32; - #[allow(unsafe_code)] - unsafe fn get_rowspan(self) -> u32; - #[allow(unsafe_code)] - unsafe fn is_html_element(self) -> bool; + fn get_colspan(self) -> u32; + fn get_rowspan(self) -> u32; + fn is_html_element(self) -> bool; fn id_attribute(self) -> *const Option; fn style_attribute(self) -> *const Option>>; fn local_name(self) -> &'dom LocalName; @@ -951,8 +948,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { } } - #[allow(unsafe_code)] - unsafe fn get_colspan(self) -> u32 { + fn get_colspan(self) -> u32 { if let Some(this) = self.downcast::() { this.get_colspan().unwrap_or(1) } else { @@ -962,8 +958,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { } } - #[allow(unsafe_code)] - unsafe fn get_rowspan(self) -> u32 { + fn get_rowspan(self) -> u32 { if let Some(this) = self.downcast::() { this.get_rowspan().unwrap_or(1) } else { @@ -974,9 +969,8 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { } #[inline] - #[allow(unsafe_code)] - unsafe fn is_html_element(self) -> bool { - (*self.unsafe_get()).namespace == ns!(html) + fn is_html_element(self) -> bool { + *self.namespace() == ns!(html) } #[allow(unsafe_code)] From e56191106693f5cbfd1978ead55d410cc43663a0 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 20:58:20 +0200 Subject: [PATCH 06/23] Make LayoutCanvasRenderingContext2DHelpers::get_canvas_id be safe --- .../script/dom/canvasrenderingcontext2d.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs index f386cb64c2d..bb60a591060 100644 --- a/components/script/dom/canvasrenderingcontext2d.rs +++ b/components/script/dom/canvasrenderingcontext2d.rs @@ -153,8 +153,7 @@ impl CanvasRenderingContext2D { pub trait LayoutCanvasRenderingContext2DHelpers { #[allow(unsafe_code)] unsafe fn get_ipc_renderer(self) -> IpcSender; - #[allow(unsafe_code)] - unsafe fn get_canvas_id(self) -> CanvasId; + fn get_canvas_id(self) -> CanvasId; } impl LayoutCanvasRenderingContext2DHelpers for LayoutDom<'_, CanvasRenderingContext2D> { @@ -168,11 +167,16 @@ impl LayoutCanvasRenderingContext2DHelpers for LayoutDom<'_, CanvasRenderingCont } #[allow(unsafe_code)] - unsafe fn get_canvas_id(self) -> CanvasId { - (*self.unsafe_get()) - .canvas_state - .borrow_for_layout() - .get_canvas_id() + fn get_canvas_id(self) -> CanvasId { + // FIXME(nox): This relies on the fact that CanvasState::get_canvas_id + // does nothing fancy but it would be easier to trust a + // LayoutDom<_>-like type that would wrap the &CanvasState. + unsafe { + self.unsafe_get() + .canvas_state + .borrow_for_layout() + .get_canvas_id() + } } } From 72c0771299151ce93f8178c84b1eb1d13c29a49c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 21:24:07 +0200 Subject: [PATCH 07/23] Make a bunch of LayoutDocumentHelpers be safe The other methods are actually unsafe. --- components/layout_thread/dom_wrapper.rs | 4 +- components/layout_thread_2020/dom_wrapper.rs | 4 +- components/script/dom/document.rs | 40 +++++++++++--------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 2528a474211..7d3ad1e5c5a 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -345,11 +345,11 @@ impl<'ld> TDocument for ServoLayoutDocument<'ld> { } fn quirks_mode(&self) -> QuirksMode { - unsafe { self.document.quirks_mode() } + self.document.quirks_mode() } fn is_html_document(&self) -> bool { - unsafe { self.document.is_html_document_for_layout() } + self.document.is_html_document_for_layout() } } diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index dea364d1723..4516a6c20d5 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -352,11 +352,11 @@ impl<'ld> TDocument for ServoLayoutDocument<'ld> { } fn quirks_mode(&self) -> QuirksMode { - unsafe { self.document.quirks_mode() } + self.document.quirks_mode() } fn is_html_document(&self) -> bool { - unsafe { self.document.is_html_document_for_layout() } + self.document.is_html_document_for_layout() } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 1e60b3b79fc..c3c64babfef 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2606,21 +2606,21 @@ pub enum DocumentSource { #[allow(unsafe_code)] pub trait LayoutDocumentHelpers<'dom> { - unsafe fn is_html_document_for_layout(self) -> bool; + fn is_html_document_for_layout(self) -> bool; unsafe fn needs_paint_from_layout(self); unsafe fn will_paint(self); - unsafe fn quirks_mode(self) -> QuirksMode; + fn quirks_mode(self) -> QuirksMode; unsafe fn style_shared_lock(self) -> &'dom StyleSharedRwLock; - unsafe fn shadow_roots(self) -> Vec>; - unsafe fn shadow_roots_styles_changed(self) -> bool; + fn shadow_roots(self) -> Vec>; + fn shadow_roots_styles_changed(self) -> bool; unsafe fn flush_shadow_roots_stylesheets(self); } #[allow(unsafe_code)] impl<'dom> LayoutDocumentHelpers<'dom> for LayoutDom<'dom, Document> { #[inline] - unsafe fn is_html_document_for_layout(self) -> bool { - (*self.unsafe_get()).is_html_document + fn is_html_document_for_layout(self) -> bool { + unsafe { self.unsafe_get().is_html_document } } #[inline] @@ -2634,8 +2634,8 @@ impl<'dom> LayoutDocumentHelpers<'dom> for LayoutDom<'dom, Document> { } #[inline] - unsafe fn quirks_mode(self) -> QuirksMode { - (*self.unsafe_get()).quirks_mode() + fn quirks_mode(self) -> QuirksMode { + unsafe { self.unsafe_get().quirks_mode.get() } } #[inline] @@ -2644,18 +2644,24 @@ impl<'dom> LayoutDocumentHelpers<'dom> for LayoutDom<'dom, Document> { } #[inline] - unsafe fn shadow_roots(self) -> Vec> { - (*self.unsafe_get()) - .shadow_roots - .borrow_for_layout() - .iter() - .map(|sr| sr.to_layout()) - .collect() + fn shadow_roots(self) -> Vec> { + // FIXME(nox): We should just return a + // &'dom HashSet> here but not until + // I rework the ToLayout trait as mentioned in + // LayoutDom::to_layout_slice. + unsafe { + self.unsafe_get() + .shadow_roots + .borrow_for_layout() + .iter() + .map(|sr| sr.to_layout()) + .collect() + } } #[inline] - unsafe fn shadow_roots_styles_changed(self) -> bool { - (*self.unsafe_get()).shadow_roots_styles_changed() + fn shadow_roots_styles_changed(self) -> bool { + unsafe { self.unsafe_get().shadow_roots_styles_changed.get() } } #[inline] From dd750c6f8630dc5cdf27896fe420da9d441459c1 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 21:32:08 +0200 Subject: [PATCH 08/23] Introduce LayoutDom::to_layout_slice It generalises >::attrs. --- components/script/dom/bindings/root.rs | 9 +++++++++ components/script/dom/element.rs | 9 +-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index 9afb421abbb..9f571f1379c 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -736,6 +736,15 @@ where debug_assert!(thread_state::get().is_layout()); self.value } + + /// Transforms a slice of Dom into a slice of LayoutDom. + // FIXME(nox): This should probably be done through a ToLayout trait. + pub unsafe fn to_layout_slice(slice: &'dom [Dom]) -> &'dom [LayoutDom<'dom, T>] { + // This doesn't compile if Dom and LayoutDom don't have the same + // representation. + let _ = mem::transmute::, LayoutDom>; + &*(slice as *const [Dom] as *const [LayoutDom]) + } } /// Helper trait for safer manipulations of `Option>` values. diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 446fffea01a..48ef41cbff3 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -598,14 +598,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[allow(unsafe_code)] #[inline] fn attrs(self) -> &'dom [LayoutDom<'dom, Attr>] { - unsafe { - // FIXME(nox): This should probably be done through a ToLayout trait. - let attrs: &[Dom] = &self.unsafe_get().attrs.borrow_for_layout(); - // This doesn't compile if Dom and LayoutDom don't have the same - // representation. - let _ = mem::transmute::, LayoutDom>; - &*(attrs as *const [Dom] as *const [LayoutDom]) - } + unsafe { LayoutDom::to_layout_slice(self.unsafe_get().attrs.borrow_for_layout()) } } #[inline] From 414d477b5482e44b47ab8034b158b2ad2e92bf27 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 21:54:02 +0200 Subject: [PATCH 09/23] Don't generate rare_data_for_layout methods anymore It is only used twice. --- components/script/dom/element.rs | 5 +++-- components/script/dom/macros.rs | 5 ----- components/script/dom/node.rs | 5 +++-- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 48ef41cbff3..cca6ae7edbd 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -1039,8 +1039,9 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[inline] #[allow(unsafe_code)] unsafe fn get_shadow_root_for_layout(self) -> Option> { - (*self.unsafe_get()) - .rare_data_for_layout() + self.unsafe_get() + .rare_data + .borrow_for_layout() .as_ref()? .shadow_root .as_ref() diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs index 1a2a17d652c..3969cb8bdd0 100644 --- a/components/script/dom/macros.rs +++ b/components/script/dom/macros.rs @@ -671,11 +671,6 @@ macro_rules! impl_rare_data ( rare_data.as_mut().unwrap() }) } - - #[allow(unsafe_code)] - fn rare_data_for_layout(&self) -> &Option> { - unsafe { self.rare_data.borrow_for_layout() } - } ); ); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 251f6fcab18..61e164dbd12 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1400,8 +1400,9 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] unsafe fn containing_shadow_root_for_layout(self) -> Option> { - (*self.unsafe_get()) - .rare_data_for_layout() + self.unsafe_get() + .rare_data + .borrow_for_layout() .as_ref()? .containing_shadow_root .as_ref() From 9c8540af5c37c7df2d4c0dff27ca76e36a6e6c02 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 21:59:01 +0200 Subject: [PATCH 10/23] Make layout methods accessing rare data be safe They don't do anything fancy so there is no additional unsafety calling them compared to using LayoutDom in the first place, the usual story of all those changes. --- components/layout_thread/dom_wrapper.rs | 18 +++++++---------- components/layout_thread_2020/dom_wrapper.rs | 18 +++++++---------- components/script/dom/element.rs | 21 ++++++++++---------- components/script/dom/node.rs | 20 ++++++++++--------- 4 files changed, 36 insertions(+), 41 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 7d3ad1e5c5a..3ad5927c89b 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -649,21 +649,17 @@ impl<'le> TElement for ServoLayoutElement<'le> { /// The shadow root this element is a host of. fn shadow_root(&self) -> Option> { - unsafe { - self.element - .get_shadow_root_for_layout() - .map(ServoShadowRoot::from_layout_js) - } + self.element + .get_shadow_root_for_layout() + .map(ServoShadowRoot::from_layout_js) } /// The shadow root which roots the subtree this element is contained in. fn containing_shadow(&self) -> Option> { - unsafe { - self.element - .upcast() - .containing_shadow_root_for_layout() - .map(ServoShadowRoot::from_layout_js) - } + self.element + .upcast() + .containing_shadow_root_for_layout() + .map(ServoShadowRoot::from_layout_js) } fn local_name(&self) -> &LocalName { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 4516a6c20d5..673f6d29db9 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -656,21 +656,17 @@ impl<'le> TElement for ServoLayoutElement<'le> { /// The shadow root this element is a host of. fn shadow_root(&self) -> Option> { - unsafe { - self.element - .get_shadow_root_for_layout() - .map(ServoShadowRoot::from_layout_js) - } + self.element + .get_shadow_root_for_layout() + .map(ServoShadowRoot::from_layout_js) } /// The shadow root which roots the subtree this element is contained in. fn containing_shadow(&self) -> Option> { - unsafe { - self.element - .upcast() - .containing_shadow_root_for_layout() - .map(ServoShadowRoot::from_layout_js) - } + self.element + .upcast() + .containing_shadow_root_for_layout() + .map(ServoShadowRoot::from_layout_js) } fn local_name(&self) -> &LocalName { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index cca6ae7edbd..8bbdbf6e7dd 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -583,8 +583,7 @@ pub trait LayoutElementHelpers<'dom> { fn insert_selector_flags(self, flags: ElementSelectorFlags); fn has_selector_flags(self, flags: ElementSelectorFlags) -> bool; /// The shadow root this element is a host of. - #[allow(unsafe_code)] - unsafe fn get_shadow_root_for_layout(self) -> Option>; + fn get_shadow_root_for_layout(self) -> Option>; fn get_attr_for_layout( self, namespace: &Namespace, @@ -1038,14 +1037,16 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[inline] #[allow(unsafe_code)] - unsafe fn get_shadow_root_for_layout(self) -> Option> { - self.unsafe_get() - .rare_data - .borrow_for_layout() - .as_ref()? - .shadow_root - .as_ref() - .map(|sr| sr.to_layout()) + fn get_shadow_root_for_layout(self) -> Option> { + unsafe { + self.unsafe_get() + .rare_data + .borrow_for_layout() + .as_ref()? + .shadow_root + .as_ref() + .map(|sr| sr.to_layout()) + } } #[inline] diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 61e164dbd12..5f8d518b514 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1314,7 +1314,7 @@ pub trait LayoutNodeHelpers<'dom> { unsafe fn next_sibling_ref(self) -> Option>; unsafe fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document>; - unsafe fn containing_shadow_root_for_layout(self) -> Option>; + fn containing_shadow_root_for_layout(self) -> Option>; unsafe fn is_element_for_layout(self) -> bool; unsafe fn get_flag(self, flag: NodeFlags) -> bool; @@ -1399,14 +1399,16 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] - unsafe fn containing_shadow_root_for_layout(self) -> Option> { - self.unsafe_get() - .rare_data - .borrow_for_layout() - .as_ref()? - .containing_shadow_root - .as_ref() - .map(|sr| sr.to_layout()) + fn containing_shadow_root_for_layout(self) -> Option> { + unsafe { + self.unsafe_get() + .rare_data + .borrow_for_layout() + .as_ref()? + .containing_shadow_root + .as_ref() + .map(|sr| sr.to_layout()) + } } #[inline] From f014da9565a491455c8cf0570a4bde9ec6328c35 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 22:06:26 +0200 Subject: [PATCH 11/23] Introduce LayoutDom::is Just like Castable::is. --- components/layout_thread/dom_wrapper.rs | 2 +- components/layout_thread_2020/dom_wrapper.rs | 2 +- components/script/dom/bindings/root.rs | 9 +++++++++ components/script/dom/node.rs | 7 +++---- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 3ad5927c89b..4e6f187d5d5 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -141,7 +141,7 @@ impl<'ln> ServoLayoutNode<'ln> { impl<'ln> NodeInfo for ServoLayoutNode<'ln> { fn is_element(&self) -> bool { - unsafe { self.node.is_element_for_layout() } + self.node.is_element_for_layout() } fn is_text_node(&self) -> bool { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 673f6d29db9..15616315542 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -148,7 +148,7 @@ impl<'ln> ServoLayoutNode<'ln> { impl<'ln> NodeInfo for ServoLayoutNode<'ln> { fn is_element(&self) -> bool { - unsafe { self.node.is_element_for_layout() } + self.node.is_element_for_layout() } fn is_text_node(&self) -> bool { diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index 9f571f1379c..4f4d376e951 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -441,6 +441,15 @@ where debug_assert!(thread_state::get().is_layout()); self.value.downcast::().map(|value| LayoutDom { value }) } + + /// Returns whether this inner object is a U. + pub fn is(&self) -> bool + where + U: DerivedFrom, + { + debug_assert!(thread_state::get().is_layout()); + self.value.is::() + } } impl LayoutDom<'_, T> diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 5f8d518b514..eaceeb9ea2c 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1316,7 +1316,7 @@ pub trait LayoutNodeHelpers<'dom> { unsafe fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document>; fn containing_shadow_root_for_layout(self) -> Option>; - unsafe fn is_element_for_layout(self) -> bool; + fn is_element_for_layout(self) -> bool; unsafe fn get_flag(self, flag: NodeFlags) -> bool; unsafe fn set_flag(self, flag: NodeFlags, value: bool); @@ -1347,9 +1347,8 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { } #[inline] - #[allow(unsafe_code)] - unsafe fn is_element_for_layout(self) -> bool { - (*self.unsafe_get()).is::() + fn is_element_for_layout(self) -> bool { + self.is::() } #[inline] From 68d5cfffd500877333f88b98682520b5f680fcd1 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 22:13:06 +0200 Subject: [PATCH 12/23] Make a bunch of LayoutNodeHelpers be safe --- components/layout_thread/dom_wrapper.rs | 12 +++--- components/layout_thread_2020/dom_wrapper.rs | 12 +++--- components/script/dom/node.rs | 45 +++++++++----------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 4e6f187d5d5..d6b33c40c99 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -135,7 +135,7 @@ impl<'ln> ServoLayoutNode<'ln> { } fn script_type_id(&self) -> NodeTypeId { - unsafe { self.node.type_id_for_layout() } + self.node.type_id_for_layout() } } @@ -211,23 +211,23 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } fn first_child(&self) -> Option { - unsafe { self.node.first_child_ref().map(Self::from_layout_js) } + self.node.first_child_ref().map(Self::from_layout_js) } fn last_child(&self) -> Option { - unsafe { self.node.last_child_ref().map(Self::from_layout_js) } + self.node.last_child_ref().map(Self::from_layout_js) } fn prev_sibling(&self) -> Option { - unsafe { self.node.prev_sibling_ref().map(Self::from_layout_js) } + self.node.prev_sibling_ref().map(Self::from_layout_js) } fn next_sibling(&self) -> Option { - unsafe { self.node.next_sibling_ref().map(Self::from_layout_js) } + self.node.next_sibling_ref().map(Self::from_layout_js) } fn owner_doc(&self) -> Self::ConcreteDocument { - ServoLayoutDocument::from_layout_js(unsafe { self.node.owner_doc_for_layout() }) + ServoLayoutDocument::from_layout_js(self.node.owner_doc_for_layout()) } fn traversal_parent(&self) -> Option> { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 15616315542..436a88b1d9e 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -142,7 +142,7 @@ impl<'ln> ServoLayoutNode<'ln> { } fn script_type_id(&self) -> NodeTypeId { - unsafe { self.node.type_id_for_layout() } + self.node.type_id_for_layout() } } @@ -218,23 +218,23 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } fn first_child(&self) -> Option { - unsafe { self.node.first_child_ref().map(Self::from_layout_js) } + self.node.first_child_ref().map(Self::from_layout_js) } fn last_child(&self) -> Option { - unsafe { self.node.last_child_ref().map(Self::from_layout_js) } + self.node.last_child_ref().map(Self::from_layout_js) } fn prev_sibling(&self) -> Option { - unsafe { self.node.prev_sibling_ref().map(Self::from_layout_js) } + self.node.prev_sibling_ref().map(Self::from_layout_js) } fn next_sibling(&self) -> Option { - unsafe { self.node.next_sibling_ref().map(Self::from_layout_js) } + self.node.next_sibling_ref().map(Self::from_layout_js) } fn owner_doc(&self) -> Self::ConcreteDocument { - ServoLayoutDocument::from_layout_js(unsafe { self.node.owner_doc_for_layout() }) + ServoLayoutDocument::from_layout_js(self.node.owner_doc_for_layout()) } fn traversal_parent(&self) -> Option> { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index eaceeb9ea2c..a42f2371d3d 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1305,22 +1305,22 @@ pub unsafe fn from_untrusted_node_address( #[allow(unsafe_code)] pub trait LayoutNodeHelpers<'dom> { - unsafe fn type_id_for_layout(self) -> NodeTypeId; + fn type_id_for_layout(self) -> NodeTypeId; unsafe fn composed_parent_node_ref(self) -> Option>; - unsafe fn first_child_ref(self) -> Option>; - unsafe fn last_child_ref(self) -> Option>; - unsafe fn prev_sibling_ref(self) -> Option>; - unsafe fn next_sibling_ref(self) -> Option>; + fn first_child_ref(self) -> Option>; + fn last_child_ref(self) -> Option>; + fn prev_sibling_ref(self) -> Option>; + fn next_sibling_ref(self) -> Option>; - unsafe fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document>; + fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document>; fn containing_shadow_root_for_layout(self) -> Option>; fn is_element_for_layout(self) -> bool; unsafe fn get_flag(self, flag: NodeFlags) -> bool; unsafe fn set_flag(self, flag: NodeFlags, value: bool); - unsafe fn children_count(self) -> u32; + fn children_count(self) -> u32; unsafe fn get_style_and_layout_data(self) -> Option; unsafe fn init_style_and_layout_data(self, _: OpaqueStyleAndLayoutData); @@ -1342,8 +1342,8 @@ pub trait LayoutNodeHelpers<'dom> { impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] - unsafe fn type_id_for_layout(self) -> NodeTypeId { - (*self.unsafe_get()).type_id() + fn type_id_for_layout(self) -> NodeTypeId { + unsafe { self.unsafe_get().type_id() } } #[inline] @@ -1365,35 +1365,32 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] - unsafe fn first_child_ref(self) -> Option> { - (*self.unsafe_get()).first_child.get_inner_as_layout() + fn first_child_ref(self) -> Option> { + unsafe { self.unsafe_get().first_child.get_inner_as_layout() } } #[inline] #[allow(unsafe_code)] - unsafe fn last_child_ref(self) -> Option> { - (*self.unsafe_get()).last_child.get_inner_as_layout() + fn last_child_ref(self) -> Option> { + unsafe { self.unsafe_get().last_child.get_inner_as_layout() } } #[inline] #[allow(unsafe_code)] - unsafe fn prev_sibling_ref(self) -> Option> { - (*self.unsafe_get()).prev_sibling.get_inner_as_layout() + fn prev_sibling_ref(self) -> Option> { + unsafe { self.unsafe_get().prev_sibling.get_inner_as_layout() } } #[inline] #[allow(unsafe_code)] - unsafe fn next_sibling_ref(self) -> Option> { - (*self.unsafe_get()).next_sibling.get_inner_as_layout() + fn next_sibling_ref(self) -> Option> { + unsafe { self.unsafe_get().next_sibling.get_inner_as_layout() } } #[inline] #[allow(unsafe_code)] - unsafe fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document> { - (*self.unsafe_get()) - .owner_doc - .get_inner_as_layout() - .unwrap() + fn owner_doc_for_layout(self) -> LayoutDom<'dom, Document> { + unsafe { self.unsafe_get().owner_doc.get_inner_as_layout().unwrap() } } #[inline] @@ -1433,8 +1430,8 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] - unsafe fn children_count(self) -> u32 { - (*self.unsafe_get()).children_count.get() + fn children_count(self) -> u32 { + unsafe { self.unsafe_get().children_count.get() } } #[inline] From f712b0bcf8ee2694170d3e92f03fb87539f81324 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 22:30:42 +0200 Subject: [PATCH 13/23] Make LayoutShadowRootHelpers::get_host_for_layout be safe --- components/layout_thread/dom_wrapper.rs | 2 +- components/layout_thread_2020/dom_wrapper.rs | 2 +- components/script/dom/shadowroot.rs | 14 ++++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index d6b33c40c99..da605b9958b 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -170,7 +170,7 @@ impl<'lr> TShadowRoot for ServoShadowRoot<'lr> { } fn host(&self) -> ServoLayoutElement<'lr> { - ServoLayoutElement::from_layout_js(unsafe { self.shadow_root.get_host_for_layout() }) + ServoLayoutElement::from_layout_js(self.shadow_root.get_host_for_layout()) } fn style_data<'a>(&self) -> Option<&'a CascadeData> diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 436a88b1d9e..c96585a4456 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -177,7 +177,7 @@ impl<'lr> TShadowRoot for ServoShadowRoot<'lr> { } fn host(&self) -> ServoLayoutElement<'lr> { - ServoLayoutElement::from_layout_js(unsafe { self.shadow_root.get_host_for_layout() }) + ServoLayoutElement::from_layout_js(self.shadow_root.get_host_for_layout()) } fn style_data<'a>(&self) -> Option<&'a CascadeData> diff --git a/components/script/dom/shadowroot.rs b/components/script/dom/shadowroot.rs index 6dd2ee122e6..df9337eabcc 100644 --- a/components/script/dom/shadowroot.rs +++ b/components/script/dom/shadowroot.rs @@ -240,7 +240,7 @@ impl ShadowRootMethods for ShadowRoot { #[allow(unsafe_code)] pub trait LayoutShadowRootHelpers<'dom> { - unsafe fn get_host_for_layout(self) -> LayoutDom<'dom, Element>; + fn get_host_for_layout(self) -> LayoutDom<'dom, Element>; unsafe fn get_style_data_for_layout(self) -> &'dom AuthorStyles; unsafe fn flush_stylesheets( self, @@ -253,11 +253,13 @@ pub trait LayoutShadowRootHelpers<'dom> { impl<'dom> LayoutShadowRootHelpers<'dom> for LayoutDom<'dom, ShadowRoot> { #[inline] #[allow(unsafe_code)] - unsafe fn get_host_for_layout(self) -> LayoutDom<'dom, Element> { - (*self.unsafe_get()) - .host - .get_inner_as_layout() - .expect("We should never do layout on a detached shadow root") + fn get_host_for_layout(self) -> LayoutDom<'dom, Element> { + unsafe { + self.unsafe_get() + .host + .get_inner_as_layout() + .expect("We should never do layout on a detached shadow root") + } } #[inline] From fc07a5147cd26fa3d8778b3366358f8ae5bcee36 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 22:32:35 +0200 Subject: [PATCH 14/23] Make LayoutNodeHelpers::composed_parent_node_ref be safe For clarity, I introduce >::parent_node_ref to contain the remaining unsafety bits out of composed_parent_node_ref which is more complex than just a field access. --- components/layout_thread/dom_wrapper.rs | 18 ++++----- components/layout_thread_2020/dom_wrapper.rs | 18 ++++----- components/script/dom/element.rs | 41 +++++++++----------- components/script/dom/node.rs | 17 +++++--- 4 files changed, 44 insertions(+), 50 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index da605b9958b..5c9cf183053 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -203,11 +203,9 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { type ConcreteShadowRoot = ServoShadowRoot<'ln>; fn parent_node(&self) -> Option { - unsafe { - self.node - .composed_parent_node_ref() - .map(Self::from_layout_js) - } + self.node + .composed_parent_node_ref() + .map(Self::from_layout_js) } fn first_child(&self) -> Option { @@ -745,12 +743,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn parent_element(&self) -> Option> { - unsafe { - self.element - .upcast() - .composed_parent_node_ref() - .and_then(as_element) - } + self.element + .upcast() + .composed_parent_node_ref() + .and_then(as_element) } fn parent_node_is_shadow_root(&self) -> bool { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index c96585a4456..0e527bd75d9 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -210,11 +210,9 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { type ConcreteShadowRoot = ServoShadowRoot<'ln>; fn parent_node(&self) -> Option { - unsafe { - self.node - .composed_parent_node_ref() - .map(Self::from_layout_js) - } + self.node + .composed_parent_node_ref() + .map(Self::from_layout_js) } fn first_child(&self) -> Option { @@ -752,12 +750,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn parent_element(&self) -> Option> { - unsafe { - self.element - .upcast() - .composed_parent_node_ref() - .and_then(as_element) - } + self.element + .upcast() + .composed_parent_node_ref() + .and_then(as_element) } fn parent_node_is_shadow_root(&self) -> bool { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 8bbdbf6e7dd..b59b342024f 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -985,32 +985,27 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { unsafe { &(*self.unsafe_get()).namespace } } - #[allow(unsafe_code)] fn get_lang_for_layout(self) -> String { - unsafe { - let mut current_node = Some(self.upcast::()); - while let Some(node) = current_node { - current_node = node.composed_parent_node_ref(); - match node.downcast::() { - Some(elem) => { - if let Some(attr) = - elem.get_attr_val_for_layout(&ns!(xml), &local_name!("lang")) - { - return attr.to_owned(); - } - if let Some(attr) = - elem.get_attr_val_for_layout(&ns!(), &local_name!("lang")) - { - return attr.to_owned(); - } - }, - None => continue, - } + let mut current_node = Some(self.upcast::()); + while let Some(node) = current_node { + current_node = node.composed_parent_node_ref(); + match node.downcast::() { + Some(elem) => { + if let Some(attr) = + elem.get_attr_val_for_layout(&ns!(xml), &local_name!("lang")) + { + return attr.to_owned(); + } + if let Some(attr) = elem.get_attr_val_for_layout(&ns!(), &local_name!("lang")) { + return attr.to_owned(); + } + }, + None => continue, } - // TODO: Check meta tags for a pragma-set default language - // TODO: Check HTTP Content-Language header - String::new() } + // TODO: Check meta tags for a pragma-set default language + // TODO: Check HTTP Content-Language header + String::new() } #[inline] diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index a42f2371d3d..7348431d4f3 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1307,7 +1307,7 @@ pub unsafe fn from_untrusted_node_address( pub trait LayoutNodeHelpers<'dom> { fn type_id_for_layout(self) -> NodeTypeId; - unsafe fn composed_parent_node_ref(self) -> Option>; + fn composed_parent_node_ref(self) -> Option>; fn first_child_ref(self) -> Option>; fn last_child_ref(self) -> Option>; fn prev_sibling_ref(self) -> Option>; @@ -1339,6 +1339,14 @@ pub trait LayoutNodeHelpers<'dom> { fn opaque(self) -> OpaqueNode; } +impl<'dom> LayoutDom<'dom, Node> { + #[inline] + #[allow(unsafe_code)] + fn parent_node_ref(self) -> Option> { + unsafe { self.unsafe_get().parent_node.get_inner_as_layout() } + } +} + impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] @@ -1352,10 +1360,9 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { } #[inline] - #[allow(unsafe_code)] - unsafe fn composed_parent_node_ref(self) -> Option> { - let parent = (*self.unsafe_get()).parent_node.get_inner_as_layout(); - if let Some(ref parent) = parent { + fn composed_parent_node_ref(self) -> Option> { + let parent = self.parent_node_ref(); + if let Some(parent) = parent { if let Some(shadow_root) = parent.downcast::() { return Some(shadow_root.get_host_for_layout().upcast()); } From 1cd3d6bd4cd0071c8c35d1f891b9f1edaaf20ac7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 22:44:47 +0200 Subject: [PATCH 15/23] Introduce >::current_request This safe helper contains the only source of unsafety from the actual image layout helpers methods, making them completely safe. --- components/script/dom/htmlimageelement.rs | 51 +++++++++-------------- components/script/dom/node.rs | 18 ++++---- 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index cd32e6c95de..b5f20784c8e 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -1366,53 +1366,40 @@ impl MicrotaskRunnable for ImageElementMicrotask { } pub trait LayoutHTMLImageElementHelpers { - #[allow(unsafe_code)] - unsafe fn image(self) -> Option>; - #[allow(unsafe_code)] - unsafe fn image_url(self) -> Option; - #[allow(unsafe_code)] - unsafe fn image_density(self) -> Option; - #[allow(unsafe_code)] - unsafe fn image_data(self) -> (Option>, Option); + fn image(self) -> Option>; + fn image_url(self) -> Option; + fn image_density(self) -> Option; + fn image_data(self) -> (Option>, Option); fn get_width(self) -> LengthOrPercentageOrAuto; fn get_height(self) -> LengthOrPercentageOrAuto; } +impl<'dom> LayoutDom<'dom, HTMLImageElement> { + #[allow(unsafe_code)] + fn current_request(self) -> &'dom ImageRequest { + unsafe { self.unsafe_get().current_request.borrow_for_layout() } + } +} + impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> { - #[allow(unsafe_code)] - unsafe fn image(self) -> Option> { - (*self.unsafe_get()) - .current_request - .borrow_for_layout() - .image - .clone() + fn image(self) -> Option> { + self.current_request().image.clone() } - #[allow(unsafe_code)] - unsafe fn image_url(self) -> Option { - (*self.unsafe_get()) - .current_request - .borrow_for_layout() - .parsed_url - .clone() + fn image_url(self) -> Option { + self.current_request().parsed_url.clone() } - #[allow(unsafe_code)] - unsafe fn image_data(self) -> (Option>, Option) { - let current_request = (*self.unsafe_get()).current_request.borrow_for_layout(); + fn image_data(self) -> (Option>, Option) { + let current_request = self.current_request(); ( current_request.image.clone(), current_request.metadata.clone(), ) } - #[allow(unsafe_code)] - unsafe fn image_density(self) -> Option { - (*self.unsafe_get()) - .current_request - .borrow_for_layout() - .current_pixel_density - .clone() + fn image_density(self) -> Option { + self.current_request().current_pixel_density.clone() } fn get_width(self) -> LengthOrPercentageOrAuto { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 7348431d4f3..b7215cfd92b 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1493,25 +1493,21 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[allow(unsafe_code)] fn image_url(self) -> Option { - unsafe { - self.downcast::() - .expect("not an image!") - .image_url() - } + self.downcast::() + .expect("not an image!") + .image_url() } #[allow(unsafe_code)] fn image_data(self) -> Option<(Option>, Option)> { - unsafe { self.downcast::().map(|e| e.image_data()) } + self.downcast::().map(|e| e.image_data()) } #[allow(unsafe_code)] fn image_density(self) -> Option { - unsafe { - self.downcast::() - .expect("not an image!") - .image_density() - } + self.downcast::() + .expect("not an image!") + .image_density() } fn canvas_data(self) -> Option { From f8af8176dedd87de86c61ff05b4026ad32ebe86c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 23:06:20 +0200 Subject: [PATCH 16/23] Introduce a bunch of LayoutDom private helpers Those help contain the unsafety in most of the actual helpers used by layout. --- components/script/dom/htmlinputelement.rs | 61 ++++++++++++----------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index ef4c6e0d7d9..f84296b3a66 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -705,26 +705,34 @@ impl HTMLInputElement { pub trait LayoutHTMLInputElementHelpers<'dom> { fn value_for_layout(self) -> Cow<'dom, str>; - #[allow(unsafe_code)] - unsafe fn size_for_layout(self) -> u32; + fn size_for_layout(self) -> u32; #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option>; - #[allow(unsafe_code)] - unsafe fn checked_state_for_layout(self) -> bool; - #[allow(unsafe_code)] - unsafe fn indeterminate_state_for_layout(self) -> bool; + fn checked_state_for_layout(self) -> bool; + fn indeterminate_state_for_layout(self) -> bool; } #[allow(unsafe_code)] -unsafe fn get_raw_textinput_value(input: LayoutDom) -> DOMString { - (*input.unsafe_get()) - .textinput - .borrow_for_layout() - .get_content() +impl<'dom> LayoutDom<'dom, HTMLInputElement> { + fn get_raw_textinput_value(self) -> DOMString { + unsafe { + self.unsafe_get() + .textinput + .borrow_for_layout() + .get_content() + } + } + + fn placeholder(self) -> &'dom str { + unsafe { self.unsafe_get().placeholder.borrow_for_layout() } + } + + fn input_type(self) -> InputType { + unsafe { self.unsafe_get().input_type.get() } + } } impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElement> { - #[allow(unsafe_code)] fn value_for_layout(self) -> Cow<'dom, str> { fn get_raw_attr_value<'dom>( input: LayoutDom<'dom, HTMLInputElement>, @@ -737,42 +745,39 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem .into() } - let placeholder = unsafe { &**self.unsafe_get().placeholder.borrow_for_layout() }; - match unsafe { self.unsafe_get().input_type() } { + match self.input_type() { InputType::Checkbox | InputType::Radio => "".into(), InputType::File | InputType::Image => "".into(), InputType::Button => get_raw_attr_value(self, ""), InputType::Submit => get_raw_attr_value(self, DEFAULT_SUBMIT_VALUE), InputType::Reset => get_raw_attr_value(self, DEFAULT_RESET_VALUE), InputType::Password => { - let text = unsafe { get_raw_textinput_value(self) }; + let text = self.get_raw_textinput_value(); if !text.is_empty() { text.chars() .map(|_| PASSWORD_REPLACEMENT_CHAR) .collect::() .into() } else { - placeholder.into() + self.placeholder().into() } }, _ => { - let text = unsafe { get_raw_textinput_value(self) }; + let text = self.get_raw_textinput_value(); if !text.is_empty() { text.into() } else { - placeholder.into() + self.placeholder().into() } }, } } - #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn size_for_layout(self) -> u32 { - (*self.unsafe_get()).size.get() + fn size_for_layout(self) -> u32 { + unsafe { self.unsafe_get().size.get() } } - #[allow(unrooted_must_root)] #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option> { if !(*self.unsafe_get()).upcast::().focus_state() { @@ -781,9 +786,9 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem let textinput = (*self.unsafe_get()).textinput.borrow_for_layout(); - match (*self.unsafe_get()).input_type() { + match self.input_type() { InputType::Password => { - let text = get_raw_textinput_value(self); + let text = self.get_raw_textinput_value(); let sel = UTF8Bytes::unwrap_range(textinput.sorted_selection_offsets_range()); // Translate indices from the raw value to indices in the replacement value. @@ -800,17 +805,13 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem } } - #[allow(unrooted_must_root)] - #[allow(unsafe_code)] - unsafe fn checked_state_for_layout(self) -> bool { + fn checked_state_for_layout(self) -> bool { self.upcast::() .get_state_for_layout() .contains(ElementState::IN_CHECKED_STATE) } - #[allow(unrooted_must_root)] - #[allow(unsafe_code)] - unsafe fn indeterminate_state_for_layout(self) -> bool { + fn indeterminate_state_for_layout(self) -> bool { self.upcast::() .get_state_for_layout() .contains(ElementState::IN_INDETERMINATE_STATE) From 4636507fa1f218d4c0d858089946b3e6f9b9aabc Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 00:02:37 +0200 Subject: [PATCH 17/23] Move unsafe code out of >::value_for_layout --- components/script/dom/htmltextareaelement.rs | 29 +++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 248dfa33047..1e1e04ee4cb 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -59,32 +59,41 @@ pub trait LayoutHTMLTextAreaElementHelpers { fn value_for_layout(self) -> String; #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option>; - #[allow(unsafe_code)] fn get_cols(self) -> u32; - #[allow(unsafe_code)] fn get_rows(self) -> u32; } -impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { - #[allow(unsafe_code)] - fn value_for_layout(self) -> String { - let text = unsafe { +#[allow(unsafe_code)] +impl<'dom> LayoutDom<'dom, HTMLTextAreaElement> { + fn textinput_content(self) -> DOMString { + unsafe { self.unsafe_get() .textinput .borrow_for_layout() .get_content() - }; + } + } + + fn placeholder(self) -> &'dom str { + unsafe { self.unsafe_get().placeholder.borrow_for_layout() } + } +} + +impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { + fn value_for_layout(self) -> String { + let text = self.textinput_content(); if text.is_empty() { - let placeholder = unsafe { self.unsafe_get().placeholder.borrow_for_layout() }; // FIXME(nox): Would be cool to not allocate a new string if the // placeholder is single line, but that's an unimportant detail. - placeholder.replace("\r\n", "\n").replace("\r", "\n").into() + self.placeholder() + .replace("\r\n", "\n") + .replace("\r", "\n") + .into() } else { text.into() } } - #[allow(unrooted_must_root)] #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option> { if !(*self.unsafe_get()).upcast::().focus_state() { From 0c0027ecfdf380f6813d03a270428b62d42434e5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 00:22:22 +0200 Subject: [PATCH 18/23] Make LayoutDocumentHelpers::style_shared_lock be safe StyleSharedRwLock is Sync. --- components/layout_thread/dom_wrapper.rs | 2 +- components/layout_thread_2020/dom_wrapper.rs | 2 +- components/script/dom/document.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 5c9cf183053..db8d0ffc36d 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -368,7 +368,7 @@ impl<'ld> ServoLayoutDocument<'ld> { } pub fn style_shared_lock(&self) -> &StyleSharedRwLock { - unsafe { self.document.style_shared_lock() } + self.document.style_shared_lock() } pub fn shadow_roots(&self) -> Vec { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 0e527bd75d9..bd187482ee5 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -375,7 +375,7 @@ impl<'ld> ServoLayoutDocument<'ld> { } pub fn style_shared_lock(&self) -> &StyleSharedRwLock { - unsafe { self.document.style_shared_lock() } + self.document.style_shared_lock() } pub fn shadow_roots(&self) -> Vec { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index c3c64babfef..afaf0a5292b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2610,7 +2610,7 @@ pub trait LayoutDocumentHelpers<'dom> { unsafe fn needs_paint_from_layout(self); unsafe fn will_paint(self); fn quirks_mode(self) -> QuirksMode; - unsafe fn style_shared_lock(self) -> &'dom StyleSharedRwLock; + fn style_shared_lock(self) -> &'dom StyleSharedRwLock; fn shadow_roots(self) -> Vec>; fn shadow_roots_styles_changed(self) -> bool; unsafe fn flush_shadow_roots_stylesheets(self); @@ -2639,8 +2639,8 @@ impl<'dom> LayoutDocumentHelpers<'dom> for LayoutDom<'dom, Document> { } #[inline] - unsafe fn style_shared_lock(self) -> &'dom StyleSharedRwLock { - (*self.unsafe_get()).style_shared_lock() + fn style_shared_lock(self) -> &'dom StyleSharedRwLock { + unsafe { self.unsafe_get().style_shared_lock() } } #[inline] From ebd289215852c0fe65ab9633d1bb83243ea6221b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 00:25:49 +0200 Subject: [PATCH 19/23] Make synthesize_presentational_hints_for_legacy_attributes be safe --- components/layout_thread/dom_wrapper.rs | 6 ++---- components/layout_thread_2020/dom_wrapper.rs | 6 ++---- components/script/dom/element.rs | 6 ++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index db8d0ffc36d..e25c67b7f1c 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -639,10 +639,8 @@ impl<'le> TElement for ServoLayoutElement<'le> { ) where V: Push, { - unsafe { - self.element - .synthesize_presentational_hints_for_legacy_attributes(hints); - } + self.element + .synthesize_presentational_hints_for_legacy_attributes(hints); } /// The shadow root this element is a host of. diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index bd187482ee5..3a1128cc46e 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -646,10 +646,8 @@ impl<'le> TElement for ServoLayoutElement<'le> { ) where V: Push, { - unsafe { - self.element - .synthesize_presentational_hints_for_legacy_attributes(hints); - } + self.element + .synthesize_presentational_hints_for_legacy_attributes(hints); } /// The shadow root this element is a host of. diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index b59b342024f..fd5ef8e4ebb 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -567,8 +567,7 @@ pub trait LayoutElementHelpers<'dom> { fn has_class_for_layout(self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool; fn get_classes_for_layout(self) -> Option<&'dom [Atom]>; - #[allow(unsafe_code)] - unsafe fn synthesize_presentational_hints_for_legacy_attributes(self, _: &mut V) + fn synthesize_presentational_hints_for_legacy_attributes(self, hints: &mut V) where V: Push; fn get_colspan(self) -> u32; @@ -616,8 +615,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { .map(|attr| attr.as_tokens().unwrap()) } - #[allow(unsafe_code)] - unsafe fn synthesize_presentational_hints_for_legacy_attributes(self, hints: &mut V) + fn synthesize_presentational_hints_for_legacy_attributes(self, hints: &mut V) where V: Push, { From 295f1204257a804a866e48efcc95c226fd7ea3ed Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 11:25:39 +0200 Subject: [PATCH 20/23] Make LayoutShadowRootHelpers::get_style_data_for_layout return a &CascadeData That return type is Sync, which thus means that the method can be safe. --- components/layout_thread/dom_wrapper.rs | 2 +- components/layout_thread_2020/dom_wrapper.rs | 2 +- components/script/dom/shadowroot.rs | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index e25c67b7f1c..9237e9d47e7 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -177,7 +177,7 @@ impl<'lr> TShadowRoot for ServoShadowRoot<'lr> { where Self: 'a, { - Some(unsafe { &self.shadow_root.get_style_data_for_layout().data }) + Some(&self.shadow_root.get_style_data_for_layout()) } } diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 3a1128cc46e..5a722cd82b8 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -184,7 +184,7 @@ impl<'lr> TShadowRoot for ServoShadowRoot<'lr> { where Self: 'a, { - Some(unsafe { &self.shadow_root.get_style_data_for_layout().data }) + Some(&self.shadow_root.get_style_data_for_layout()) } } diff --git a/components/script/dom/shadowroot.rs b/components/script/dom/shadowroot.rs index df9337eabcc..7e375c4fbba 100644 --- a/components/script/dom/shadowroot.rs +++ b/components/script/dom/shadowroot.rs @@ -27,6 +27,7 @@ use style::dom::TElement; use style::media_queries::Device; use style::shared_lock::SharedRwLockReadGuard; use style::stylesheets::Stylesheet; +use style::stylist::CascadeData; /// Whether a shadow root hosts an User Agent widget. #[derive(JSTraceable, MallocSizeOf, PartialEq)] @@ -241,7 +242,7 @@ impl ShadowRootMethods for ShadowRoot { #[allow(unsafe_code)] pub trait LayoutShadowRootHelpers<'dom> { fn get_host_for_layout(self) -> LayoutDom<'dom, Element>; - unsafe fn get_style_data_for_layout(self) -> &'dom AuthorStyles; + fn get_style_data_for_layout(self) -> &'dom CascadeData; unsafe fn flush_stylesheets( self, device: &Device, @@ -264,8 +265,10 @@ impl<'dom> LayoutShadowRootHelpers<'dom> for LayoutDom<'dom, ShadowRoot> { #[inline] #[allow(unsafe_code)] - unsafe fn get_style_data_for_layout(self) -> &'dom AuthorStyles { - (*self.unsafe_get()).author_styles.borrow_for_layout() + fn get_style_data_for_layout(self) -> &'dom CascadeData { + fn is_sync() {} + let _ = is_sync::; + unsafe { &self.unsafe_get().author_styles.borrow_for_layout().data } } #[inline] From 28e5abe60667653464336ce04a274a907c807d83 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 11:30:21 +0200 Subject: [PATCH 21/23] Introduce >::focus_state --- components/script/dom/element.rs | 12 ++++++++++++ components/script/dom/htmlinputelement.rs | 2 +- components/script/dom/htmltextareaelement.rs | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index fd5ef8e4ebb..87d4327562d 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -592,6 +592,18 @@ pub trait LayoutElementHelpers<'dom> { fn get_attr_vals_for_layout(self, name: &LocalName) -> Vec<&'dom AttrValue>; } +impl<'dom> LayoutDom<'dom, Element> { + #[allow(unsafe_code)] + pub(super) fn focus_state(self) -> bool { + unsafe { + self.unsafe_get() + .state + .get() + .contains(ElementState::IN_FOCUS_STATE) + } + } +} + impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[allow(unsafe_code)] #[inline] diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index f84296b3a66..1e38abc9feb 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -780,7 +780,7 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option> { - if !(*self.unsafe_get()).upcast::().focus_state() { + if !self.upcast::().focus_state() { return None; } diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 1e1e04ee4cb..1cfd6903302 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -96,7 +96,7 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option> { - if !(*self.unsafe_get()).upcast::().focus_state() { + if !self.upcast::().focus_state() { return None; } let textinput = (*self.unsafe_get()).textinput.borrow_for_layout(); From d9e4f7a0ba3f5853174d1aa0185932de9cb5ae06 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 11:35:07 +0200 Subject: [PATCH 22/23] Introduce more layout helpers to make selection_for_layout be safe --- components/script/dom/htmlinputelement.rs | 25 +++++++++++++------- components/script/dom/htmltextareaelement.rs | 18 +++++++++----- components/script/dom/node.rs | 5 ++-- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 1e38abc9feb..22260f30d9d 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -706,8 +706,7 @@ impl HTMLInputElement { pub trait LayoutHTMLInputElementHelpers<'dom> { fn value_for_layout(self) -> Cow<'dom, str>; fn size_for_layout(self) -> u32; - #[allow(unsafe_code)] - unsafe fn selection_for_layout(self) -> Option>; + fn selection_for_layout(self) -> Option>; fn checked_state_for_layout(self) -> bool; fn indeterminate_state_for_layout(self) -> bool; } @@ -730,6 +729,15 @@ impl<'dom> LayoutDom<'dom, HTMLInputElement> { fn input_type(self) -> InputType { unsafe { self.unsafe_get().input_type.get() } } + + fn textinput_sorted_selection_offsets_range(self) -> Range { + unsafe { + self.unsafe_get() + .textinput + .borrow_for_layout() + .sorted_selection_offsets_range() + } + } } impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElement> { @@ -778,18 +786,17 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem unsafe { self.unsafe_get().size.get() } } - #[allow(unsafe_code)] - unsafe fn selection_for_layout(self) -> Option> { + fn selection_for_layout(self) -> Option> { if !self.upcast::().focus_state() { return None; } - let textinput = (*self.unsafe_get()).textinput.borrow_for_layout(); + let sorted_selection_offsets_range = self.textinput_sorted_selection_offsets_range(); match self.input_type() { InputType::Password => { let text = self.get_raw_textinput_value(); - let sel = UTF8Bytes::unwrap_range(textinput.sorted_selection_offsets_range()); + let sel = UTF8Bytes::unwrap_range(sorted_selection_offsets_range); // Translate indices from the raw value to indices in the replacement value. let char_start = text[..sel.start].chars().count(); @@ -798,9 +805,9 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem let bytes_per_char = PASSWORD_REPLACEMENT_CHAR.len_utf8(); Some(char_start * bytes_per_char..char_end * bytes_per_char) }, - input_type if input_type.is_textual() => Some(UTF8Bytes::unwrap_range( - textinput.sorted_selection_offsets_range(), - )), + input_type if input_type.is_textual() => { + Some(UTF8Bytes::unwrap_range(sorted_selection_offsets_range)) + }, _ => None, } } diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 1cfd6903302..38fa642ca4c 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -57,8 +57,7 @@ pub struct HTMLTextAreaElement { pub trait LayoutHTMLTextAreaElementHelpers { fn value_for_layout(self) -> String; - #[allow(unsafe_code)] - unsafe fn selection_for_layout(self) -> Option>; + fn selection_for_layout(self) -> Option>; fn get_cols(self) -> u32; fn get_rows(self) -> u32; } @@ -74,6 +73,15 @@ impl<'dom> LayoutDom<'dom, HTMLTextAreaElement> { } } + fn textinput_sorted_selection_offsets_range(self) -> Range { + unsafe { + self.unsafe_get() + .textinput + .borrow_for_layout() + .sorted_selection_offsets_range() + } + } + fn placeholder(self) -> &'dom str { unsafe { self.unsafe_get().placeholder.borrow_for_layout() } } @@ -94,14 +102,12 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { } } - #[allow(unsafe_code)] - unsafe fn selection_for_layout(self) -> Option> { + fn selection_for_layout(self) -> Option> { if !self.upcast::().focus_state() { return None; } - let textinput = (*self.unsafe_get()).textinput.borrow_for_layout(); Some(UTF8Bytes::unwrap_range( - textinput.sorted_selection_offsets_range(), + self.textinput_sorted_selection_offsets_range(), )) } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index b7215cfd92b..1a889ae0ac1 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1478,14 +1478,13 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { panic!("not text!") } - #[allow(unsafe_code)] fn selection(self) -> Option> { if let Some(area) = self.downcast::() { - return unsafe { area.selection_for_layout() }; + return area.selection_for_layout(); } if let Some(input) = self.downcast::() { - return unsafe { input.selection_for_layout() }; + return input.selection_for_layout(); } None From 4e64a1c682183a5f982f9b6b9cbc6d99b4a3b6ba Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 1 Apr 2020 11:36:25 +0200 Subject: [PATCH 23/23] Add some comments and remove obsolete allow attributes --- components/script/dom/node.rs | 10 +++++++--- components/script/dom/shadowroot.rs | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 1a889ae0ac1..f9007bcf9c6 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1414,6 +1414,10 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { } } + // FIXME(nox): get_flag/set_flag (especially the latter) are not safe because + // they mutate stuff while values of this type can be used from multiple + // threads at once, this should be revisited. + #[inline] #[allow(unsafe_code)] unsafe fn get_flag(self, flag: NodeFlags) -> bool { @@ -1441,6 +1445,9 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { unsafe { self.unsafe_get().children_count.get() } } + // FIXME(nox): How we handle style and layout data needs to be completely + // revisited so we can do that more cleanly and safely in layout 2020. + #[inline] #[allow(unsafe_code)] unsafe fn get_style_and_layout_data(self) -> Option { @@ -1490,19 +1497,16 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { None } - #[allow(unsafe_code)] fn image_url(self) -> Option { self.downcast::() .expect("not an image!") .image_url() } - #[allow(unsafe_code)] fn image_data(self) -> Option<(Option>, Option)> { self.downcast::().map(|e| e.image_data()) } - #[allow(unsafe_code)] fn image_density(self) -> Option { self.downcast::() .expect("not an image!") diff --git a/components/script/dom/shadowroot.rs b/components/script/dom/shadowroot.rs index 7e375c4fbba..f1c0567a399 100644 --- a/components/script/dom/shadowroot.rs +++ b/components/script/dom/shadowroot.rs @@ -271,6 +271,8 @@ impl<'dom> LayoutShadowRootHelpers<'dom> for LayoutDom<'dom, ShadowRoot> { unsafe { &self.unsafe_get().author_styles.borrow_for_layout().data } } + // FIXME(nox): This uses the dreaded borrow_mut_for_layout so this should + // probably be revisited. #[inline] #[allow(unsafe_code)] unsafe fn flush_stylesheets(