Fix button callbacks and avoid double-locks in listview.

This commit is contained in:
Ryan McGrath 2021-03-26 14:26:17 -07:00
parent 2f9a5b5e67
commit 2894699ace
No known key found for this signature in database
GPG key ID: DA6CBD9233593DEA
3 changed files with 28 additions and 7 deletions

View file

@ -23,7 +23,7 @@ use std::sync::Once;
use std::cell::RefCell; use std::cell::RefCell;
use std::rc::Rc; use std::rc::Rc;
use objc_id::Id; use objc_id::ShareId;
use objc::declare::ClassDecl; use objc::declare::ClassDecl;
use objc::runtime::{Class, Object, Sel}; use objc::runtime::{Class, Object, Sel};
use objc::{class, msg_send, sel, sel_impl}; use objc::{class, msg_send, sel, sel_impl};
@ -157,8 +157,10 @@ impl Button {
/// Attaches a callback for button press events. Don't get too creative now... /// Attaches a callback for button press events. Don't get too creative now...
/// best just to message pass or something. /// best just to message pass or something.
pub fn set_action<F: Fn() + Send + Sync + 'static>(&mut self, action: F) { pub fn set_action<F: Fn() + Send + Sync + 'static>(&mut self, action: F) {
//let handler = TargetActionHandler::new(&*self.objc, action); // @TODO: This probably isn't ideal but gets the job done for now; needs revisiting.
//self.handler = Some(handler); let this = self.objc.get(|obj| unsafe { ShareId::from_ptr(msg_send![obj, self]) });
let handler = TargetActionHandler::new(&*this, action);
self.handler = Some(handler);
} }
/// Call this to set the background color for the backing layer. /// Call this to set the background color for the backing layer.

View file

@ -428,14 +428,21 @@ impl<T> ListView<T> {
// Note that we need to thread the `with_mut` calls carefully, to avoid deadlocking. // Note that we need to thread the `with_mut` calls carefully, to avoid deadlocking.
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
{ {
self.objc.with_mut(|obj| unsafe { self.objc.get(|obj| unsafe {
let _: () = msg_send![obj, beginUpdates]; let _: () = msg_send![obj, beginUpdates];
}); });
let handle = self.clone_as_handle(); let handle = self.clone_as_handle();
update(handle); update(handle);
self.objc.with_mut(|obj| unsafe { // This is cheating, but there's no good way around it at the moment. If we (mutably) lock in
// Rust here, firing this call will loop back around into `dequeue`, which will then
// hit a double lock.
//
// Personally, I can live with this - `endUpdates` is effectively just flushing the
// already added updates, so with this small hack here we're able to keep the mutable
// borrow structure everywhere else, which feels "correct".
self.objc.get(|obj| unsafe {
let _: () = msg_send![obj, endUpdates]; let _: () = msg_send![obj, endUpdates];
}); });
} }
@ -461,6 +468,7 @@ impl<T> ListView<T> {
// We need to temporarily retain this; it can drop after the underlying NSTableView // We need to temporarily retain this; it can drop after the underlying NSTableView
// has also retained it. // has also retained it.
let x = ShareId::from_ptr(index_set); let x = ShareId::from_ptr(index_set);
self.objc.with_mut(|obj| { self.objc.with_mut(|obj| {
let _: () = msg_send![obj, insertRowsAtIndexes:&*x withAnimation:animation_options]; let _: () = msg_send![obj, insertRowsAtIndexes:&*x withAnimation:animation_options];
}); });
@ -483,7 +491,8 @@ impl<T> ListView<T> {
let ye: id = msg_send![class!(NSIndexSet), indexSetWithIndex:0]; let ye: id = msg_send![class!(NSIndexSet), indexSetWithIndex:0];
let y = ShareId::from_ptr(ye); let y = ShareId::from_ptr(ye);
self.objc.with_mut(|obj| { // Must use `get` to avoid a double lock.
self.objc.get(|obj| {
let _: () = msg_send![obj, reloadDataForRowIndexes:&*x columnIndexes:&*y]; let _: () = msg_send![obj, reloadDataForRowIndexes:&*x columnIndexes:&*y];
}); });
} }
@ -620,7 +629,10 @@ impl<T> ListView<T> {
impl<T> Layout for ListView<T> { impl<T> Layout for ListView<T> {
fn with_backing_node<F: Fn(id)>(&self, handler: F) { fn with_backing_node<F: Fn(id)>(&self, handler: F) {
self.objc.with_mut(handler); // On macOS, we need to provide the scrollview for layout purposes - iOS and tvOS will know
// what to do normally.
#[cfg(target_os = "macos")]
self.scrollview.objc.with_mut(handler);
} }
} }

View file

@ -25,6 +25,13 @@ impl ObjcProperty {
}))) })))
} }
/// Given an Objective-C object, retains it and wraps it as a `Property`.
pub fn from_retained(obj: id) -> Self {
ObjcProperty(Rc::new(RefCell::new(unsafe {
Id::from_retained_ptr(obj)
})))
}
/// Runs a handler with mutable access for the underlying Objective-C object. /// Runs a handler with mutable access for the underlying Objective-C object.
/// ///
/// Note that this is mutable access from the Rust side; we make every effort to ensure things are valid /// Note that this is mutable access from the Rust side; we make every effort to ensure things are valid