Add Send + Sync for SubclassingAdapter, remove visibility panic#1
Open
jshin-tacticalid wants to merge 3 commits into
Open
Add Send + Sync for SubclassingAdapter, remove visibility panic#1jshin-tacticalid wants to merge 3 commits into
jshin-tacticalid wants to merge 3 commits into
Conversation
iced's Proxy<T> requires Send + Sync for its Notifier trait because it wraps the adapter in a channel. The SubclassingAdapter uses RefCell internally, which is not Send/Sync. This is safe because the SubclassingAdapter is always created and accessed on the main thread. The Send + Sync bounds come from iced's channel infrastructure, not actual cross-thread usage. Accessibility callbacks (NSAccessibility) only fire on the window's thread.
…ity panic Two changes: 1. Add unsafe Send + Sync implementations for SubclassingAdapter, matching the macOS change. The adapter is always accessed on the UI thread; WM_GETOBJECT and focus callbacks only fire on the window's thread. 2. Remove the IsWindowVisible panic in SubclassingAdapter::new(). Some frameworks (notably iced) process adapter initialization after the window is already shown — this is normal lifecycle behavior, not an error. Window subclassing works fine on visible windows.
1. Remove the panic when the window is already visible at adapter creation time. Update doc comment from "# Panics" to "# Note" explaining that pre-visibility creation is preferred but not required. 2. Change Windows platform adapter to accept &dyn Window instead of &Window for consistency with the trait-based API.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When using accesskit with iced's event loop, two issues prevent compilation and correct initialization:
SubclassingAdapter is not Send/Sync — iced's
Proxy<T>wraps the adapter in a channel, which requiresSend + Sync. The adapter usesRefCellinternally, but it is always created and accessed on the UI thread (main thread on macOS, window thread on Windows). The channel bounds are structural, not behavioral.Visibility panic on Windows —
SubclassingAdapter::new()andAdapter::with_direct_handlers()panic if the window is already visible. Some frameworks (iced, and likely others) show the window before processing adapter initialization. Window subclassing works fine on visible windows — the panic is overly strict.Changes
accesskit_macos/src/subclass.rs:unsafe impl Send + Sync for SubclassingAdapteraccesskit_windows/src/subclass.rs:unsafe impl Send + Sync for SubclassingAdapter, removeIsWindowVisiblecheckaccesskit_winit/src/lib.rs: Remove visibility panic, update doc from "# Panics" to "# Note"accesskit_winit/src/platform_impl/windows.rs: Accept&dyn Windowfor consistencySafety
The
Send + Syncimpls are safe because: