glib: Remove SignalClassHandlerToken, rework overriden signals

Use the signal invocation hint instead to type check.

glib/src/lib.rs

@@ -164,6 +164,8 @@ pub use self::send_unique::{SendUnique, SendUniqueCell};
 #[macro_use]
 pub mod subclass;
 
+pub use subclass::object::{signal_chain_from_overridden, signal_chain_from_overridden_unchecked};
+
 mod main_context_futures;
 mod source_futures;
 pub use self::source_futures::*;

glib/src/subclass/mod.rs

@@ -276,7 +276,5 @@ pub mod prelude {
 
 pub use self::boxed::register_boxed_type;
 pub use self::interface::register_interface;
-pub use self::signal::{
-    Signal, SignalClassHandlerToken, SignalId, SignalInvocationHint, SignalQuery, SignalType,
-};
+pub use self::signal::{Signal, SignalId, SignalInvocationHint, SignalQuery, SignalType};
 pub use self::types::{register_type, InitializingObject, InitializingType, TypeData};

glib/src/subclass/object.rs

@@ -6,8 +6,10 @@
 
 use super::prelude::*;
 use super::Signal;
+use crate::closure::TryFromClosureReturnValue;
 use crate::translate::*;
-use crate::{Cast, Object, ObjectType, ParamSpec, Value};
+use crate::ToValue;
+use crate::{Cast, Closure, Object, ParamSpec, RustClosure, Type, Value};
 use std::mem;
 use std::ptr;
 
@@ -138,20 +140,76 @@ unsafe extern "C" fn dispose<T: ObjectImpl>(obj: *mut gobject_ffi::GObject) {
 ///
 /// This contains various class methods and allows subclasses to override signal class handlers.
 pub unsafe trait ObjectClassSubclassExt: Sized + 'static {
+    // rustdoc-stripper-ignore-next
+    /// Overrides the handler for a signal. The signal `name` must be installed on the parent
+    /// class or this method will panic.
     fn override_signal_class_handler<F>(&mut self, name: &str, class_handler: F)
     where
-        F: Fn(&super::SignalClassHandlerToken, &[Value]) -> Option<Value> + Send + Sync + 'static,
+        F: Fn(&[Value]) -> Option<Value> + Send + Sync + 'static,
     {
+        let type_ = unsafe { from_glib(*(self as *mut _ as *mut ffi::GType)) };
+        let (signal_id, _) = super::SignalId::parse_name(name, type_, false)
+            .unwrap_or_else(|| panic!("Signal '{}' not found", name));
+        let query = signal_id.query();
+        let return_type = query.return_type().type_();
+        let closure = Closure::new(move |values| {
+            let res = class_handler(values);
+
+            if return_type == Type::UNIT {
+                if let Some(ref v) = res {
+                    panic!(
+                        "Signal has no return value but class handler returned a value of type {}",
+                        v.type_()
+                    );
+                }
+            } else {
+                match res {
+                    None => {
+                        panic!("Signal has a return value but class handler returned none");
+                    }
+                    Some(ref v) => {
+                        assert!(
+                            v.type_().is_a(return_type),
+                            "Signal has a return type of {} but class handler returned {}",
+                            return_type,
+                            v.type_()
+                        );
+                    }
+                }
+            }
+
+            res
+        });
         unsafe {
-            super::types::signal_override_class_handler(
-                name,
-                *(self as *mut _ as *mut ffi::GType),
-                class_handler,
-            );
+            override_signal_class_closure(signal_id, type_, &closure);
+        }
+    }
+    // rustdoc-stripper-ignore-next
+    /// Overrides the handler for a signal using a closure. The signal `name` must be installed on
+    /// the parent class or this method will panic.
+    fn override_signal_class_closure(&mut self, name: &str, closure: RustClosure) {
+        unsafe {
+            let type_ = from_glib(*(self as *mut _ as *mut ffi::GType));
+            let (signal_id, _) = super::SignalId::parse_name(name, type_, false)
+                .unwrap_or_else(|| panic!("Signal '{}' not found", name));
+            override_signal_class_closure(signal_id, type_, closure.as_ref());
         }
     }
 }
 
+#[inline]
+unsafe fn override_signal_class_closure(
+    signal_id: super::SignalId,
+    instance_type: Type,
+    closure: &Closure,
+) {
+    gobject_ffi::g_signal_override_class_closure(
+        signal_id.into_glib(),
+        instance_type.into_glib(),
+        closure.to_glib_none().0,
+    );
+}
+
 unsafe impl ObjectClassSubclassExt for crate::Class<Object> {}
 
 unsafe impl<T: ObjectImpl> IsSubclassable<T> for Object {
@@ -197,12 +255,20 @@ pub trait ObjectImplExt: ObjectSubclass {
     fn parent_constructed(&self, obj: &Self::Type);
 
     // rustdoc-stripper-ignore-next
-    /// Chain up to parent class signal handler.
-    fn signal_chain_from_overridden(
+    /// Calls the original class closure of a signal. This function should only be called from inside
+    /// an overridden class closure registered with
+    /// [`ObjectClassSubclassExt::override_signal_class_handler`] or
+    /// [`ObjectClassSubclassExt::override_signal_class_closure`].
+    ///
+    /// `values` must be the parameters to pass to the parent class closure. Calling this function
+    /// with an incorrect number of values or incorrect types for the values will panic. Logic
+    /// errors may happen if this function is called more than once inside a class closure or if
+    /// this function is called outside a class closure, but it is not undefined behavior.
+    #[doc(alias = "g_signal_chain_from_overridden")]
+    fn signal_chain_from_overridden<R: TryFromClosureReturnValue>(
         &self,
-        token: &super::SignalClassHandlerToken,
-        values: &[Value],
-    ) -> Option<Value>;
+        values: &[&dyn ToValue],
+    ) -> R;
 }
 
 impl<T: ObjectImpl> ObjectImplExt for T {
@@ -216,20 +282,97 @@ impl<T: ObjectImpl> ObjectImplExt for T {
             }
         }
     }
-
-    fn signal_chain_from_overridden(
+    fn signal_chain_from_overridden<R: TryFromClosureReturnValue>(
         &self,
-        token: &super::SignalClassHandlerToken,
-        values: &[Value],
-    ) -> Option<Value> {
-        unsafe {
-            super::types::signal_chain_from_overridden(
-                self.instance().as_ptr() as *mut _,
-                token,
-                values,
-            )
+        values: &[&dyn ToValue],
+    ) -> R {
+        let mut vs = smallvec::SmallVec::<[Value; 10]>::with_capacity(values.len() + 1);
+        vs.push(self.instance().to_value());
+        vs.extend(values.iter().copied().map(ToValue::to_value));
+
+        R::try_from_closure_return_value(signal_chain_from_overridden(&vs))
+            .expect("Invalid return value")
+    }
+}
+
+// rustdoc-stripper-ignore-next
+/// Calls the original class closure of a signal. This function should only be called from inside
+/// an overridden class closure registered with
+/// [`ObjectClassSubclassExt::override_signal_class_handler`] or
+/// [`ObjectClassSubclassExt::override_signal_class_closure`].
+///
+/// The first value in `values` must be the object currently emitting the signal, and the rest are
+/// the parameters to pass to the parent class closure. Calling this function with an incorrect
+/// number of values or incorrect types for the values will panic. Logic errors may happen if this
+/// function is called more than once inside a class closure or if this function is called outside
+/// a class closure, but it is not undefined behavior.
+#[doc(alias = "g_signal_chain_from_overridden")]
+pub fn signal_chain_from_overridden(values: &[Value]) -> Option<Value> {
+    let instance = values
+        .get(0)
+        .and_then(|v| v.get::<&Object>().ok())
+        .expect("First value in array passed to `signal_chain_from_overridden` must be a GObject");
+    let hint: super::SignalInvocationHint = unsafe {
+        from_glib(gobject_ffi::g_signal_get_invocation_hint(
+            instance.to_glib_none().0,
+        ))
+    };
+    let query = hint.signal_id().query();
+    let n_args = query.n_params() as usize + 1;
+    if n_args != values.len() {
+        panic!(
+            "Expected {} values when chaining signal `{}`, got {}",
+            n_args,
+            query.signal_name(),
+            values.len(),
+        );
+    }
+    for (index, arg_type) in query.param_types().iter().enumerate() {
+        let arg_type = arg_type.type_();
+        let index = index + 1;
+        let value = &values[index];
+        if !value.is_type(arg_type) {
+            panic!(
+                "Wrong type for argument {} when chaining signal `{}`: Actual {:?}, requested {:?}",
+                index,
+                query.signal_name(),
+                value.type_(),
+                arg_type,
+            );
         }
     }
+
+    let return_type = query.return_type().type_();
+
+    unsafe { signal_chain_from_overridden_unchecked(return_type, values) }
+}
+
+// rustdoc-stripper-ignore-next
+/// Calls the original class closure of a signal. This function should only be called from inside
+/// an overridden class closure registered with
+/// [`ObjectClassSubclassExt::override_signal_class_handler`] or
+/// [`ObjectClassSubclassExt::override_signal_class_closure`].
+///
+/// The first value in `values` must be the object currently emitting the signal, and the rest are
+/// the parameters to pass to the parent class closure. Logic errors may happen if this
+/// function is called more than once inside a class closure or if this function is called outside
+/// a class closure, but it is not undefined behavior.
+///
+/// # Safety
+///
+/// The types and number of argument values are not checked, and `return_type` must match the
+/// return type of the signal. It is undefined behavior to call this function with types that do
+/// not match the type of the currently running class closure.
+pub unsafe fn signal_chain_from_overridden_unchecked(
+    return_type: Type,
+    values: &[Value],
+) -> Option<Value> {
+    let mut result = Value::from_type(return_type);
+    gobject_ffi::g_signal_chain_from_overridden(
+        values.as_ptr() as *mut Value as *mut gobject_ffi::GValue,
+        result.to_glib_none_mut().0,
+    );
+    Some(result).filter(|r| r.type_().is_valid() && r.type_() != Type::UNIT)
 }
 
 #[cfg(test)]
@@ -241,6 +384,7 @@ mod test {
     // generate the glib namespace through the crate_ident_new utility,
     // and that returns `glib` (and not `crate`) when called inside the glib crate
     use crate as glib;
+    use crate::ObjectType;
     use crate::StaticType;
 
     use std::cell::RefCell;
@@ -329,7 +473,7 @@ mod test {
                             String::static_type().into(),
                         )
                         .action()
-                        .class_handler(|_, args| {
+                        .class_handler(|args| {
                             let obj = args[0]
                                 .get::<super::SimpleObject>()
                                 .expect("Failed to get Object from args[0]");

glib/src/subclass/signal.rs

@@ -45,36 +45,15 @@ pub struct Signal {
     registration: Mutex<SignalRegistration>,
 }
 
-// rustdoc-stripper-ignore-next
-/// Token passed to signal class handlers.
-pub struct SignalClassHandlerToken(
-    // rustdoc-stripper-ignore-next
-    /// Instance for which the signal is emitted.
-    pub(super) *mut gobject_ffi::GTypeInstance,
-    // rustdoc-stripper-ignore-next
-    /// Return type.
-    pub(super) Type,
-    // rustdoc-stripper-ignore-next
-    /// Arguments value array.
-    pub(super) *const Value,
-);
-
-impl fmt::Debug for SignalClassHandlerToken {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        f.debug_struct("SignalClassHandlerToken")
-            .field("type", &unsafe {
-                crate::Object::from_glib_borrow(self.0 as *mut gobject_ffi::GObject)
-            })
-            .finish()
-    }
-}
-
 // rustdoc-stripper-ignore-next
 /// Signal invocation hint passed to signal accumulators.
 #[repr(transparent)]
 pub struct SignalInvocationHint(gobject_ffi::GSignalInvocationHint);
 
 impl SignalInvocationHint {
+    pub fn signal_id(&self) -> SignalId {
+        unsafe { from_glib(self.0.signal_id) }
+    }
     pub fn detail(&self) -> Option<crate::Quark> {
         unsafe { try_from_glib(self.0.detail).ok() }
     }
@@ -84,6 +63,14 @@ impl SignalInvocationHint {
     }
 }
 
+#[doc(hidden)]
+impl FromGlib<*mut gobject_ffi::GSignalInvocationHint> for SignalInvocationHint {
+    unsafe fn from_glib(hint: *mut gobject_ffi::GSignalInvocationHint) -> Self {
+        assert!(!hint.is_null());
+        Self(*hint)
+    }
+}
+
 impl fmt::Debug for SignalInvocationHint {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
         f.debug_struct("SignalInvocationHint")
@@ -662,8 +649,7 @@ impl Signal {
                 handler_return.type_()
             );
 
-            let res = (accumulator.1)(&SignalInvocationHint(*ihint), return_accu, handler_return)
-                .into_glib();
+            let res = (accumulator.1)(&from_glib(ihint), return_accu, handler_return).into_glib();
 
             assert!(
                 return_accu.type_().is_a(return_type.into()),