Skip to content

Commit

Permalink
Make LayoutNodeHelpers::composed_parent_node_ref be safe
Browse files Browse the repository at this point in the history
For clarity, I introduce <LayoutDom<Element>>::parent_node_ref to contain
the remaining unsafety bits out of composed_parent_node_ref which is more
complex than just a field access.
  • Loading branch information
nox committed Apr 1, 2020
1 parent f712b0b commit fc07a51
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 50 deletions.
18 changes: 7 additions & 11 deletions components/layout_thread/dom_wrapper.rs
Expand Up @@ -203,11 +203,9 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
type ConcreteShadowRoot = ServoShadowRoot<'ln>;

fn parent_node(&self) -> Option<Self> {
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<Self> {
Expand Down Expand Up @@ -745,12 +743,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
}

fn parent_element(&self) -> Option<ServoLayoutElement<'le>> {
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 {
Expand Down
18 changes: 7 additions & 11 deletions components/layout_thread_2020/dom_wrapper.rs
Expand Up @@ -210,11 +210,9 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
type ConcreteShadowRoot = ServoShadowRoot<'ln>;

fn parent_node(&self) -> Option<Self> {
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<Self> {
Expand Down Expand Up @@ -752,12 +750,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
}

fn parent_element(&self) -> Option<ServoLayoutElement<'le>> {
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 {
Expand Down
41 changes: 18 additions & 23 deletions components/script/dom/element.rs
Expand Up @@ -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::<Node>());
while let Some(node) = current_node {
current_node = node.composed_parent_node_ref();
match node.downcast::<Element>() {
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::<Node>());
while let Some(node) = current_node {
current_node = node.composed_parent_node_ref();
match node.downcast::<Element>() {
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]
Expand Down
17 changes: 12 additions & 5 deletions components/script/dom/node.rs
Expand Up @@ -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<LayoutDom<'dom, Node>>;
fn composed_parent_node_ref(self) -> Option<LayoutDom<'dom, Node>>;
fn first_child_ref(self) -> Option<LayoutDom<'dom, Node>>;
fn last_child_ref(self) -> Option<LayoutDom<'dom, Node>>;
fn prev_sibling_ref(self) -> Option<LayoutDom<'dom, Node>>;
Expand Down Expand Up @@ -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<LayoutDom<'dom, Node>> {
unsafe { self.unsafe_get().parent_node.get_inner_as_layout() }
}
}

impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
#[inline]
#[allow(unsafe_code)]
Expand All @@ -1352,10 +1360,9 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
}

#[inline]
#[allow(unsafe_code)]
unsafe fn composed_parent_node_ref(self) -> Option<LayoutDom<'dom, Node>> {
let parent = (*self.unsafe_get()).parent_node.get_inner_as_layout();
if let Some(ref parent) = parent {
fn composed_parent_node_ref(self) -> Option<LayoutDom<'dom, Node>> {
let parent = self.parent_node_ref();
if let Some(parent) = parent {
if let Some(shadow_root) = parent.downcast::<ShadowRoot>() {
return Some(shadow_root.get_host_for_layout().upcast());
}
Expand Down

0 comments on commit fc07a51

Please sign in to comment.