From 4766f41e55caf931004716464f790260893752dd Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 23 Apr 2024 15:42:50 +0200 Subject: [PATCH] Use objc2 and its framework crates `objc2` is a replacement for `objc`/`objc_id` that contains a bunch of safety improvements, including `msg_send_id!` which automatically upholds memory management rules (`Id::from_ptr`/`Id::from_retained_ptr` is no longer necessary). Additionally, we use the framework crates `objc2-foundation` and `objc2-app-kit`, which provide for example the `NSPasteboard` type, which has the methods that arboard needs already defined, and with the correct types, ensuring that passing e.g. `Id` and thus accidentally giving away ownership over the array won't happen again. These crates are automatically generated, ensuring that if you need some obscure API in the future, it's very likely to be there already. --- Cargo.lock | 78 ++++++++++++-------- Cargo.toml | 7 +- src/platform/osx.rs | 170 +++++++++++++++++++------------------------- 3 files changed, 128 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb49fd9..94d3091 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,9 +26,9 @@ dependencies = [ "env_logger", "image", "log", - "objc", - "objc-foundation", - "objc_id", + "objc2", + "objc2-app-kit", + "objc2-foundation", "parking_lot", "thiserror", "windows-sys", @@ -66,10 +66,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4682ae6287fcf752ecaabbfcc7b6f9b72aa33933dc23a554d853aea8eea8635" [[package]] -name = "block" -version = "0.1.6" +name = "block2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d8c1fef690941d3e7788d328517591fecc684c084084702d6ff1641e993699a" +checksum = "43ff7d91d3c1d568065b06c899777d1e48dcf76103a672a0adbc238a7f247f1e" +dependencies = [ + "objc2", +] [[package]] name = "bytecount" @@ -384,15 +387,6 @@ version = "0.4.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" -[[package]] -name = "malloc_buf" -version = "0.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62bb907fe88d54d8d9ce32a3cceab4218ed2f6b7d35617cafe9adf84e43919cb" -dependencies = [ - "libc", -] - [[package]] name = "memchr" version = "2.5.0" @@ -456,32 +450,58 @@ dependencies = [ ] [[package]] -name = "objc" -version = "0.2.7" +name = "objc-sys" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da284c198fb9b7b0603f8635185e85fbd5b64ee154b1ed406d489077de2d6d60" + +[[package]] +name = "objc2" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "915b1b472bc21c53464d6c8461c9d3af805ba1ef837e1cac254428f4a77177b1" +checksum = "b4b25e1034d0e636cd84707ccdaa9f81243d399196b8a773946dcffec0401659" dependencies = [ - "malloc_buf", + "objc-sys", + "objc2-encode", ] [[package]] -name = "objc-foundation" -version = "0.1.1" +name = "objc2-app-kit" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1add1b659e36c9607c7aab864a76c7a4c2760cd0cd2e120f3fb8b952c7e22bf9" +checksum = "fb79768a710a9a1798848179edb186d1af7e8a8679f369e4b8d201dd2a034047" dependencies = [ - "block", - "objc", - "objc_id", + "block2", + "objc2", + "objc2-core-data", + "objc2-foundation", ] [[package]] -name = "objc_id" -version = "0.1.1" +name = "objc2-core-data" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e092bc42eaf30a08844e6a076938c60751225ec81431ab89f5d1ccd9f958d6c" +dependencies = [ + "block2", + "objc2", + "objc2-foundation", +] + +[[package]] +name = "objc2-encode" +version = "4.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88658da63e4cc2c8adb1262902cd6af51094df0488b760d6fd27194269c0950a" + +[[package]] +name = "objc2-foundation" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c92d4ddb4bd7b50d730c215ff871754d0da6b2178849f8a2a2ab69712d0c073b" +checksum = "cfaefe14254871ea16c7d88968c0ff14ba554712a20d76421eec52f0a7fb8904" dependencies = [ - "objc", + "block2", + "objc2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 7443610..6ab3516 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,9 +33,10 @@ clipboard-win = "5.3.1" log = "0.4" [target.'cfg(target_os = "macos")'.dependencies] -objc = "0.2" -objc_id = "0.1" -objc-foundation = "0.1" +# Use `relax-void-encoding`, as that allows us to pass `c_void` instead of implementing `Encode` correctly for `&CGImageRef` +objc2 = { version = "0.5.1", features = ["relax-void-encoding"] } +objc2-foundation = { version = "0.2.0", features = ["NSArray", "NSString", "NSEnumerator", "NSGeometry"] } +objc2-app-kit = { version = "0.2.0", features = ["NSPasteboard", "NSPasteboardItem", "NSImage"] } core-graphics = { version = "0.23", optional = true } image = { version = "0.25", optional = true, default-features = false, features = ["tiff"] } diff --git a/src/platform/osx.rs b/src/platform/osx.rs index c48df73..ac4dc79 100644 --- a/src/platform/osx.rs +++ b/src/platform/osx.rs @@ -11,31 +11,18 @@ and conditions of the chosen license apply to this file. use crate::common::Error; #[cfg(feature = "image-data")] use crate::common::ImageData; -#[cfg(feature = "image-data")] -use core_graphics::{ - base::{kCGBitmapByteOrderDefault, kCGImageAlphaLast, kCGRenderingIntentDefault, CGFloat}, - color_space::CGColorSpace, - data_provider::{CGDataProvider, CustomData}, - image::CGImage, +use objc2::{ + msg_send_id, + rc::{autoreleasepool, Id}, + runtime::ProtocolObject, + ClassType, }; -use objc::{ - msg_send, - rc::autoreleasepool, - runtime::{Class, Object}, - sel, sel_impl, +use objc2_app_kit::{NSPasteboard, NSPasteboardTypeHTML, NSPasteboardTypeString}; +use objc2_foundation::{NSArray, NSString}; +use std::{ + borrow::Cow, + panic::{RefUnwindSafe, UnwindSafe}, }; -use objc_foundation::{INSArray, INSFastEnumeration, INSString, NSArray, NSObject, NSString}; -use objc_id::{Id, Owned}; -use std::{borrow::Cow, ptr::NonNull}; - -// Required to bring NSPasteboard into the path of the class-resolver -#[link(name = "AppKit", kind = "framework")] -extern "C" { - static NSPasteboardTypeHTML: *const Object; - static NSPasteboardTypeString: *const Object; - #[cfg(feature = "image-data")] - static NSPasteboardTypeTIFF: *const Object; -} /// Returns an NSImage object on success. #[cfg(feature = "image-data")] @@ -43,13 +30,16 @@ fn image_from_pixels( pixels: Vec, width: usize, height: usize, -) -> Result, Box> { - #[repr(C)] - #[derive(Copy, Clone)] - struct NSSize { - width: CGFloat, - height: CGFloat, - } +) -> Result, Box> { + use core_graphics::{ + base::{kCGBitmapByteOrderDefault, kCGImageAlphaLast, kCGRenderingIntentDefault, CGFloat}, + color_space::CGColorSpace, + data_provider::{CGDataProvider, CustomData}, + image::{CGImage, CGImageRef}, + }; + use objc2_app_kit::NSImage; + use objc2_foundation::NSSize; + use std::ffi::c_void; #[derive(Debug)] struct PixelArray { @@ -81,42 +71,54 @@ fn image_from_pixels( false, kCGRenderingIntentDefault, ); + + // Convert the owned `CGImage` into a reference `&CGImageRef`, and pass + // that as `*const c_void`, since `CGImageRef` does not implement + // `RefEncode`. + let cg_image: *const CGImageRef = &*cg_image; + let cg_image: *const c_void = cg_image.cast(); + let size = NSSize { width: width as CGFloat, height: height as CGFloat }; - let nsimage_class = objc::class!(NSImage); - // Take ownership of the newly allocated object, which has an existing retain count. - let image: Id = unsafe { Id::from_retained_ptr(msg_send![nsimage_class, alloc]) }; - #[allow(clippy::let_unit_value)] - { - // Note: `initWithCGImage` expects a reference (`CGImageRef`), not an actual object. - let _: () = unsafe { msg_send![image, initWithCGImage: &*cg_image size:size] }; - } + // XXX: Use `NSImage::initWithCGImage_size` once `objc2-app-kit` supports + // CoreGraphics. + let image: Id = + unsafe { msg_send_id![NSImage::alloc(), initWithCGImage: cg_image, size:size] }; Ok(image) } pub(crate) struct Clipboard { - pasteboard: Id, + pasteboard: Id, } +unsafe impl Send for Clipboard {} +unsafe impl Sync for Clipboard {} +impl UnwindSafe for Clipboard {} +impl RefUnwindSafe for Clipboard {} + impl Clipboard { pub(crate) fn new() -> Result { - let cls = Class::get("NSPasteboard").expect("NSPasteboard not registered"); - let pasteboard: *mut Object = unsafe { msg_send![cls, generalPasteboard] }; + // Rust only supports 10.7+, while `generalPasteboard` first appeared + // in 10.0, so this should always be available. + // + // However, in some edge cases, like running under launchd (in some + // modes) as a daemon, the clipboard object may be unavailable, and + // then `generalPasteboard` will return NULL even though it's + // documented not to. + // + // Otherwise we'd just use `NSPasteboard::generalPasteboard()` here. + let pasteboard: Option> = + unsafe { msg_send_id![NSPasteboard::class(), generalPasteboard] }; - if !pasteboard.is_null() { - // SAFETY: `generalPasteboard` is not null and a valid object pointer. - let pasteboard: Id = unsafe { Id::from_ptr(pasteboard) }; + if let Some(pasteboard) = pasteboard { Ok(Clipboard { pasteboard }) } else { - // Rust only supports 10.7+, while `generalPasteboard` first appeared in 10.0, so this - // is unreachable in "normal apps". However in some edge cases, like running under - // launchd (in some modes) as a daemon, the clipboard object may be unavailable. Err(Error::ClipboardNotSupported) } } fn clear(&mut self) { - let _: usize = unsafe { msg_send![self.pasteboard, clearContents] }; + unsafe { self.pasteboard.clearContents() }; } // fn get_binary_contents(&mut self) -> Result, Box> { @@ -171,38 +173,31 @@ impl Clipboard { } pub(crate) struct Get<'clipboard> { - pasteboard: &'clipboard Object, + clipboard: &'clipboard Clipboard, } impl<'clipboard> Get<'clipboard> { pub(crate) fn new(clipboard: &'clipboard mut Clipboard) -> Self { - Self { pasteboard: &*clipboard.pasteboard } + Self { clipboard } } pub(crate) fn text(self) -> Result { // XXX: There does not appear to be an alternative for obtaining text without the need for // autorelease behavior. - autoreleasepool(|| { + autoreleasepool(|_| { // XXX: We explicitly use `pasteboardItems` and not `stringForType` since the latter will concat // multiple strings, if present, into one and return it instead of reading just the first which is `arboard`'s // historical behavior. - let contents: Option>> = - unsafe { msg_send![self.pasteboard, pasteboardItems] }; - - let contents = contents.map(|c| unsafe { c.as_ref() }).ok_or_else(|| { - Error::Unknown { description: String::from("NSPasteboard#pasteboardItems errored") } - })?; - - for item in contents.enumerator() { - let maybe_str: Option> = - unsafe { msg_send![item, stringForType:NSPasteboardTypeString] }; - - match maybe_str { - Some(string) => { - let string: Id = unsafe { Id::from_ptr(string.as_ptr()) }; - return Ok(string.as_str().to_owned()); + let contents = + unsafe { self.clipboard.pasteboard.pasteboardItems() }.ok_or_else(|| { + Error::Unknown { + description: String::from("NSPasteboard#pasteboardItems errored"), } - None => continue, + })?; + + for item in contents { + if let Some(string) = unsafe { item.stringForType(NSPasteboardTypeString) } { + return Ok(string.to_string()); } } @@ -212,27 +207,16 @@ impl<'clipboard> Get<'clipboard> { #[cfg(feature = "image-data")] pub(crate) fn image(self) -> Result, Error> { - use objc_foundation::NSData; + use objc2_app_kit::NSPasteboardTypeTIFF; use std::io::Cursor; // XXX: There does not appear to be an alternative for obtaining images without the need for // autorelease behavior. - let image = autoreleasepool(|| { - let obj: Option> = - unsafe { msg_send![self.pasteboard, dataForType: NSPasteboardTypeTIFF] }; - - let image_data: Id = if let Some(obj) = obj { - unsafe { Id::from_ptr(obj.as_ptr()) } - } else { - return Err(Error::ContentNotAvailable); - }; - - let data = unsafe { - let len: usize = msg_send![&*image_data, length]; - let bytes: *const u8 = msg_send![&*image_data, bytes]; + let image = autoreleasepool(|_| { + let image_data = unsafe { self.clipboard.pasteboard.dataForType(NSPasteboardTypeTIFF) } + .ok_or(Error::ContentNotAvailable)?; - Cursor::new(std::slice::from_raw_parts(bytes, len)) - }; + let data = Cursor::new(image_data.bytes()); let reader = image::io::Reader::with_format(data, image::ImageFormat::Tiff); reader.decode().map_err(|_| Error::ConversionFailure) @@ -261,11 +245,9 @@ impl<'clipboard> Set<'clipboard> { pub(crate) fn text(self, data: Cow<'_, str>) -> Result<(), Error> { self.clipboard.clear(); - let string_array = NSArray::from_vec(vec![NSString::from_str(&data)]); - // Make sure that we pass a pointer to the system and not the array object itself. Otherwise, - // the system won't free it because the API doesn't give it ownership of the data. This results in - // a memory leak because Rust can never run its destructor. - let success = unsafe { msg_send![self.clipboard.pasteboard, writeObjects: &*string_array] }; + let string_array = + NSArray::from_vec(vec![ProtocolObject::from_id(NSString::from_str(&data))]); + let success = unsafe { self.clipboard.pasteboard.writeObjects(&string_array) }; if success { Ok(()) } else { @@ -286,15 +268,14 @@ impl<'clipboard> Set<'clipboard> { ); let html_nss = NSString::from_str(&html); // Make sure that we pass a pointer to the string and not the object itself. - let mut success: bool = unsafe { - msg_send![self.clipboard.pasteboard, setString: &*html_nss forType:NSPasteboardTypeHTML] - }; + let mut success = + unsafe { self.clipboard.pasteboard.setString_forType(&html_nss, NSPasteboardTypeHTML) }; if success { if let Some(alt_text) = alt { let alt_nss = NSString::from_str(&alt_text); // Similar to the primary string, we only want a pointer here too. success = unsafe { - msg_send![self.clipboard.pasteboard, setString: &*alt_nss forType:NSPasteboardTypeString] + self.clipboard.pasteboard.setString_forType(&alt_nss, NSPasteboardTypeString) }; } } @@ -313,9 +294,8 @@ impl<'clipboard> Set<'clipboard> { self.clipboard.clear(); - let image_array: Id> = NSArray::from_vec(vec![image]); - // Make sure that we pass a pointer to the system and not the array object itself. - let success = unsafe { msg_send![self.clipboard.pasteboard, writeObjects: &*image_array] }; + let image_array = NSArray::from_vec(vec![ProtocolObject::from_id(image)]); + let success = unsafe { self.clipboard.pasteboard.writeObjects(&image_array) }; if success { Ok(()) } else {