diff --git a/Cargo.lock b/Cargo.lock index 2d4bc7f45..a29d73b9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -96,6 +96,7 @@ dependencies = [ "accesskit_consumer", "hashbrown", "once_cell", + "parking_lot", "scopeguard", "static_assertions", "windows", @@ -945,7 +946,7 @@ checksum = "3af92c55d7d839293953fcd0fda5ecfe93297cfde6ffbdec13b41d99c0ba6607" dependencies = [ "bitflags 2.8.0", "libc", - "redox_syscall", + "redox_syscall 0.4.1", ] [[package]] @@ -960,6 +961,16 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fe7db12097d22ec582439daf8618b8fdd1a7bef6270e9af3b1ebcd30893cf413" +[[package]] +name = "lock_api" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96936507f153605bddfcda068dd804796c84324ed2510809e5b2a624c81da765" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.17" @@ -1346,6 +1357,29 @@ version = "2.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f38d5652c16fde515bb1ecef450ab0f6a219d619a7274976324d5e377f7dceba" +[[package]] +name = "parking_lot" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70d58bf43669b5795d1576d0641cfb6fbb2057bf629506267a92807158584a13" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc838d2a56b5b1a6c25f55575dfc605fabb63bb2365f6c2353ef9159aa69e4a5" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.5.13", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "percent-encoding" version = "2.2.0" @@ -1558,6 +1592,15 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "redox_syscall" +version = "0.5.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d04b7d0ee6b4a0207a0a7adb104d23ecb0b47d6beae7152d0fa34b692b29fd6" +dependencies = [ + "bitflags 2.8.0", +] + [[package]] name = "rustc-demangle" version = "0.1.23" @@ -2588,7 +2631,7 @@ dependencies = [ "pin-project", "raw-window-handle 0.5.2", "raw-window-handle 0.6.2", - "redox_syscall", + "redox_syscall 0.4.1", "rustix 0.38.44", "sctk-adwaita", "smithay-client-toolkit", diff --git a/platforms/macos/src/subclass.rs b/platforms/macos/src/subclass.rs index 0b9a2d533..9d5b194fb 100644 --- a/platforms/macos/src/subclass.rs +++ b/platforms/macos/src/subclass.rs @@ -146,6 +146,13 @@ impl SubclassingAdapter { action_handler: impl 'static + ActionHandler, ) -> Self { let view = Id::as_ptr(&retained_view) as *mut NSView; + if !unsafe { + objc_getAssociatedObject(view as *const NSView as *const _, associated_object_key()) + } + .is_null() + { + panic!("subclassing adapter already instantiated on view {view:?}"); + } let adapter = unsafe { Adapter::new(view as *mut c_void, false, action_handler) }; // Cast to a pointer and back to force the lifetime to 'static // SAFETY: We know the class will live as long as the instance, diff --git a/platforms/windows/Cargo.toml b/platforms/windows/Cargo.toml index d86717ca5..a9aa3c212 100644 --- a/platforms/windows/Cargo.toml +++ b/platforms/windows/Cargo.toml @@ -38,5 +38,6 @@ features = [ [dev-dependencies] once_cell = "1.13.0" +parking_lot = "0.12.4" scopeguard = "1.1.0" winit = "0.30" diff --git a/platforms/windows/src/subclass.rs b/platforms/windows/src/subclass.rs index b14c39347..0f93418ba 100644 --- a/platforms/windows/src/subclass.rs +++ b/platforms/windows/src/subclass.rs @@ -93,6 +93,12 @@ impl SubclassImpl { } fn install(&mut self) { + if !unsafe { GetPropW(self.hwnd, PROP_NAME) }.0.is_null() { + panic!( + "subclassing adapter already instantiated on window {:?}", + self.hwnd.0 + ); + } unsafe { SetPropW( self.hwnd, diff --git a/platforms/windows/src/tests/mod.rs b/platforms/windows/src/tests/mod.rs index 2ee2cbae4..17aed8661 100644 --- a/platforms/windows/src/tests/mod.rs +++ b/platforms/windows/src/tests/mod.rs @@ -180,7 +180,9 @@ impl Scope { } // It's not safe to run these UI-related tests concurrently. -pub(crate) static MUTEX: Mutex<()> = Mutex::new(()); +// We need a non-poisoning mutex here because the subclassing adapter's +// double-instantiation test intentionally panics. +pub(crate) static MUTEX: parking_lot::Mutex<()> = parking_lot::const_mutex(()); pub(crate) fn scope( window_title: &str, @@ -191,7 +193,7 @@ pub(crate) fn scope( where F: FnOnce(&Scope) -> Result<()>, { - let _lock_guard = MUTEX.lock().unwrap(); + let _lock_guard = MUTEX.lock(); let window_mutex: Mutex> = Mutex::new(None); let window_cv = Condvar::new(); diff --git a/platforms/windows/src/tests/subclassed.rs b/platforms/windows/src/tests/subclassed.rs index 255f6c9bc..b9eeb4c7d 100644 --- a/platforms/windows/src/tests/subclassed.rs +++ b/platforms/windows/src/tests/subclassed.rs @@ -6,7 +6,15 @@ use accesskit::{ Action, ActionHandler, ActionRequest, ActivationHandler, Node, NodeId, Role, Tree, TreeUpdate, }; -use windows::Win32::{Foundation::*, UI::Accessibility::*}; +use once_cell::sync::Lazy; +use windows::{ + core::*, + Win32::{ + Foundation::*, + System::LibraryLoader::GetModuleHandleW, + UI::{Accessibility::*, WindowsAndMessaging::*}, + }, +}; use winit::{ application::ApplicationHandler, event::WindowEvent, @@ -63,7 +71,14 @@ impl ActivationHandler for SimpleActivationHandler { } // This module uses winit for the purpose of testing with a real third-party -// window implementation that we don't control. +// window implementation that we don't control. However, only one test +// can use winit, because winit only allows an event loop to be created +// once per process. So we end up creating our own window anyway for the +// double-instantiation test. +// +// Also, while these tests don't use the main test harness or show the window, +// they still need to run with the main harness's mutex, to avoid disturbing +// other tests, particularly the focus test. struct TestApplication; @@ -92,12 +107,69 @@ impl ApplicationHandler<()> for TestApplication { #[test] fn has_native_uia() { - // This test is simple enough that we know it's fine to run entirely - // on one thread, so we don't need a full multithreaded test harness. - // Still, we must prevent this test from running concurrently with other - // tests, especially the focus test. - let _lock_guard = MUTEX.lock().unwrap(); + let _lock_guard = MUTEX.lock(); let event_loop = EventLoop::builder().with_any_thread(true).build().unwrap(); let mut state = TestApplication {}; event_loop.run_app(&mut state).unwrap(); } + +extern "system" fn wndproc(window: HWND, message: u32, wparam: WPARAM, lparam: LPARAM) -> LRESULT { + unsafe { DefWindowProcW(window, message, wparam, lparam) } +} + +static WINDOW_CLASS_ATOM: Lazy = Lazy::new(|| { + let class_name = w!("AccessKitSubclassTest"); + + let wc = WNDCLASSW { + hCursor: unsafe { LoadCursorW(None, IDC_ARROW) }.unwrap(), + hInstance: unsafe { GetModuleHandleW(None) }.unwrap().into(), + lpszClassName: class_name, + style: CS_HREDRAW | CS_VREDRAW, + lpfnWndProc: Some(wndproc), + ..Default::default() + }; + + let atom = unsafe { RegisterClassW(&wc) }; + if atom == 0 { + panic!("{}", Error::from_win32()); + } + atom +}); + +fn create_window(title: &str) -> HWND { + let module = HINSTANCE::from(unsafe { GetModuleHandleW(None).unwrap() }); + + let window = unsafe { + CreateWindowExW( + Default::default(), + PCWSTR(*WINDOW_CLASS_ATOM as usize as _), + &HSTRING::from(title), + WS_OVERLAPPEDWINDOW, + CW_USEDEFAULT, + CW_USEDEFAULT, + CW_USEDEFAULT, + CW_USEDEFAULT, + None, + None, + Some(module), + None, + ) + } + .unwrap(); + if window.is_invalid() { + panic!("{}", Error::from_win32()); + } + + window +} + +#[test] +#[should_panic(expected = "already instantiated")] +fn double_instantiate() { + let _lock_guard = MUTEX.lock(); + let window = create_window(WINDOW_TITLE); + let _adapter1 = + SubclassingAdapter::new(window, SimpleActivationHandler {}, NullActionHandler {}); + let _adapter2 = + SubclassingAdapter::new(window, SimpleActivationHandler {}, NullActionHandler {}); +}