diff options
author | bors-servo <servo-ops@mozilla.com> | 2020-06-04 20:55:08 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-04 20:55:08 -0400 |
commit | bce6eccced1428df44c803ba81d7011abbd120ac (patch) | |
tree | f3af16a1a4a3884471c636e5caea2f1ff30f7781 | |
parent | 98fe3603906f4b76bc2fc1f8a23db5b2e73556ff (diff) | |
parent | a5d0e0b1c12b3bd60d1ad3e71139de2c6b493e50 (diff) | |
download | servo-bce6eccced1428df44c803ba81d7011abbd120ac.tar.gz servo-bce6eccced1428df44c803ba81d7011abbd120ac.zip |
Auto merge of #26790 - jdm:fewer-generics, r=SimonSapin
Reduce scope of generic code in script
Combined, these changes account for almost 100k lines of generated code in a debug build for the script crate.
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes
-rw-r--r-- | components/script/dom/bindings/cell.rs | 12 | ||||
-rw-r--r-- | components/script/dom/bindings/conversions.rs | 28 | ||||
-rw-r--r-- | components/script/dom/bindings/refcounted.rs | 58 | ||||
-rw-r--r-- | components/script/dom/bindings/root.rs | 62 | ||||
-rw-r--r-- | components/script/dom/bindings/utils.rs | 9 |
5 files changed, 103 insertions, 66 deletions
diff --git a/components/script/dom/bindings/cell.rs b/components/script/dom/bindings/cell.rs index 5807138c948..a9aa8119fa9 100644 --- a/components/script/dom/bindings/cell.rs +++ b/components/script/dom/bindings/cell.rs @@ -4,6 +4,7 @@ //! A shareable mutable container for the DOM. +use crate::dom::bindings::root::{assert_in_layout, assert_in_script}; #[cfg(feature = "refcell_backtrace")] pub use accountable_refcell::{ref_filter_map, Ref, RefCell, RefMut}; #[cfg(not(feature = "refcell_backtrace"))] @@ -11,7 +12,6 @@ pub use ref_filter_map::ref_filter_map; use std::cell::{BorrowError, BorrowMutError}; #[cfg(not(feature = "refcell_backtrace"))] pub use std::cell::{Ref, RefCell, RefMut}; -use style::thread_state::{self, ThreadState}; /// A mutable field in the DOM. /// @@ -31,7 +31,7 @@ impl<T> DomRefCell<T> { /// For use in the layout thread only. #[allow(unsafe_code)] pub unsafe fn borrow_for_layout(&self) -> &T { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); self.value .try_borrow_unguarded() .expect("cell is mutably borrowed") @@ -41,7 +41,7 @@ impl<T> DomRefCell<T> { /// #[allow(unsafe_code)] pub unsafe fn borrow_for_script_deallocation(&self) -> &mut T { - debug_assert!(thread_state::get().contains(ThreadState::SCRIPT)); + assert_in_script(); &mut *self.value.as_ptr() } @@ -49,7 +49,7 @@ impl<T> DomRefCell<T> { /// `RefCell::try_borrow_mut_unguarded` but that doesn't exist yet. #[allow(unsafe_code)] pub unsafe fn borrow_mut_for_layout(&self) -> &mut T { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); &mut *self.value.as_ptr() } } @@ -105,7 +105,7 @@ impl<T> DomRefCell<T> { /// /// Panics if this is called off the script thread. pub fn try_borrow(&self) -> Result<Ref<T>, BorrowError> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); self.value.try_borrow() } @@ -120,7 +120,7 @@ impl<T> DomRefCell<T> { /// /// Panics if this is called off the script thread. pub fn try_borrow_mut(&self) -> Result<RefMut<T>, BorrowMutError> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); self.value.try_borrow_mut() } } diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index da4334207bf..97bd3338d8d 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -404,6 +404,11 @@ pub unsafe fn get_dom_class(obj: *mut JSObject) -> Result<&'static DOMClass, ()> Err(()) } +pub(crate) enum PrototypeCheck { + Derive(fn(&'static DOMClass) -> bool), + Depth { depth: usize, proto_id: u16 }, +} + /// Get a `*const libc::c_void` for the given DOM object, unwrapping any /// wrapper around it first, and checking if the object is of the correct type. /// @@ -411,14 +416,11 @@ pub unsafe fn get_dom_class(obj: *mut JSObject) -> Result<&'static DOMClass, ()> /// not an object for a DOM object of the given type (as defined by the /// proto_id and proto_depth). #[inline] -pub unsafe fn private_from_proto_check<F>( +pub(crate) unsafe fn private_from_proto_check( mut obj: *mut JSObject, cx: *mut JSContext, - proto_check: F, -) -> Result<*const libc::c_void, ()> -where - F: Fn(&'static DOMClass) -> bool, -{ + proto_check: PrototypeCheck, +) -> Result<*const libc::c_void, ()> { let dom_class = get_dom_class(obj).or_else(|_| { if IsWrapper(obj) { trace!("found wrapper"); @@ -437,7 +439,14 @@ where } })?; - if proto_check(dom_class) { + let prototype_matches = match proto_check { + PrototypeCheck::Derive(f) => (f)(dom_class), + PrototypeCheck::Depth { depth, proto_id } => { + dom_class.interface_chain[depth] as u16 == proto_id + }, + }; + + if prototype_matches { trace!("good prototype"); Ok(private_from_object(obj)) } else { @@ -471,7 +480,10 @@ pub fn native_from_object<T>(obj: *mut JSObject, cx: *mut JSContext) -> Result<* where T: DomObject + IDLInterface, { - unsafe { private_from_proto_check(obj, cx, T::derives).map(|ptr| ptr as *const T) } + unsafe { + private_from_proto_check(obj, cx, PrototypeCheck::Derive(T::derives)) + .map(|ptr| ptr as *const T) + } } /// Get a `*const T` for a DOM object accessible from a `JSObject`, where the DOM object diff --git a/components/script/dom/bindings/refcounted.rs b/components/script/dom/bindings/refcounted.rs index ad5be5bb0cf..ab7aea3e922 100644 --- a/components/script/dom/bindings/refcounted.rs +++ b/components/script/dom/bindings/refcounted.rs @@ -50,12 +50,15 @@ mod dummy { pub use self::dummy::LIVE_REFERENCES; /// A pointer to a Rust DOM object that needs to be destroyed. -pub struct TrustedReference(*const libc::c_void); +struct TrustedReference(*const libc::c_void); unsafe impl Send for TrustedReference {} impl TrustedReference { - fn new<T: DomObject>(ptr: *const T) -> TrustedReference { - TrustedReference(ptr as *const libc::c_void) + /// Creates a new TrustedReference from a pointer to a value that impements DOMObject. + /// This is not enforced by the type system to reduce duplicated generic code, + /// which is acceptable since this method is internal to this module. + unsafe fn new(ptr: *const libc::c_void) -> TrustedReference { + TrustedReference(ptr) } } @@ -156,7 +159,7 @@ pub struct Trusted<T: DomObject> { /// A pointer to the Rust DOM object of type T, but void to allow /// sending `Trusted<T>` between threads, regardless of T's sendability. refcount: Arc<TrustedReference>, - owner_thread: *const libc::c_void, + owner_thread: *const LiveDOMReferences, phantom: PhantomData<T>, } @@ -167,27 +170,37 @@ impl<T: DomObject> Trusted<T> { /// be prevented from being GCed for the duration of the resulting `Trusted<T>` object's /// lifetime. pub fn new(ptr: &T) -> Trusted<T> { - LIVE_REFERENCES.with(|ref r| { - let r = r.borrow(); - let live_references = r.as_ref().unwrap(); - let refcount = live_references.addref(&*ptr as *const T); - Trusted { - refcount: refcount, - owner_thread: (&*live_references) as *const _ as *const libc::c_void, - phantom: PhantomData, - } - }) + fn add_live_reference( + ptr: *const libc::c_void, + ) -> (Arc<TrustedReference>, *const LiveDOMReferences) { + LIVE_REFERENCES.with(|ref r| { + let r = r.borrow(); + let live_references = r.as_ref().unwrap(); + let refcount = unsafe { live_references.addref(ptr) }; + (refcount, live_references as *const _) + }) + } + + let (refcount, owner_thread) = add_live_reference(ptr as *const T as *const _); + Trusted { + refcount, + owner_thread, + phantom: PhantomData, + } } /// Obtain a usable DOM pointer from a pinned `Trusted<T>` value. Fails if used on /// a different thread than the original value from which this `Trusted<T>` was /// obtained. pub fn root(&self) -> DomRoot<T> { - assert!(LIVE_REFERENCES.with(|ref r| { - let r = r.borrow(); - let live_references = r.as_ref().unwrap(); - self.owner_thread == (&*live_references) as *const _ as *const libc::c_void - })); + fn validate(owner_thread: *const LiveDOMReferences) { + assert!(LIVE_REFERENCES.with(|ref r| { + let r = r.borrow(); + let live_references = r.as_ref().unwrap(); + owner_thread == live_references + })); + } + validate(self.owner_thread); unsafe { DomRoot::from_ref(&*(self.refcount.0 as *const T)) } } } @@ -234,7 +247,10 @@ impl LiveDOMReferences { table.entry(&*promise).or_insert(vec![]).push(promise) } - fn addref<T: DomObject>(&self, ptr: *const T) -> Arc<TrustedReference> { + /// ptr must be a pointer to a type that implements DOMObject. + /// This is not enforced by the type system to reduce duplicated generic code, + /// which is acceptable since this method is internal to this module. + unsafe fn addref(&self, ptr: *const libc::c_void) -> Arc<TrustedReference> { let mut table = self.reflectable_table.borrow_mut(); let capacity = table.capacity(); let len = table.len(); @@ -243,7 +259,7 @@ impl LiveDOMReferences { remove_nulls(&mut table); table.reserve(len); } - match table.entry(ptr as *const libc::c_void) { + match table.entry(ptr) { Occupied(mut entry) => match entry.get().upgrade() { Some(refcount) => refcount, None => { diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index 36bc2296ee5..b0b7eeb146d 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -64,7 +64,7 @@ where #[allow(unrooted_must_root)] pub unsafe fn new(value: T) -> Self { unsafe fn add_to_root_list(object: *const dyn JSTraceable) -> *const RootCollection { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); STACK_ROOTS.with(|ref root_list| { let root_list = &*root_list.get().unwrap(); root_list.root(object); @@ -137,7 +137,7 @@ where type Target = <T as Deref>::Target; fn deref(&self) -> &Self::Target { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); &self.value } } @@ -251,7 +251,7 @@ impl<'a> Drop for ThreadLocalStackRoots<'a> { impl RootCollection { /// Create an empty collection of roots pub fn new() -> RootCollection { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); RootCollection { roots: UnsafeCell::new(vec![]), } @@ -259,13 +259,13 @@ impl RootCollection { /// Starts tracking a trace object. unsafe fn root(&self, object: *const dyn JSTraceable) { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); (*self.roots.get()).push(object); } /// Stops tracking a trace object, asserting if it isn't found. unsafe fn unroot(&self, object: *const dyn JSTraceable) { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); let roots = &mut *self.roots.get(); match roots .iter() @@ -333,7 +333,7 @@ impl<T> MallocSizeOf for Dom<T> { impl<T> Dom<T> { /// Returns `LayoutDom<T>` containing the same pointer. pub unsafe fn to_layout(&self) -> LayoutDom<T> { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); LayoutDom { value: self.ptr.as_ref(), } @@ -344,7 +344,7 @@ impl<T: DomObject> Dom<T> { /// Create a Dom<T> from a &T #[allow(unrooted_must_root)] pub fn from_ref(obj: &T) -> Dom<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); Dom { ptr: ptr::NonNull::from(obj), } @@ -355,7 +355,7 @@ impl<T: DomObject> Deref for Dom<T> { type Target = T; fn deref(&self) -> &T { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); // We can only have &Dom<T> from a rooted thing, so it's safe to deref // it to &T. unsafe { &*self.ptr.as_ptr() } @@ -432,7 +432,7 @@ where U: Castable, T: DerivedFrom<U>, { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); LayoutDom { value: self.value.upcast::<U>(), } @@ -443,7 +443,7 @@ where where U: DerivedFrom<T>, { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); self.value.downcast::<U>().map(|value| LayoutDom { value }) } @@ -452,7 +452,7 @@ where where U: DerivedFrom<T>, { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); self.value.is::<U>() } } @@ -463,7 +463,7 @@ where { /// Get the reflector. pub unsafe fn get_jsobject(&self) -> *mut JSObject { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); self.value.reflector().get_jsobject().get() } } @@ -508,7 +508,7 @@ impl<T> Clone for Dom<T> { #[inline] #[allow(unrooted_must_root)] fn clone(&self) -> Self { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); Dom { ptr: self.ptr.clone(), } @@ -518,7 +518,7 @@ impl<T> Clone for Dom<T> { impl<T> Clone for LayoutDom<'_, T> { #[inline] fn clone(&self) -> Self { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); LayoutDom { value: self.value } } } @@ -527,7 +527,7 @@ impl LayoutDom<'_, Node> { /// Create a new JS-owned value wrapped from an address known to be a /// `Node` pointer. pub unsafe fn from_trusted_node_address(inner: TrustedNodeAddress) -> Self { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); let TrustedNodeAddress(addr) = inner; LayoutDom { value: &*(addr as *const Node), @@ -549,7 +549,7 @@ pub struct MutDom<T: DomObject> { impl<T: DomObject> MutDom<T> { /// Create a new `MutDom`. pub fn new(initial: &T) -> MutDom<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); MutDom { val: UnsafeCell::new(Dom::from_ref(initial)), } @@ -557,7 +557,7 @@ impl<T: DomObject> MutDom<T> { /// Set this `MutDom` to the given value. pub fn set(&self, val: &T) { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); unsafe { *self.val.get() = Dom::from_ref(val); } @@ -565,7 +565,7 @@ impl<T: DomObject> MutDom<T> { /// Get the value in this `MutDom`. pub fn get(&self) -> DomRoot<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); unsafe { DomRoot::from_ref(&*ptr::read(self.val.get())) } } } @@ -589,6 +589,14 @@ impl<T: DomObject + PartialEq> PartialEq<T> for MutDom<T> { } } +pub(crate) fn assert_in_script() { + debug_assert!(thread_state::get().is_script()); +} + +pub(crate) fn assert_in_layout() { + debug_assert!(thread_state::get().is_layout()); +} + /// A holder that provides interior mutability for GC-managed values such as /// `Dom<T>`, with nullability represented by an enclosing Option wrapper. /// Essentially a `Cell<Option<Dom<T>>>`, but safer. @@ -604,7 +612,7 @@ pub struct MutNullableDom<T: DomObject> { impl<T: DomObject> MutNullableDom<T> { /// Create a new `MutNullableDom`. pub fn new(initial: Option<&T>) -> MutNullableDom<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); MutNullableDom { ptr: UnsafeCell::new(initial.map(Dom::from_ref)), } @@ -616,7 +624,7 @@ impl<T: DomObject> MutNullableDom<T> { where F: FnOnce() -> DomRoot<T>, { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); match self.get() { Some(inner) => inner, None => { @@ -631,20 +639,20 @@ impl<T: DomObject> MutNullableDom<T> { /// For use by layout, which can't use safe types like Temporary. #[allow(unrooted_must_root)] pub unsafe fn get_inner_as_layout(&self) -> Option<LayoutDom<T>> { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); (*self.ptr.get()).as_ref().map(|js| js.to_layout()) } /// Get a rooted value out of this object #[allow(unrooted_must_root)] pub fn get(&self) -> Option<DomRoot<T>> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); unsafe { ptr::read(self.ptr.get()).map(|o| DomRoot::from_ref(&*o)) } } /// Set this `MutNullableDom` to the given value. pub fn set(&self, val: Option<&T>) { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); unsafe { *self.ptr.get() = val.map(|p| Dom::from_ref(p)); } @@ -673,7 +681,7 @@ impl<'a, T: DomObject> PartialEq<Option<&'a T>> for MutNullableDom<T> { impl<T: DomObject> Default for MutNullableDom<T> { #[allow(unrooted_must_root)] fn default() -> MutNullableDom<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); MutNullableDom { ptr: UnsafeCell::new(None), } @@ -709,7 +717,7 @@ where where F: FnOnce() -> DomRoot<T>, { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); &self.ptr.init_once(|| Dom::from_ref(&cb())) } } @@ -717,7 +725,7 @@ where impl<T: DomObject> Default for DomOnceCell<T> { #[allow(unrooted_must_root)] fn default() -> DomOnceCell<T> { - debug_assert!(thread_state::get().is_script()); + assert_in_script(); DomOnceCell { ptr: OnceCell::new(), } @@ -747,7 +755,7 @@ where /// Returns a reference to the interior of this JS object. The fact /// that this is unsafe is what necessitates the layout wrappers. pub unsafe fn unsafe_get(self) -> &'dom T { - debug_assert!(thread_state::get().is_layout()); + assert_in_layout(); self.value } diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 80217fea8fb..be513aa0244 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -7,7 +7,9 @@ use crate::dom::bindings::codegen::InterfaceObjectMap; use crate::dom::bindings::codegen::PrototypeList; use crate::dom::bindings::codegen::PrototypeList::{MAX_PROTO_CHAIN_LENGTH, PROTO_OR_IFACE_LENGTH}; -use crate::dom::bindings::conversions::{jsstring_to_str, private_from_proto_check}; +use crate::dom::bindings::conversions::{ + jsstring_to_str, private_from_proto_check, PrototypeCheck, +}; use crate::dom::bindings::error::throw_invalid_this; use crate::dom::bindings::inheritance::TopTypeId; use crate::dom::bindings::str::DOMString; @@ -507,9 +509,8 @@ unsafe fn generic_call( } else { GetNonCCWObjectGlobal(JS_CALLEE(cx, vp).to_object_or_null()) }); - let depth = (*info).__bindgen_anon_3.depth; - let proto_check = - |class: &'static DOMClass| class.interface_chain[depth as usize] as u16 == proto_id; + let depth = (*info).__bindgen_anon_3.depth as usize; + let proto_check = PrototypeCheck::Depth { depth, proto_id }; let this = match private_from_proto_check(obj.get(), cx, proto_check) { Ok(val) => val, Err(()) => { |