script: Replace use of UnsafeCell in WeakRangeVec. (#37327)

I can't find any historical motivation for the use of UnsafeCell in the
implementation of WeakRangeVec from #8506. We can replace all the uses
of unsafe in this code by using RefCell, and the resulting code is
easier to understand.

Testing: Existing WPT tests using Ranges show no behaviour changes.
Fixes: #37276

Signed-off-by: Josh Matthews <josh@joshmatthews.net>
This commit is contained in:
Josh Matthews 2025-06-11 00:01:12 -04:00 committed by GitHub
parent 6617fe3b91
commit 73361d0f5f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -2,14 +2,13 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use std::cell::UnsafeCell; use std::cell::RefCell;
use std::cmp::{Ordering, PartialOrd}; use std::cmp::{Ordering, PartialOrd};
use std::iter; use std::iter;
use dom_struct::dom_struct; use dom_struct::dom_struct;
use js::jsapi::JSTracer; use js::jsapi::JSTracer;
use js::rust::HandleObject; use js::rust::HandleObject;
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use crate::dom::abstractrange::{AbstractRange, BoundaryPoint, bp_position}; use crate::dom::abstractrange::{AbstractRange, BoundaryPoint, bp_position};
use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::cell::DomRefCell;
@ -1194,14 +1193,15 @@ impl RangeMethods<crate::DomTypeHolder> for Range {
} }
} }
#[derive(MallocSizeOf)]
pub(crate) struct WeakRangeVec { pub(crate) struct WeakRangeVec {
cell: UnsafeCell<WeakRefVec<Range>>, cell: RefCell<WeakRefVec<Range>>,
} }
impl Default for WeakRangeVec { impl Default for WeakRangeVec {
fn default() -> Self { fn default() -> Self {
WeakRangeVec { WeakRangeVec {
cell: UnsafeCell::new(WeakRefVec::new()), cell: RefCell::new(WeakRefVec::new()),
} }
} }
} }
@ -1210,7 +1210,7 @@ impl Default for WeakRangeVec {
impl WeakRangeVec { impl WeakRangeVec {
/// Whether that vector of ranges is empty. /// Whether that vector of ranges is empty.
pub(crate) fn is_empty(&self) -> bool { pub(crate) fn is_empty(&self) -> bool {
unsafe { (*self.cell.get()).is_empty() } self.cell.borrow().is_empty()
} }
/// Used for steps 2.1-2. when inserting a node. /// Used for steps 2.1-2. when inserting a node.
@ -1235,8 +1235,7 @@ impl WeakRangeVec {
let offset = context.index(); let offset = context.index();
let parent = context.parent; let parent = context.parent;
unsafe { let ranges = &mut *self.cell.borrow_mut();
let ranges = &mut *self.cell.get();
ranges.update(|entry| { ranges.update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
@ -1253,8 +1252,12 @@ impl WeakRangeVec {
} }
}); });
(*context.parent.ranges().cell.get()).extend(ranges.drain(..)); context
} .parent
.ranges()
.cell
.borrow_mut()
.extend(ranges.drain(..));
} }
/// Used for steps 7.1-2. when normalizing a node. /// Used for steps 7.1-2. when normalizing a node.
@ -1264,8 +1267,7 @@ impl WeakRangeVec {
return; return;
} }
unsafe { let ranges = &mut *self.cell.borrow_mut();
let ranges = &mut *self.cell.get();
ranges.update(|entry| { ranges.update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
@ -1282,8 +1284,7 @@ impl WeakRangeVec {
} }
}); });
(*sibling.ranges().cell.get()).extend(ranges.drain(..)); sibling.ranges().cell.borrow_mut().extend(ranges.drain(..));
}
} }
/// Used for steps 7.3-4. when normalizing a node. /// Used for steps 7.3-4. when normalizing a node.
@ -1295,10 +1296,10 @@ impl WeakRangeVec {
child: &Node, child: &Node,
new_offset: u32, new_offset: u32,
) { ) {
unsafe { let child_ranges = child.ranges();
let child_ranges = &mut *child.ranges().cell.get(); let mut child_ranges = child_ranges.cell.borrow_mut();
(*self.cell.get()).update(|entry| { self.cell.borrow_mut().update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
let node_is_start = range.start().node() == node; let node_is_start = range.start().node() == node;
@ -1332,7 +1333,6 @@ impl WeakRangeVec {
} }
}); });
} }
}
/// Used for steps 8-11. when replacing character data. /// Used for steps 8-11. when replacing character data.
/// <https://dom.spec.whatwg.org/#concept-cd-replace> /// <https://dom.spec.whatwg.org/#concept-cd-replace>
@ -1360,10 +1360,10 @@ impl WeakRangeVec {
offset: u32, offset: u32,
sibling: &Node, sibling: &Node,
) { ) {
unsafe { let sibling_ranges = sibling.ranges();
let sibling_ranges = &mut *sibling.ranges().cell.get(); let mut sibling_ranges = sibling_ranges.cell.borrow_mut();
(*self.cell.get()).update(|entry| { self.cell.borrow_mut().update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
let start_offset = range.start_offset(); let start_offset = range.start_offset();
let end_offset = range.end_offset(); let end_offset = range.end_offset();
@ -1400,13 +1400,11 @@ impl WeakRangeVec {
} }
}); });
} }
}
/// Used for steps 7.4-5. when splitting a text node. /// Used for steps 7.4-5. when splitting a text node.
/// <https://dom.spec.whatwg.org/#concept-text-split> /// <https://dom.spec.whatwg.org/#concept-text-split>
pub(crate) fn increment_at(&self, node: &Node, offset: u32) { pub(crate) fn increment_at(&self, node: &Node, offset: u32) {
unsafe { self.cell.borrow_mut().update(|entry| {
(*self.cell.get()).update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
if range.start().node() == node && offset == range.start_offset() { if range.start().node() == node && offset == range.start_offset() {
range.report_change(); range.report_change();
@ -1418,11 +1416,9 @@ impl WeakRangeVec {
} }
}); });
} }
}
fn map_offset_above<F: FnMut(u32) -> u32>(&self, node: &Node, offset: u32, mut f: F) { fn map_offset_above<F: FnMut(u32) -> u32>(&self, node: &Node, offset: u32, mut f: F) {
unsafe { self.cell.borrow_mut().update(|entry| {
(*self.cell.get()).update(|entry| {
let range = entry.root().unwrap(); let range = entry.root().unwrap();
let start_offset = range.start_offset(); let start_offset = range.start_offset();
if range.start().node() == node && start_offset > offset { if range.start().node() == node && start_offset > offset {
@ -1436,33 +1432,21 @@ impl WeakRangeVec {
} }
}); });
} }
}
pub(crate) fn push(&self, ref_: WeakRef<Range>) { pub(crate) fn push(&self, ref_: WeakRef<Range>) {
unsafe { self.cell.borrow_mut().push(ref_);
(*self.cell.get()).push(ref_);
}
} }
fn remove(&self, range: &Range) -> WeakRef<Range> { fn remove(&self, range: &Range) -> WeakRef<Range> {
unsafe { let mut ranges = self.cell.borrow_mut();
let ranges = &mut *self.cell.get();
let position = ranges.iter().position(|ref_| ref_ == range).unwrap(); let position = ranges.iter().position(|ref_| ref_ == range).unwrap();
ranges.swap_remove(position) ranges.swap_remove(position)
} }
}
}
#[allow(unsafe_code)]
impl MallocSizeOf for WeakRangeVec {
fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
unsafe { (*self.cell.get()).size_of(ops) }
}
} }
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe impl JSTraceable for WeakRangeVec { unsafe impl JSTraceable for WeakRangeVec {
unsafe fn trace(&self, _: *mut JSTracer) { unsafe fn trace(&self, _: *mut JSTracer) {
(*self.cell.get()).retain_alive() self.cell.borrow_mut().retain_alive()
} }
} }