Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-19 13:23:46 +01:00
Code Issues Projects Releases Packages Wiki Activity

script: Wrapping unsafe code in unsafe blocks for basic DOM types (#37997)

Browse source 
There is a new default cargo clippy lint, `unsafe_op_in_unsafe_fn`,
which requires unsafe code to be wrapped in unsafe blocks, even inside
functions marked as unsafe. The lint is disabled as much of our code
doesn't fulfill this contract. The thing itself is pretty useful in
order to gradually remove unsafety, so this change starts adding
`unsafe` blocks so we can eventually enable this lint.

Testing: This doesn't change behavior so existings tests should suffice.
Fixes: This is part of #35955.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-07-11 13:38:02 +02:00 • committed by GitHub
parent a13cc1b25a
commit 2366a8bf9e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 51 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

16
components/script/dom/node.rs
View file

@ -1600,7 +1600,8 @@ where
/// returns it. /// returns it.
#[allow(unsafe_code)] #[allow(unsafe_code)]
pub(crate) unsafe fn from_untrusted_node_address(candidate: UntrustedNodeAddress) -> DomRoot<Node> { pub(crate) unsafe fn from_untrusted_node_address(candidate: UntrustedNodeAddress) -> DomRoot<Node> {
DomRoot::from_ref(Node::from_untrusted_node_address(candidate)) let node = unsafe { Node::from_untrusted_node_address(candidate) };
DomRoot::from_ref(node)
} }
#[allow(unsafe_code)] #[allow(unsafe_code)]
@ -1806,7 +1807,7 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
#[inline] #[inline]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn initialize_style_data(self) { unsafe fn initialize_style_data(self) {
let data = self.unsafe_get().style_data.borrow_mut_for_layout(); let data = unsafe { self.unsafe_get().style_data.borrow_mut_for_layout() };
debug_assert!(data.is_none()); debug_assert!(data.is_none());
*data = Some(Box::default()); *data = Some(Box::default());
} }
@ -1814,7 +1815,7 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
#[inline] #[inline]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn initialize_layout_data(self, new_data: Box<GenericLayoutData>) { unsafe fn initialize_layout_data(self, new_data: Box<GenericLayoutData>) {
let data = self.unsafe_get().layout_data.borrow_mut_for_layout(); let data = unsafe { self.unsafe_get().layout_data.borrow_mut_for_layout() };
debug_assert!(data.is_none()); debug_assert!(data.is_none());
*data = Some(new_data); *data = Some(new_data);
} }
@ -1822,8 +1823,10 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
#[inline] #[inline]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn clear_style_and_layout_data(self) { unsafe fn clear_style_and_layout_data(self) {
self.unsafe_get().style_data.borrow_mut_for_layout().take(); unsafe {
self.unsafe_get().layout_data.borrow_mut_for_layout().take(); self.unsafe_get().style_data.borrow_mut_for_layout().take();
self.unsafe_get().layout_data.borrow_mut_for_layout().take();
}
} }
fn is_text_input(&self) -> bool { fn is_text_input(&self) -> bool {
@ -3054,7 +3057,8 @@ impl Node {
if object.is_null() { if object.is_null() {
panic!("Attempted to create a `Node` from an invalid pointer!") panic!("Attempted to create a `Node` from an invalid pointer!")
} }
&*(conversions::private_from_object(object) as *const Self)
unsafe { &*(conversions::private_from_object(object) as *const Self) }
} }
pub(crate) fn html_serialize( pub(crate) fn html_serialize(

2
components/script/dom/window.rs
View file

@ -3201,7 +3201,7 @@ pub(crate) struct LayoutValue<T: MallocSizeOf> {
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe impl<T: JSTraceable + MallocSizeOf> JSTraceable for LayoutValue<T> { unsafe impl<T: JSTraceable + MallocSizeOf> JSTraceable for LayoutValue<T> {
unsafe fn trace(&self, trc: *mut js::jsapi::JSTracer) { unsafe fn trace(&self, trc: *mut js::jsapi::JSTracer) {
self.value.trace(trc) unsafe { self.value.trace(trc) };
} }
} }

42
components/script/layout_dom/element.rs
View file

@ -99,9 +99,11 @@ impl<'dom> ServoLayoutElement<'dom> {
/// This function accesses and modifies the underlying DOM object and should /// This function accesses and modifies the underlying DOM object and should
/// not be used by more than a single thread at once. /// not be used by more than a single thread at once.
pub unsafe fn unset_snapshot_flags(&self) { pub unsafe fn unset_snapshot_flags(&self) {
self.as_node() unsafe {
.node self.as_node()
.set_flag(NodeFlags::HAS_SNAPSHOT | NodeFlags::HANDLED_SNAPSHOT, false); .node
.set_flag(NodeFlags::HAS_SNAPSHOT | NodeFlags::HANDLED_SNAPSHOT, false);
}
} }
/// Unset the snapshot flags on the underlying DOM object for this element. /// Unset the snapshot flags on the underlying DOM object for this element.
@ -111,7 +113,9 @@ impl<'dom> ServoLayoutElement<'dom> {
/// This function accesses and modifies the underlying DOM object and should /// This function accesses and modifies the underlying DOM object and should
/// not be used by more than a single thread at once. /// not be used by more than a single thread at once.
pub unsafe fn set_has_snapshot(&self) { pub unsafe fn set_has_snapshot(&self) {
self.as_node().node.set_flag(NodeFlags::HAS_SNAPSHOT, true); unsafe {
self.as_node().node.set_flag(NodeFlags::HAS_SNAPSHOT, true);
}
} }
/// Returns true if this element is the body child of an html element root element. /// Returns true if this element is the body child of an html element root element.
@ -355,22 +359,28 @@ impl<'dom> style::dom::TElement for ServoLayoutElement<'dom> {
} }
unsafe fn set_handled_snapshot(&self) { unsafe fn set_handled_snapshot(&self) {
self.as_node() unsafe {
.node self.as_node()
.set_flag(NodeFlags::HANDLED_SNAPSHOT, true); .node
.set_flag(NodeFlags::HANDLED_SNAPSHOT, true);
}
} }
unsafe fn set_dirty_descendants(&self) { unsafe fn set_dirty_descendants(&self) {
debug_assert!(self.as_node().is_connected()); debug_assert!(self.as_node().is_connected());
self.as_node() unsafe {
.node self.as_node()
.set_flag(NodeFlags::HAS_DIRTY_DESCENDANTS, true) .node
.set_flag(NodeFlags::HAS_DIRTY_DESCENDANTS, true)
}
} }
unsafe fn unset_dirty_descendants(&self) { unsafe fn unset_dirty_descendants(&self) {
self.as_node() unsafe {
.node self.as_node()
.set_flag(NodeFlags::HAS_DIRTY_DESCENDANTS, false) .node
.set_flag(NodeFlags::HAS_DIRTY_DESCENDANTS, false)
}
} }
/// Whether this element should match user and content rules. /// Whether this element should match user and content rules.
@ -406,11 +416,13 @@ impl<'dom> style::dom::TElement for ServoLayoutElement<'dom> {
} }
unsafe fn clear_data(&self) { unsafe fn clear_data(&self) {
self.as_node().get_jsmanaged().clear_style_and_layout_data() unsafe { self.as_node().get_jsmanaged().clear_style_and_layout_data() }
} }
unsafe fn ensure_data(&self) -> AtomicRefMut<ElementData> { unsafe fn ensure_data(&self) -> AtomicRefMut<ElementData> {
self.as_node().get_jsmanaged().initialize_style_data(); unsafe {
self.as_node().get_jsmanaged().initialize_style_data();
};
self.mutate_data().unwrap() self.mutate_data().unwrap()
} }

13
components/script/layout_dom/node.rs
View file

@ -77,7 +77,8 @@ impl<'dom> ServoLayoutNode<'dom> {
/// ///
/// The address pointed to by `address` should point to a valid node in memory. /// The address pointed to by `address` should point to a valid node in memory.
pub unsafe fn new(address: &TrustedNodeAddress) -> Self { pub unsafe fn new(address: &TrustedNodeAddress) -> Self {
ServoLayoutNode::from_layout_js(LayoutDom::from_trusted_node_address(*address)) let node = unsafe { LayoutDom::from_trusted_node_address(*address) };
ServoLayoutNode::from_layout_js(node)
} }
pub(super) fn script_type_id(&self) -> NodeTypeId { pub(super) fn script_type_id(&self) -> NodeTypeId {
@ -194,10 +195,10 @@ impl<'dom> LayoutNode<'dom> for ServoLayoutNode<'dom> {
unsafe fn initialize_style_and_layout_data<RequestedLayoutDataType: LayoutDataTrait>(&self) { unsafe fn initialize_style_and_layout_data<RequestedLayoutDataType: LayoutDataTrait>(&self) {
let inner = self.get_jsmanaged(); let inner = self.get_jsmanaged();
if inner.style_data().is_none() { if inner.style_data().is_none() {
inner.initialize_style_data(); unsafe { inner.initialize_style_data() };
} }
if inner.layout_data().is_none() { if inner.layout_data().is_none() {
inner.initialize_layout_data(Box::<RequestedLayoutDataType>::default()); unsafe { inner.initialize_layout_data(Box::<RequestedLayoutDataType>::default()) };
} }
} }
@ -251,7 +252,8 @@ impl<'dom> ServoThreadSafeLayoutNode<'dom> {
/// Get the first child of this node. Important: this is not safe for /// Get the first child of this node. Important: this is not safe for
/// layout to call, so it should *never* be made public. /// layout to call, so it should *never* be made public.
unsafe fn dangerous_first_child(&self) -> Option<Self> { unsafe fn dangerous_first_child(&self) -> Option<Self> {
self.get_jsmanaged() let js_managed = unsafe { self.get_jsmanaged() };
js_managed
.first_child_ref() .first_child_ref()
.map(ServoLayoutNode::from_layout_js) .map(ServoLayoutNode::from_layout_js)
.map(Self::new) .map(Self::new)
@ -260,7 +262,8 @@ impl<'dom> ServoThreadSafeLayoutNode<'dom> {
/// Get the next sibling of this node. Important: this is not safe for /// Get the next sibling of this node. Important: this is not safe for
/// layout to call, so it should *never* be made public. /// layout to call, so it should *never* be made public.
unsafe fn dangerous_next_sibling(&self) -> Option<Self> { unsafe fn dangerous_next_sibling(&self) -> Option<Self> {
self.get_jsmanaged() let js_managed = unsafe { self.get_jsmanaged() };
js_managed
.next_sibling_ref() .next_sibling_ref()
.map(ServoLayoutNode::from_layout_js) .map(ServoLayoutNode::from_layout_js)
.map(Self::new) .map(Self::new)

6
components/script/layout_dom/shadow_root.rs
View file

@ -59,7 +59,9 @@ impl<'dom> ServoShadowRoot<'dom> {
stylist: &mut Stylist, stylist: &mut Stylist,
guard: &StyleSharedRwLockReadGuard, guard: &StyleSharedRwLockReadGuard,
) { ) {
self.shadow_root unsafe {
.flush_stylesheets::<ServoLayoutElement>(stylist, guard) self.shadow_root
.flush_stylesheets::<ServoLayoutElement>(stylist, guard)
}
} }
} }

2
components/script/script_thread.rs
View file

@ -179,7 +179,7 @@ pub(crate) fn with_script_thread<R: Default>(f: impl FnOnce(&ScriptThread) -> R)
pub(crate) unsafe fn trace_thread(tr: *mut JSTracer) { pub(crate) unsafe fn trace_thread(tr: *mut JSTracer) {
with_script_thread(|script_thread| { with_script_thread(|script_thread| {
trace!("tracing fields of ScriptThread"); trace!("tracing fields of ScriptThread");
script_thread.trace(tr); unsafe { script_thread.trace(tr) };
}) })
} }