From db33c382b7abeb8fdcac5bd9d0cc83773e2d5703 Mon Sep 17 00:00:00 2001
From: Ryan McGrath <ryan@rymc.io>
Date: Wed, 11 Mar 2020 19:45:35 -0700
Subject: [PATCH] Fix up the NSToolbar wrapper to follow the new paradigm

---
 appkit/src/lib.rs                   |   2 +-
 appkit/src/pasteboard/pasteboard.rs |   2 +-
 appkit/src/toolbar/item.rs          |  18 ++--
 appkit/src/toolbar/mod.rs           |   9 +-
 appkit/src/toolbar/toolbar.rs       | 153 +++++++++++++++-------------
 appkit/src/toolbar/traits.rs        |  17 ++++
 appkit/src/view/view.rs             |   1 -
 appkit/src/window/handle.rs         |  20 +---
 appkit/src/window/window.rs         |   7 +-
 9 files changed, 133 insertions(+), 96 deletions(-)
 create mode 100644 appkit/src/toolbar/traits.rs

diff --git a/appkit/src/lib.rs b/appkit/src/lib.rs
index d6f35b4..bb88de8 100644
--- a/appkit/src/lib.rs
+++ b/appkit/src/lib.rs
@@ -49,7 +49,7 @@ pub mod prelude {
     
     pub use crate::menu::{Menu, MenuItem};
     pub use crate::notifications::{Notification, NotificationCenter, NotificationAuthOption};
-    pub use crate::toolbar::{ToolbarDelegate};
+    pub use crate::toolbar::{Toolbar, ToolbarController};
 
     pub use crate::networking::URLRequest;
 
diff --git a/appkit/src/pasteboard/pasteboard.rs b/appkit/src/pasteboard/pasteboard.rs
index 7e5f315..0ae054b 100644
--- a/appkit/src/pasteboard/pasteboard.rs
+++ b/appkit/src/pasteboard/pasteboard.rs
@@ -12,7 +12,7 @@ use objc_id::Id;
 use url::Url;
 
 use crate::error::AppKitError;
-use crate::pasteboard::types::{PasteboardName, PasteboardType};
+use crate::pasteboard::types::{PasteboardName};
 use crate::utils::str_from;
 
 /// Represents an `NSPasteboard`, enabling you to handle copy/paste/drag and drop.
diff --git a/appkit/src/toolbar/item.rs b/appkit/src/toolbar/item.rs
index 305f1ca..4150a80 100644
--- a/appkit/src/toolbar/item.rs
+++ b/appkit/src/toolbar/item.rs
@@ -14,21 +14,23 @@ use crate::button::Button;
 
 /// A wrapper for `NSWindow`. Holds (retains) pointers for the Objective-C runtime 
 /// where our `NSWindow` and associated delegate live.
-pub struct ToolbarItem<'a> {
-    pub identifier: &'a str,
+pub struct ToolbarItem {
+    pub identifier: String,
     pub inner: Id<Object>,
     pub button: Option<Button>
 }
 
-impl<'a> ToolbarItem<'a> {
+impl ToolbarItem {
     /// Creates a new `NSWindow` instance, configures it appropriately (e.g, titlebar appearance),
     /// injects an `NSObject` delegate wrapper, and retains the necessary Objective-C runtime
     /// pointers.
-    pub fn new(identifier: &'a str) -> Self {
+    pub fn new<S: Into<String>>(identifier: S) -> Self {
+        let identifier = identifier.into();
+
         let inner = unsafe {
-            let identifier = NSString::alloc(nil).init_str(identifier);
+            let identifr = NSString::alloc(nil).init_str(&identifier);
             let alloc: id = msg_send![class!(NSToolbarItem), alloc];
-            let item: id = msg_send![alloc, initWithItemIdentifier:identifier];
+            let item: id = msg_send![alloc, initWithItemIdentifier:identifr];
             Id::from_ptr(item)
         };
 
@@ -39,6 +41,7 @@ impl<'a> ToolbarItem<'a> {
         }
     }
 
+    /// Sets the title for this item.
     pub fn set_title(&mut self, title: &str) {
         unsafe {
             let title = NSString::alloc(nil).init_str(title);
@@ -46,6 +49,7 @@ impl<'a> ToolbarItem<'a> {
         }
     }
 
+    /// Sets and takes ownership of the button for this item.
     pub fn set_button(&mut self, button: Button) {
         button.set_bezel_style(11);
 
@@ -56,6 +60,7 @@ impl<'a> ToolbarItem<'a> {
         self.button = Some(button);
     }
 
+    /// Sets the minimum size for this button.
     pub fn set_min_size(&mut self, width: f64, height: f64) {
         unsafe {
             let size = NSSize::new(width.into(), height.into());
@@ -63,6 +68,7 @@ impl<'a> ToolbarItem<'a> {
         }
     }
 
+    /// Sets the maximum size for this button.
     pub fn set_max_size(&mut self, width: f64, height: f64) {
         unsafe {
             let size = NSSize::new(width.into(), height.into());
diff --git a/appkit/src/toolbar/mod.rs b/appkit/src/toolbar/mod.rs
index bd326a2..e09d744 100644
--- a/appkit/src/toolbar/mod.rs
+++ b/appkit/src/toolbar/mod.rs
@@ -1,7 +1,10 @@
 //! Module hoisting.
 
-pub mod toolbar;
-pub use toolbar::{Toolbar, ToolbarDelegate};
-
 pub mod item;
 pub use item::ToolbarItem;
+
+pub mod traits;
+pub use traits::ToolbarController;
+
+pub mod toolbar;
+pub use toolbar::Toolbar;
diff --git a/appkit/src/toolbar/toolbar.rs b/appkit/src/toolbar/toolbar.rs
index 33b19f7..5de843d 100644
--- a/appkit/src/toolbar/toolbar.rs
+++ b/appkit/src/toolbar/toolbar.rs
@@ -3,127 +3,144 @@
 //!
 //! UNFORTUNATELY, this is a very old and janky API. So... yeah.
 
+use std::cell::RefCell;
+use std::rc::Rc;
 use std::sync::Once;
 
 use cocoa::base::{id, nil};
 use cocoa::foundation::{NSArray, NSString};
 
-use objc_id::Id;
+use objc_id::ShareId;
 use objc::declare::ClassDecl;
 use objc::runtime::{Class, Object, Sel};
 use objc::{class, msg_send, sel, sel_impl};
 
 use crate::constants::TOOLBAR_PTR;
-use crate::toolbar::item::ToolbarItem;
-use crate::utils::str_from;
-
-/// A trait that you can implement to have your struct/etc act as an `NSToolbarDelegate`.
-pub trait ToolbarDelegate {
-    /// What items are allowed in this toolbar.
-    fn allowed_item_identifiers(&self) -> Vec<&'static str>;
-
-    /// The default items in this toolbar.
-    fn default_item_identifiers(&self) -> Vec<&'static str>;
-
-    /// For a given `identifier`, return the `ToolbarItem` that should be displayed.
-    fn item_for(&self, _identifier: &str) -> ToolbarItem;
-}
+use crate::toolbar::traits::ToolbarController;
+use crate::utils::{load, str_from};
 
 /// A wrapper for `NSToolbar`. Holds (retains) pointers for the Objective-C runtime 
 /// where our `NSToolbar` and associated delegate live.
-pub struct Toolbar {
-    pub inner: Id<Object>,
-    pub objc_delegate: Id<Object>,
-    pub delegate: Box<dyn ToolbarDelegate>
+pub struct Toolbar<T> {
+    internal_callback_ptr: *const RefCell<T>,
+    pub identifier: String,
+    pub objc_controller: ShareId<Object>,
+    pub controller: Rc<RefCell<T>>
 }
 
-impl Toolbar {
+impl<T> Toolbar<T> where T: ToolbarController + 'static {
     /// Creates a new `NSToolbar` instance, configures it appropriately, injects an `NSObject`
     /// delegate wrapper, and retains the necessary Objective-C runtime pointers.
-    pub fn new<D: ToolbarDelegate + 'static>(identifier: &str, delegate: D) -> Self {
-        let inner = unsafe {
-            let identifier = NSString::alloc(nil).init_str(identifier);
-            let alloc: id = msg_send![class!(NSToolbar), alloc];
-            let toolbar: id = msg_send![alloc, initWithIdentifier:identifier];
-            Id::from_ptr(toolbar)
+    pub fn new<S: Into<String>>(identifier: S, controller: T) -> Self {
+        let identifier = identifier.into();
+        let controller = Rc::new(RefCell::new(controller));
+        
+        let internal_callback_ptr = {
+            let cloned = Rc::clone(&controller);
+            Rc::into_raw(cloned)
         };
 
-        let toolbar_delegate = Box::new(delegate);
+        let objc_controller = unsafe {
+            let delegate_class = register_delegate_class::<T>();
+            let identifier = NSString::alloc(nil).init_str(&identifier);
+            let alloc: id = msg_send![delegate_class, alloc];
+            let toolbar: id = msg_send![alloc, initWithIdentifier:identifier];
 
-        let objc_delegate = unsafe {
-            let delegate_class = register_delegate_class::<D>();
-            let objc_delegate: id = msg_send![delegate_class, new];
-            let delegate_ptr: *const D = &*toolbar_delegate;
-            (&mut *objc_delegate).set_ivar(TOOLBAR_PTR, delegate_ptr as usize);
-            let _: () = msg_send![&*inner, setDelegate:objc_delegate];
-            Id::from_ptr(objc_delegate)
+            (&mut *toolbar).set_ivar(TOOLBAR_PTR, internal_callback_ptr as usize);
+            let _: () = msg_send![toolbar, setDelegate:toolbar];
+
+            ShareId::from_ptr(toolbar)
         };
 
         Toolbar {
-            inner: inner,
-            objc_delegate: objc_delegate,
-            delegate: toolbar_delegate
+            internal_callback_ptr: internal_callback_ptr,
+            identifier: identifier,
+            objc_controller: objc_controller,
+            controller: controller
+        }
+    }
+}
+
+impl<T> Drop for Toolbar<T> {
+    /// A bit of extra cleanup for delegate callback pointers.
+    fn drop(&mut self) {
+        unsafe {
+            let _ = Rc::from_raw(self.internal_callback_ptr);
         }
     }
 }
 
 /// Loops back to the delegate.
-extern fn allowed_item_identifiers<D: ToolbarDelegate>(this: &Object, _: Sel, _: id) -> id {
-    unsafe {
-        let ptr: usize = *this.get_ivar(TOOLBAR_PTR);
-        let toolbar = ptr as *mut D;
-        let identifiers = (*toolbar).allowed_item_identifiers().iter().map(|identifier| {
-            NSString::alloc(nil).init_str(identifier)
-        }).collect::<Vec<id>>();
+extern fn allowed_item_identifiers<T: ToolbarController>(this: &Object, _: Sel, _: id) -> id {
+    let toolbar = load::<T>(this, TOOLBAR_PTR);
 
+    unsafe {
+        let identifiers = {
+            let t = toolbar.borrow();
+
+            (*t).allowed_item_identifiers().iter().map(|identifier| {
+                NSString::alloc(nil).init_str(identifier)
+            }).collect::<Vec<id>>()
+        };
+
+        Rc::into_raw(toolbar);
         NSArray::arrayWithObjects(nil, &identifiers)
     }
 }
 
 /// Loops back to the delegate.
-extern fn default_item_identifiers<D: ToolbarDelegate>(this: &Object, _: Sel, _: id) -> id {
-    unsafe {
-        let ptr: usize = *this.get_ivar(TOOLBAR_PTR);
-        let toolbar = ptr as *mut D;
-        let identifiers = (*toolbar).default_item_identifiers().iter().map(|identifier| {
-            NSString::alloc(nil).init_str(identifier)
-        }).collect::<Vec<id>>();
+extern fn default_item_identifiers<T: ToolbarController>(this: &Object, _: Sel, _: id) -> id {
+    let toolbar = load::<T>(this, TOOLBAR_PTR);
 
+    unsafe {
+        let identifiers = {
+            let t = toolbar.borrow();
+            
+            (*t).default_item_identifiers().iter().map(|identifier| {
+                NSString::alloc(nil).init_str(identifier)
+            }).collect::<Vec<id>>()
+        };
+
+        Rc::into_raw(toolbar);
         NSArray::arrayWithObjects(nil, &identifiers)
     }
 }
 
 /// Loops back to the delegate.
-extern fn item_for_identifier<D: ToolbarDelegate>(this: &Object, _: Sel, _: id, identifier: id, _: id) -> id {
-    unsafe {
-        let ptr: usize = *this.get_ivar(TOOLBAR_PTR);
-        let toolbar = ptr as *mut D;
-        let identifier = str_from(identifier);
-        let mut item = (*toolbar).item_for(identifier);
-        &mut *item.inner
-    }
+extern fn item_for_identifier<T: ToolbarController>(this: &Object, _: Sel, _: id, identifier: id, _: id) -> id {
+    let toolbar = load::<T>(this, TOOLBAR_PTR);
+    let identifier = str_from(identifier);
+    
+    let mut item = {
+        let t = toolbar.borrow();
+        let item = (*t).item_for(identifier);
+        item
+    };
+
+    Rc::into_raw(toolbar);
+    &mut *item.inner
 }
 
 /// Registers an `NSObject` subclass, and configures it to hold some ivars for various things we need
 /// to store.
-fn register_delegate_class<D: ToolbarDelegate>() -> *const Class {
-    static mut TOOLBAR_DELEGATE_CLASS: *const Class = 0 as *const Class;
+fn register_delegate_class<T: ToolbarController>() -> *const Class {
+    static mut TOOLBAR_CLASS: *const Class = 0 as *const Class;
     static INIT: Once = Once::new();
 
     INIT.call_once(|| unsafe {
-        let superclass = class!(NSObject);
-        let mut decl = ClassDecl::new("RSTToolbarDelegate", superclass).unwrap();
+        let superclass = class!(NSToolbar);
+        let mut decl = ClassDecl::new("RSTToolbar", superclass).unwrap();
         
         // For callbacks
         decl.add_ivar::<usize>(TOOLBAR_PTR);
 
         // Add callback methods
-        decl.add_method(sel!(toolbarAllowedItemIdentifiers:), allowed_item_identifiers::<D> as extern fn(&Object, _, _) -> id);
-        decl.add_method(sel!(toolbarDefaultItemIdentifiers:), default_item_identifiers::<D> as extern fn(&Object, _, _) -> id);
-        decl.add_method(sel!(toolbar:itemForItemIdentifier:willBeInsertedIntoToolbar:), item_for_identifier::<D> as extern fn(&Object, _, _, _, _) -> id);
+        decl.add_method(sel!(toolbarAllowedItemIdentifiers:), allowed_item_identifiers::<T> as extern fn(&Object, _, _) -> id);
+        decl.add_method(sel!(toolbarDefaultItemIdentifiers:), default_item_identifiers::<T> as extern fn(&Object, _, _) -> id);
+        decl.add_method(sel!(toolbar:itemForItemIdentifier:willBeInsertedIntoToolbar:), item_for_identifier::<T> as extern fn(&Object, _, _, _, _) -> id);
 
-        TOOLBAR_DELEGATE_CLASS = decl.register();
+        TOOLBAR_CLASS = decl.register();
     });
 
-    unsafe { TOOLBAR_DELEGATE_CLASS }
+    unsafe { TOOLBAR_CLASS }
 }
diff --git a/appkit/src/toolbar/traits.rs b/appkit/src/toolbar/traits.rs
new file mode 100644
index 0000000..7f56076
--- /dev/null
+++ b/appkit/src/toolbar/traits.rs
@@ -0,0 +1,17 @@
+//! Traits that can be used for Toolbar construction. Relatively straightforward, as far as these
+//! go. Currently a bit incomplete in that we don't support the customizing workflow, but feel free
+//! to pull request it.
+
+use crate::toolbar::item::ToolbarItem;
+
+/// A trait that you can implement to have your struct/etc act as an `NSToolbarDelegate`.
+pub trait ToolbarController {
+    /// What items are allowed in this toolbar.
+    fn allowed_item_identifiers(&self) -> Vec<&'static str>;
+
+    /// The default items in this toolbar.
+    fn default_item_identifiers(&self) -> Vec<&'static str>;
+
+    /// For a given `identifier`, return the `ToolbarItem` that should be displayed.
+    fn item_for(&self, _identifier: &str) -> ToolbarItem;
+}
diff --git a/appkit/src/view/view.rs b/appkit/src/view/view.rs
index eaa6030..f6c0b75 100644
--- a/appkit/src/view/view.rs
+++ b/appkit/src/view/view.rs
@@ -1,7 +1,6 @@
 //! A wrapper for `NSViewController`. Uses interior mutability to 
 
 use std::cell::RefCell;
-use std::ops::Deref;
 use std::rc::Rc;
 
 use cocoa::base::{id, nil, YES};
diff --git a/appkit/src/window/handle.rs b/appkit/src/window/handle.rs
index c0317aa..66cd570 100644
--- a/appkit/src/window/handle.rs
+++ b/appkit/src/window/handle.rs
@@ -13,8 +13,7 @@ use objc_id::ShareId;
 
 use crate::view::traits::Node;
 
-//use crate::toolbar::traits::ToolbarDelegate;
-
+use crate::toolbar::{Toolbar, ToolbarController};
 
 #[derive(Debug, Default, Clone)]
 pub struct WindowHandle(pub Option<ShareId<Object>>);
@@ -90,24 +89,15 @@ impl WindowHandle {
         }
     }
 
-    /// Used for setting a toolbar on this window. Note that this takes ownership of whatever
-    /// `ToolbarDelegate` you pass! The underlying `NSToolbar` is a bit... old, and it's just
-    /// easier to do things this way.
-    ///
-    /// If you find yourself in a position where you need your toolbar after the fact, you 
-    /// probably have bigger issues.
-    //pub fn set_toolbar<T: ToolbarDelegate + 'static>(&mut self, identifier: &str, toolbar: T) {
-        /*let toolbar = Toolbar::new(identifier, toolbar);
-        
+    /// Used for setting a toolbar on this window. 
+    pub fn set_toolbar<TB: ToolbarController>(&self, toolbar: &Toolbar<TB>) {
         if let Some(controller) = &self.0 {
             unsafe {
                 let window: id = msg_send![*controller, window];
-                let _: () = msg_send![window, setToolbar:&*toolbar.inner];
+                let _: () = msg_send![window, setToolbar:&*toolbar.objc_controller];
             }
         }
-
-        self.toolbar = Some(toolbar);*/
-    //}
+    }
 
     /// Used for setting the content view controller for this window.
     pub fn set_content_view_controller<T: Node + 'static>(&self, view_controller: &T) {
diff --git a/appkit/src/window/window.rs b/appkit/src/window/window.rs
index b6bd341..c605483 100644
--- a/appkit/src/window/window.rs
+++ b/appkit/src/window/window.rs
@@ -10,7 +10,7 @@ use objc::{msg_send, sel, sel_impl};
 use objc_id::ShareId;
 
 use crate::constants::WINDOW_CONTROLLER_PTR;
-use crate::toolbar::{Toolbar, ToolbarDelegate};
+use crate::toolbar::{Toolbar, ToolbarController};
 use crate::view::traits::Node;
 use crate::window::handle::WindowHandle;
 use crate::window::traits::WindowController;
@@ -92,6 +92,11 @@ impl<T> Window<T> where T: WindowController + 'static {
         self.objc_controller.set_title(title.into());
     }
 
+    /// Sets the toolbar for this window.
+    pub fn set_toolbar<TB: ToolbarController>(&self, toolbar: &Toolbar<TB>) {
+        self.objc_controller.set_toolbar(toolbar);
+    }
+
     /// Sets the content view controller for the window.
     pub fn set_content_view_controller<VC: Node + 'static>(&self, view_controller: &VC) {
         self.objc_controller.set_content_view_controller(view_controller);