Move AccessibilityElements dispose to the cleanUp call - #3410
Andrei Salavei (ASalavei) merged 4 commits into
Conversation
|
Hi Andrei Salavei (@ASalavei) — #3410 is right, and it matches what our analysis found too: Why disposing earlier can't stop the crash on its ownThe guard reads the flag in Kotlin, through the receiver: and UIKit reaches it through a Kotlin override, so the IMP that runs is the Kotlin bridge — the Objective-C method in In the crash, that receiver is null. The Kotlin/Native GC clears an element's back-reference once nothing retains the Objective-C object and Compose has dropped its Kotlin reference, but the Objective-C object itself is only released later, by the finalizer — That is the A reproducer with no Compose, no UIKit and no accessibility client shows exactly this combination: a Kotlin subclass of an Objective-C class, every element disposed before it is abandoned, an unretained registry that unregisters in The missing half: check the flag before entering KotlinEach UIKit-facing method becomes a front door implemented in Objective-C that checks // CMPAccessibilityElement.h
- (BOOL)isAccessibilityElement; // front door, not for overriding
CMP_CAN_OVERRIDE - (BOOL)cmp_isAccessibilityElement; // what Kotlin overrides
// CMPAccessibilityElement.m
#define CMP_FRONT_DOOR(returnType, selector, disposedValue) \
- (returnType)selector { \
if (_isDisposed) { \
return disposedValue; \
} \
return [self cmp_##selector]; \
}
CMP_FRONT_DOOR(BOOL, isAccessibilityElement, NO)
CMP_FRONT_DOOR(CGRect, bounds, CGRectZero)
CMP_FRONT_DOOR(CGPoint, contentOffset, CGPointZero)
- (BOOL)cmp_isAccessibilityElement {
return [super isAccessibilityElement];
}// Accessibility.ios.kt: the override moves to the hook; call sites keep using the UIKit names
override fun cmp_isAccessibilityElement(): Boolean = getIfAlive {
node.isAccessibilityElement
} ?: falseThat covers all 45 selectors
Measured on our appPatched
0/24 vs 4/12, Fisher two-tailed p = 0.0084. All four control crashes are backtrace-confirmed, and all 24 fix runs passed — that flow is driven entirely through accessibility identifiers, so the front doors are not muting the tree. We have not run #3410 alone on the app; the reproducer says it will still fault. The full change, on top of this PR's head ( Patch on top of 7d0d13d (3 files, +505 −85)diff --git a/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.h b/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.h
index a79e89e38..ba219d391 100644
--- a/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.h
+++ b/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.h
@@ -19,10 +19,37 @@
NS_ASSUME_NONNULL_BEGIN
+/**
+ * Base class of the accessibility elements Compose hands to UIKit.
+ *
+ * Lifetime contract
+ * -----------------
+ * The Kotlin subclass of this class and the Objective-C object are two halves of one element, and
+ * they do not die at the same moment. Kotlin/Native keeps the Kotlin peer alive only while the
+ * Objective-C object is retained by somebody; once the retain count reaches zero and Compose drops
+ * its own reference, the garbage collector clears the peer, while the Objective-C object survives
+ * until the finalizer releases it. Between those two moments the object is still a perfectly valid
+ * Objective-C object — and the accessibility runtime, which keeps unretained references to elements
+ * it has already seen and forgets them at -dealloc, can still message it.
+ *
+ * A message that lands in that window and reaches a method overridden in Kotlin enters Kotlin with
+ * a null receiver, and the first field read faults (CMP-10615). No Kotlin-side check can prevent
+ * that: evaluating the check would itself require the receiver.
+ *
+ * Therefore every UIKit-facing method below is a *front door*: it is implemented here, in
+ * Objective-C, it consults `isDisposed` — which is Objective-C state, readable without the Kotlin
+ * peer — and only then calls the `cmp_`-prefixed hook. Subclasses override the hooks, never the
+ * UIKit-facing methods. Compose must mark an element disposed while its Kotlin peer is still alive;
+ * after that point the element answers UIKit with inert values.
+ */
@interface CMPAccessibilityElement : UIAccessibilityElement <UIFocusItem>
+/// Set by Compose when the element leaves the accessibility tree, and by -dealloc. Once set, no
+/// message is dispatched to the Kotlin subclass any more.
@property (nonatomic, assign) BOOL isDisposed;
+#pragma mark - UIKit-facing methods (front doors, not for overriding)
+
- (NSArray<UIAccessibilityCustomAction *> *)accessibilityCustomActions;
- (UIAccessibilityTraits)accessibilityTraits;
@@ -43,6 +70,8 @@ NS_ASSUME_NONNULL_BEGIN
- (CGRect)accessibilityFrame;
+- (__nullable id)accessibilityContainer;
+
- (BOOL)isAccessibilityElement;
- (BOOL)accessibilityActivate;
@@ -71,6 +100,153 @@ NS_ASSUME_NONNULL_BEGIN
- (nullable id)accessibilityHitTest:(CGPoint)point withEvent:(nullable UIEvent *)event;
+// UIFocusItem / UIFocusEnvironment / UIFocusItemContainer / UIFocusItemScrollableContainer /
+// UICoordinateSpace. Declared here rather than left to the subclass so that the front door runs
+// before Kotlin: these are the methods UIKit calls while resolving an accessibility frame, and
+// `bounds`/`contentOffset` are where CMP-10615 was observed to fault.
+
+- (BOOL)canBecomeFocused;
+
+- (void)didUpdateFocusInContext:(UIFocusUpdateContext *)context
+ withAnimationCoordinator:(UIFocusAnimationCoordinator *)coordinator;
+
+- (__nullable id<UIFocusItemContainer>)focusItemContainer;
+
+- (CGRect)frame;
+
+- (CGRect)bounds;
+
+- (__nullable id<UIFocusEnvironment>)parentFocusEnvironment;
+
+- (NSArray *)preferredFocusEnvironments;
+
+- (void)setNeedsFocusUpdate;
+
+- (void)updateFocusIfNeeded;
+
+- (BOOL)shouldUpdateFocusInContext:(UIFocusUpdateContext *)context;
+
+- (id<UICoordinateSpace>)coordinateSpace;
+
+- (NSArray *)focusItemsInRect:(CGRect)rect;
+
+- (BOOL)isTransparentFocusItem;
+
+- (CGSize)visibleSize;
+
+- (CGSize)contentSize;
+
+- (CGPoint)contentOffset;
+
+- (void)setContentOffset:(CGPoint)contentOffset;
+
+- (CGPoint)convertPoint:(CGPoint)point toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+- (CGPoint)convertPoint:(CGPoint)point fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+- (CGRect)convertRect:(CGRect)rect toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+- (CGRect)convertRect:(CGRect)rect fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+#pragma mark - Hooks (override these)
+
+/**
+ * Each hook is called by the UIKit-facing method of the same name, and only while `isDisposed` is
+ * NO — i.e. only while the Kotlin peer is guaranteed to be alive. The default implementations
+ * reproduce the behaviour the UIKit-facing methods used to have.
+ */
+
+CMP_CAN_OVERRIDE - (NSArray<UIAccessibilityCustomAction *> *)cmp_accessibilityCustomActions;
+
+CMP_CAN_OVERRIDE - (UIAccessibilityTraits)cmp_accessibilityTraits;
+
+CMP_CAN_OVERRIDE - (UIAccessibilityContainerType)cmp_accessibilityContainerType;
+
+CMP_CAN_OVERRIDE - (NSString *__nullable)cmp_accessibilityIdentifier;
+
+CMP_CAN_OVERRIDE - (NSString *__nullable)cmp_accessibilityHint;
+
+CMP_CAN_OVERRIDE - (NSString *__nullable)cmp_accessibilityLabel;
+
+CMP_CAN_OVERRIDE - (NSAttributedString *__nullable)cmp_accessibilityAttributedLabel;
+
+CMP_CAN_OVERRIDE - (NSString *__nullable)cmp_accessibilityValue;
+
+CMP_CAN_OVERRIDE - (NSAttributedString *__nullable)cmp_accessibilityAttributedValue;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_accessibilityFrame;
+
+CMP_CAN_OVERRIDE - (__nullable id)cmp_accessibilityContainer;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_isAccessibilityElement;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_accessibilityActivate;
+
+CMP_CAN_OVERRIDE - (void)cmp_accessibilityIncrement;
+
+CMP_CAN_OVERRIDE - (void)cmp_accessibilityDecrement;
+
+CMP_CAN_OVERRIDE - (void)cmp_accessibilityElementDidBecomeFocused;
+
+CMP_CAN_OVERRIDE - (void)cmp_accessibilityElementDidLoseFocus;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_accessibilityScroll:(UIAccessibilityScrollDirection)direction;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_accessibilityPerformEscape;
+
+CMP_CAN_OVERRIDE - (NSArray *)cmp_accessibilityElements;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_drawsFocusRingWhenChildrenFocused;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_focusEffectRect;
+
+CMP_CAN_OVERRIDE - (nullable id)cmp_accessibilityHitTest:(CGPoint)point withEvent:(nullable UIEvent *)event;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_conformsToProtocol:(Protocol *__nullable)aProtocol;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_canBecomeFocused;
+
+CMP_CAN_OVERRIDE - (void)cmp_didUpdateFocusInContext:(UIFocusUpdateContext *)context
+ withAnimationCoordinator:(UIFocusAnimationCoordinator *)coordinator;
+
+CMP_CAN_OVERRIDE - (__nullable id<UIFocusItemContainer>)cmp_focusItemContainer;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_frame;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_bounds;
+
+CMP_CAN_OVERRIDE - (__nullable id<UIFocusEnvironment>)cmp_parentFocusEnvironment;
+
+CMP_CAN_OVERRIDE - (NSArray *)cmp_preferredFocusEnvironments;
+
+CMP_CAN_OVERRIDE - (void)cmp_setNeedsFocusUpdate;
+
+CMP_CAN_OVERRIDE - (void)cmp_updateFocusIfNeeded;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_shouldUpdateFocusInContext:(UIFocusUpdateContext *)context;
+
+CMP_CAN_OVERRIDE - (id<UICoordinateSpace>)cmp_coordinateSpace;
+
+CMP_CAN_OVERRIDE - (NSArray *)cmp_focusItemsInRect:(CGRect)rect;
+
+CMP_CAN_OVERRIDE - (BOOL)cmp_isTransparentFocusItem;
+
+CMP_CAN_OVERRIDE - (CGSize)cmp_visibleSize;
+
+CMP_CAN_OVERRIDE - (CGSize)cmp_contentSize;
+
+CMP_CAN_OVERRIDE - (CGPoint)cmp_contentOffset;
+
+CMP_CAN_OVERRIDE - (void)cmp_setContentOffset:(CGPoint)contentOffset;
+
+CMP_CAN_OVERRIDE - (CGPoint)cmp_convertPoint:(CGPoint)point toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+CMP_CAN_OVERRIDE - (CGPoint)cmp_convertPoint:(CGPoint)point fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_convertRect:(CGRect)rect toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
+CMP_CAN_OVERRIDE - (CGRect)cmp_convertRect:(CGRect)rect fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace;
+
@end
NS_ASSUME_NONNULL_END
diff --git a/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.m b/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.m
index b781b3741..5da260031 100644
--- a/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.m
+++ b/compose/ui/ui-uikit/src/iosMain/objc/CMPUIKitUtils/CMPUIKitObjcUtils/CMPAccessibilityElement.m
@@ -18,6 +18,27 @@
NS_ASSUME_NONNULL_BEGIN
+/**
+ * Front door for a method with no arguments: answer `disposedValue` without entering the subclass
+ * once the element is disposed, because from that point on its Kotlin peer may already be gone.
+ * See the lifetime contract in CMPAccessibilityElement.h.
+ */
+#define CMP_FRONT_DOOR(returnType, selector, disposedValue) \
+ - (returnType)selector { \
+ if (_isDisposed) { \
+ return disposedValue; \
+ } \
+ return [self cmp_##selector]; \
+ }
+
+#define CMP_FRONT_DOOR_VOID(selector) \
+ - (void)selector { \
+ if (_isDisposed) { \
+ return; \
+ } \
+ [self cmp_##selector]; \
+ }
+
@implementation CMPAccessibilityElement
+ (__nullable id)accessibilityContainerOfObject:(id)object {
@@ -25,108 +46,317 @@ + (__nullable id)accessibilityContainerOfObject:(id)object {
return [object accessibilityContainer];
}
-- (NSArray<UIAccessibilityCustomAction *> *)accessibilityCustomActions {
+#pragma mark - UIKit-facing methods
+
+CMP_FRONT_DOOR(NSArray<UIAccessibilityCustomAction *> *, accessibilityCustomActions, @[])
+CMP_FRONT_DOOR(UIAccessibilityTraits, accessibilityTraits, UIAccessibilityTraitNone)
+CMP_FRONT_DOOR(UIAccessibilityContainerType, accessibilityContainerType, UIAccessibilityContainerTypeNone)
+CMP_FRONT_DOOR(NSString *__nullable, accessibilityIdentifier, nil)
+CMP_FRONT_DOOR(NSString *__nullable, accessibilityHint, nil)
+CMP_FRONT_DOOR(NSString *__nullable, accessibilityLabel, nil)
+CMP_FRONT_DOOR(NSAttributedString *__nullable, accessibilityAttributedLabel, nil)
+CMP_FRONT_DOOR(NSString *__nullable, accessibilityValue, nil)
+CMP_FRONT_DOOR(NSAttributedString *__nullable, accessibilityAttributedValue, nil)
+CMP_FRONT_DOOR(CGRect, accessibilityFrame, CGRectZero)
+CMP_FRONT_DOOR(id __nullable, accessibilityContainer, nil)
+CMP_FRONT_DOOR(BOOL, isAccessibilityElement, NO)
+CMP_FRONT_DOOR(BOOL, accessibilityActivate, NO)
+CMP_FRONT_DOOR_VOID(accessibilityIncrement)
+CMP_FRONT_DOOR_VOID(accessibilityDecrement)
+CMP_FRONT_DOOR_VOID(accessibilityElementDidBecomeFocused)
+CMP_FRONT_DOOR_VOID(accessibilityElementDidLoseFocus)
+CMP_FRONT_DOOR(BOOL, accessibilityPerformEscape, NO)
+CMP_FRONT_DOOR(NSArray *, accessibilityElements, nil)
+CMP_FRONT_DOOR(BOOL, drawsFocusRingWhenChildrenFocused, NO)
+CMP_FRONT_DOOR(CGRect, focusEffectRect, CGRectZero)
+CMP_FRONT_DOOR(BOOL, canBecomeFocused, NO)
+CMP_FRONT_DOOR(id<UIFocusItemContainer> __nullable, focusItemContainer, nil)
+CMP_FRONT_DOOR(CGRect, frame, CGRectZero)
+CMP_FRONT_DOOR(CGRect, bounds, CGRectZero)
+CMP_FRONT_DOOR(id<UIFocusEnvironment> __nullable, parentFocusEnvironment, nil)
+CMP_FRONT_DOOR(NSArray *, preferredFocusEnvironments, @[])
+CMP_FRONT_DOOR_VOID(setNeedsFocusUpdate)
+CMP_FRONT_DOOR_VOID(updateFocusIfNeeded)
+// A disposed element still answers with a coordinate space — itself — because
+// UIFocusItemContainer declares this one non-null. Its conversion methods are inert.
+CMP_FRONT_DOOR(id<UICoordinateSpace>, coordinateSpace, (id<UICoordinateSpace>)self)
+CMP_FRONT_DOOR(BOOL, isTransparentFocusItem, YES)
+CMP_FRONT_DOOR(CGSize, visibleSize, CGSizeZero)
+CMP_FRONT_DOOR(CGSize, contentSize, CGSizeZero)
+CMP_FRONT_DOOR(CGPoint, contentOffset, CGPointZero)
+
+- (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
+ if (_isDisposed) {
+ return NO;
+ }
+ return [self cmp_accessibilityScroll:direction];
+}
+
+- (nullable id)accessibilityHitTest:(CGPoint)point withEvent:(nullable UIEvent *)event {
+ if (_isDisposed) {
+ return nil;
+ }
+ return [self cmp_accessibilityHitTest:point withEvent:event];
+}
+
+- (BOOL)conformsToProtocol:(Protocol *__nullable)protocol {
+ if (_isDisposed) {
+ return [super conformsToProtocol:protocol];
+ }
+ return [self cmp_conformsToProtocol:protocol];
+}
+
+- (void)didUpdateFocusInContext:(UIFocusUpdateContext *)context
+ withAnimationCoordinator:(UIFocusAnimationCoordinator *)coordinator {
+ if (_isDisposed) {
+ return;
+ }
+ [self cmp_didUpdateFocusInContext:context withAnimationCoordinator:coordinator];
+}
+
+- (BOOL)shouldUpdateFocusInContext:(UIFocusUpdateContext *)context {
+ if (_isDisposed) {
+ return YES;
+ }
+ return [self cmp_shouldUpdateFocusInContext:context];
+}
+
+- (NSArray *)focusItemsInRect:(CGRect)rect {
+ if (_isDisposed) {
+ return @[];
+ }
+ return [self cmp_focusItemsInRect:rect];
+}
+
+- (void)setContentOffset:(CGPoint)contentOffset {
+ if (_isDisposed) {
+ return;
+ }
+ [self cmp_setContentOffset:contentOffset];
+}
+
+- (CGPoint)convertPoint:(CGPoint)point toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ if (_isDisposed) {
+ return point;
+ }
+ return [self cmp_convertPoint:point toCoordinateSpace:coordinateSpace];
+}
+
+- (CGPoint)convertPoint:(CGPoint)point fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ if (_isDisposed) {
+ return point;
+ }
+ return [self cmp_convertPoint:point fromCoordinateSpace:coordinateSpace];
+}
+
+- (CGRect)convertRect:(CGRect)rect toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ if (_isDisposed) {
+ return rect;
+ }
+ return [self cmp_convertRect:rect toCoordinateSpace:coordinateSpace];
+}
+
+- (CGRect)convertRect:(CGRect)rect fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ if (_isDisposed) {
+ return rect;
+ }
+ return [self cmp_convertRect:rect fromCoordinateSpace:coordinateSpace];
+}
+
+// Not overridden in Kotlin, and safe to run on a disposed element: it only touches UIKit state.
+- (void)setAccessibilityElements:(nullable NSArray *)accessibilityElements {
+ [super setAccessibilityElements:accessibilityElements];
+}
+
+- (BOOL)_drawsFocusRingWhenChildrenFocused {
+ return [self drawsFocusRingWhenChildrenFocused];
+}
+
+- (UIFocusEffect *)focusEffect {
+ return [UIFocusHaloEffect effectWithRect:[self focusEffectRect]];
+}
+
+#pragma mark - Hooks
+
+- (NSArray<UIAccessibilityCustomAction *> *)cmp_accessibilityCustomActions {
return [super accessibilityCustomActions];
}
-- (UIAccessibilityTraits)accessibilityTraits {
+- (UIAccessibilityTraits)cmp_accessibilityTraits {
return [super accessibilityTraits];
}
-- (UIAccessibilityContainerType)accessibilityContainerType {
+- (UIAccessibilityContainerType)cmp_accessibilityContainerType {
return [super accessibilityContainerType];
}
-- (NSString *__nullable)accessibilityIdentifier {
+- (NSString *__nullable)cmp_accessibilityIdentifier {
return [super accessibilityIdentifier];
}
-- (NSString *__nullable)accessibilityHint {
+- (NSString *__nullable)cmp_accessibilityHint {
return [super accessibilityHint];
}
-- (NSString *__nullable)accessibilityLabel {
+- (NSString *__nullable)cmp_accessibilityLabel {
return [super accessibilityLabel];
}
-- (NSAttributedString *__nullable)accessibilityAttributedLabel {
+- (NSAttributedString *__nullable)cmp_accessibilityAttributedLabel {
return [super accessibilityAttributedLabel];
}
-- (NSAttributedString *__nullable)accessibilityAttributedValue {
+- (NSAttributedString *__nullable)cmp_accessibilityAttributedValue {
return [super accessibilityAttributedLabel];
}
-- (NSString *__nullable)accessibilityValue {
+- (NSString *__nullable)cmp_accessibilityValue {
return [super accessibilityValue];
}
-- (CGRect)accessibilityFrame {
+- (CGRect)cmp_accessibilityFrame {
return [super accessibilityFrame];
}
-- (BOOL)isAccessibilityElement {
+- (__nullable id)cmp_accessibilityContainer {
+ return [super accessibilityContainer];
+}
+
+- (BOOL)cmp_isAccessibilityElement {
return [super isAccessibilityElement];
}
-- (BOOL)accessibilityActivate {
+- (BOOL)cmp_accessibilityActivate {
return [super accessibilityActivate];
}
-- (void)accessibilityIncrement {
+- (void)cmp_accessibilityIncrement {
[super accessibilityIncrement];
}
-- (void)accessibilityDecrement {
+- (void)cmp_accessibilityDecrement {
[super accessibilityDecrement];
}
-- (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
+- (BOOL)cmp_accessibilityScroll:(UIAccessibilityScrollDirection)direction {
return [super accessibilityScroll:direction];
}
-- (BOOL)accessibilityPerformEscape {
+- (BOOL)cmp_accessibilityPerformEscape {
return [super accessibilityPerformEscape];
}
-- (void)accessibilityElementDidBecomeFocused {
+- (void)cmp_accessibilityElementDidBecomeFocused {
[super accessibilityElementDidBecomeFocused];
}
-- (void)accessibilityElementDidLoseFocus {
+- (void)cmp_accessibilityElementDidLoseFocus {
[super accessibilityElementDidLoseFocus];
}
-- (NSArray *)accessibilityElements {
+- (NSArray *)cmp_accessibilityElements {
return [super accessibilityElements];
}
-- (void)setAccessibilityElements:(nullable NSArray *)accessibilityElements {
- [super setAccessibilityElements:accessibilityElements];
+- (BOOL)cmp_drawsFocusRingWhenChildrenFocused {
+ return NO;
}
-- (BOOL)_drawsFocusRingWhenChildrenFocused {
- return [self drawsFocusRingWhenChildrenFocused];
+- (CGRect)cmp_focusEffectRect {
+ return CGRectZero;
+}
+
+- (nullable id)cmp_accessibilityHitTest:(CGPoint)point withEvent:(nullable UIEvent *)event {
+ if (@available(iOS 18.0, *)) {
+ return [super accessibilityHitTest:point withEvent:event];
+ } else {
+ return nil;
+ }
+}
+
+- (BOOL)cmp_conformsToProtocol:(Protocol *__nullable)aProtocol {
+ return [super conformsToProtocol:aProtocol];
}
-- (BOOL)drawsFocusRingWhenChildrenFocused {
+- (BOOL)cmp_canBecomeFocused {
return NO;
}
-- (CGRect)focusEffectRect {
+- (void)cmp_didUpdateFocusInContext:(UIFocusUpdateContext *)context
+ withAnimationCoordinator:(UIFocusAnimationCoordinator *)coordinator {
+}
+
+- (__nullable id<UIFocusItemContainer>)cmp_focusItemContainer {
+ return nil;
+}
+
+- (CGRect)cmp_frame {
return CGRectZero;
}
-- (UIFocusEffect *)focusEffect {
- return [UIFocusHaloEffect effectWithRect:[self focusEffectRect]];
+- (CGRect)cmp_bounds {
+ return CGRectZero;
}
-- (nullable id)accessibilityHitTest:(CGPoint)point withEvent:(nullable UIEvent *)event {
- if (@available(iOS 18.0, *)) {
- return [super accessibilityHitTest:point withEvent:event];
- } else {
- return nil;
- }
+- (__nullable id<UIFocusEnvironment>)cmp_parentFocusEnvironment {
+ return nil;
+}
+
+- (NSArray *)cmp_preferredFocusEnvironments {
+ return @[];
+}
+
+- (void)cmp_setNeedsFocusUpdate {
+}
+
+- (void)cmp_updateFocusIfNeeded {
+}
+
+- (BOOL)cmp_shouldUpdateFocusInContext:(UIFocusUpdateContext *)context {
+ return YES;
+}
+
+- (id<UICoordinateSpace>)cmp_coordinateSpace {
+ return (id<UICoordinateSpace>)self;
+}
+
+- (NSArray *)cmp_focusItemsInRect:(CGRect)rect {
+ return @[];
+}
+
+- (BOOL)cmp_isTransparentFocusItem {
+ return YES;
+}
+
+- (CGSize)cmp_visibleSize {
+ return CGSizeZero;
+}
+
+- (CGSize)cmp_contentSize {
+ return CGSizeZero;
+}
+
+- (CGPoint)cmp_contentOffset {
+ return CGPointZero;
+}
+
+- (void)cmp_setContentOffset:(CGPoint)contentOffset {
+}
+
+- (CGPoint)cmp_convertPoint:(CGPoint)point toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ return point;
+}
+
+- (CGPoint)cmp_convertPoint:(CGPoint)point fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ return point;
+}
+
+- (CGRect)cmp_convertRect:(CGRect)rect toCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ return rect;
+}
+
+- (CGRect)cmp_convertRect:(CGRect)rect fromCoordinateSpace:(id<UICoordinateSpace>)coordinateSpace {
+ return rect;
}
- (void)dealloc {
diff --git a/compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/platform/Accessibility.ios.kt b/compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/platform/Accessibility.ios.kt
index 056eec97b..167367c63 100644
--- a/compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/platform/Accessibility.ios.kt
+++ b/compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/platform/Accessibility.ios.kt
@@ -463,26 +463,35 @@ private class AccessibilityRoot(
setAccessibilityElements(value?.let { listOf(it) })
}
- override fun accessibilityElements(): List<*> {
+ /**
+ * Marks the root as gone before the mediator drops its reference to it, so that messages the
+ * accessibility runtime still sends afterwards never reach Kotlin. See CMP-10615.
+ */
+ fun dispose() {
+ element = null
+ isDisposed = true
+ }
+
+ override fun cmp_accessibilityElements(): List<*> {
if (mediator.isEnabled) {
mediator.activateAccessibilityIfNeeded()
}
- return super.accessibilityElements()
+ return super.cmp_accessibilityElements()
}
- override fun isAccessibilityElement(): Boolean = false
+ override fun cmp_isAccessibilityElement(): Boolean = false
- override fun accessibilityContainer() = mediator.view
+ override fun cmp_accessibilityContainer() = mediator.view
- override fun accessibilityFrame(): CValue<CGRect> =
+ override fun cmp_accessibilityFrame(): CValue<CGRect> =
mediator.view.convertRect(mediator.view.bounds, toView = null)
// UIFocusItemContainerProtocol
- override fun coordinateSpace(): UICoordinateSpaceProtocol = mediator.view
+ override fun cmp_coordinateSpace(): UICoordinateSpaceProtocol = mediator.view
- override fun focusItemsInRect(rect: CValue<CGRect>): List<*> {
+ override fun cmp_focusItemsInRect(rect: CValue<CGRect>): List<*> {
return if (mediator.isEnabled) {
mediator.activateAccessibilityIfNeeded()
listOfNotNull(element)
@@ -491,7 +500,7 @@ private class AccessibilityRoot(
}
}
- override fun accessibilityHitTest(point: CValue<CGPoint>, withEvent: UIEvent?): Any? {
+ override fun cmp_accessibilityHitTest(point: CValue<CGPoint>, withEvent: UIEvent?): Any? {
if (!mediator.isEnabled) {
return null
}
@@ -537,7 +546,7 @@ private class AccessibilityRoot(
}
// Used as a backup to iOS-like focus behavior
- return super.accessibilityHitTest(point, withEvent)
+ return super.cmp_accessibilityHitTest(point, withEvent)
}
}
@@ -561,11 +570,11 @@ private class AccessibilityElement(
private val cachedProperties = mutableMapOf<CachedAccessibilityPropertyKey<*>, Any?>()
private val scrollableProtocol = objc_getProtocol("UIFocusItemScrollableContainer")!!
- override fun conformsToProtocol(aProtocol: Protocol?): Boolean {
+ override fun cmp_conformsToProtocol(aProtocol: Protocol?): Boolean {
if (protocol_isEqual(proto = aProtocol, other = scrollableProtocol)) {
return getIfAlive { node.canScroll } ?: false
}
- return super.conformsToProtocol(aProtocol)
+ return super.cmp_conformsToProtocol(aProtocol)
}
val key: AccessibilityElementKey get() = node.key
@@ -659,80 +668,80 @@ private class AccessibilityElement(
return block()
}
- override fun accessibilityLabel(): String? = accessibilityAttributedLabel()?.string
+ override fun cmp_accessibilityLabel(): String? = accessibilityAttributedLabel()?.string
- override fun accessibilityAttributedLabel(): NSAttributedString? =
+ override fun cmp_accessibilityAttributedLabel(): NSAttributedString? =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityAttributedLabel) {
makeAccessibilityAttributedLabel()
}
- override fun accessibilityValue(): String? = accessibilityAttributedValue()?.string
+ override fun cmp_accessibilityValue(): String? = accessibilityAttributedValue()?.string
- override fun accessibilityAttributedValue(): NSAttributedString? =
+ override fun cmp_accessibilityAttributedValue(): NSAttributedString? =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityAttributedValue) {
node.accessibilityAttributedValue
}
- override fun accessibilityElementDidBecomeFocused() = runIfAlive {
+ override fun cmp_accessibilityElementDidBecomeFocused() = runIfAlive {
node.accessibilityElementDidBecomeFocused()
}
- override fun accessibilityElementDidLoseFocus() = runIfAlive {
+ override fun cmp_accessibilityElementDidLoseFocus() = runIfAlive {
node.accessibilityElementDidLoseFocus()
}
- override fun accessibilityActivate(): Boolean = getIfAlive {
+ override fun cmp_accessibilityActivate(): Boolean = getIfAlive {
node.accessibilityActivate()
} ?: false
- override fun accessibilityIncrement() = runIfAlive {
+ override fun cmp_accessibilityIncrement() = runIfAlive {
node.accessibilityIncrement()
}
- override fun accessibilityDecrement() = runIfAlive {
+ override fun cmp_accessibilityDecrement() = runIfAlive {
node.accessibilityDecrement()
}
- override fun accessibilityScroll(direction: UIAccessibilityScrollDirection): Boolean =
+ override fun cmp_accessibilityScroll(direction: UIAccessibilityScrollDirection): Boolean =
getIfAlive {
node.accessibilityScroll(direction)
} ?: false
- override fun isAccessibilityElement(): Boolean = getIfAlive {
+ override fun cmp_isAccessibilityElement(): Boolean = getIfAlive {
// Node visibility changes don't trigger accessibility semantic recalculation.
// This value should not be cached. See [SemanticsNode.isScreenReaderFocusable()]
node.isAccessibilityElement
} ?: false
- override fun accessibilityIdentifier(): String? =
+ override fun cmp_accessibilityIdentifier(): String? =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityIdentifier) {
node.accessibilityIdentifier
}
- override fun accessibilityHint(): String? =
+ override fun cmp_accessibilityHint(): String? =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityHint) {
node.accessibilityHint
}
- override fun accessibilityCustomActions(): List<UIAccessibilityCustomAction> =
+ override fun cmp_accessibilityCustomActions(): List<UIAccessibilityCustomAction> =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityCustomActions, emptyList()) {
node.accessibilityCustomActions
}
- override fun accessibilityTraits(): UIAccessibilityTraits =
+ override fun cmp_accessibilityTraits(): UIAccessibilityTraits =
getCachedIfAlive(CachedAccessibilityPropertyKeys.accessibilityTraits, UIAccessibilityTraitNone) {
node.accessibilityTraits
}
- override fun accessibilityPerformEscape(): Boolean = getIfAlive {
+ override fun cmp_accessibilityPerformEscape(): Boolean = getIfAlive {
if (node.accessibilityPerformEscape()) {
true
} else {
- super.accessibilityPerformEscape()
+ super.cmp_accessibilityPerformEscape()
}
} ?: false
- override fun accessibilityContainerType(): UIAccessibilityContainerType =
+ override fun cmp_accessibilityContainerType(): UIAccessibilityContainerType =
getIfAlive {
node.accessibilityContainerType
} ?: UIAccessibilityContainerTypeNone
@@ -756,9 +765,9 @@ private class AccessibilityElement(
// UIFocusItemProtocol & UIFocusItemContainerProtocol
- override fun canBecomeFocused(): Boolean = getIfAlive { node.canBecomeFocused } ?: false
+ override fun cmp_canBecomeFocused(): Boolean = getIfAlive { node.canBecomeFocused } ?: false
- override fun didUpdateFocusInContext(
+ override fun cmp_didUpdateFocusInContext(
context: UIFocusUpdateContext,
withAnimationCoordinator: UIFocusAnimationCoordinator
) = runIfAlive {
@@ -770,18 +779,18 @@ private class AccessibilityElement(
}
}
- override fun focusItemContainer(): UIFocusItemContainerProtocol = this
+ override fun cmp_focusItemContainer(): UIFocusItemContainerProtocol = this
var focusFrame: CValue<CGRect> = CGRectZero.readValue()
- override fun frame(): CValue<CGRect> = if (USE_HIERARCHICAL_COORDINATE_SPACE) {
+ override fun cmp_frame(): CValue<CGRect> = if (USE_HIERARCHICAL_COORDINATE_SPACE) {
focusFrame
} else {
convertRect(rect = bounds(), toCoordinateSpace = mediator.view)
}
- override fun focusEffectRect(): CValue<CGRect> = convertRect(rect = bounds, toCoordinateSpace = mediator.view)
+ override fun cmp_focusEffectRect(): CValue<CGRect> = convertRect(rect = bounds, toCoordinateSpace = mediator.view)
- override fun bounds(): CValue<CGRect> {
+ override fun cmp_bounds(): CValue<CGRect> {
val offset = contentOffset()
return CGRectMake(
x = offset.useContents { x },
@@ -791,14 +800,14 @@ private class AccessibilityElement(
)
}
- override fun parentFocusEnvironment(): UIFocusEnvironmentProtocol? =
+ override fun cmp_parentFocusEnvironment(): UIFocusEnvironmentProtocol? =
accessibilityContainer as? UIFocusEnvironmentProtocol
- override fun preferredFocusEnvironments(): List<*> =
+ override fun cmp_preferredFocusEnvironments(): List<*> =
accessibilityElements?.filterIsInstance<UIFocusEnvironmentProtocol>() ?: emptyList<Any>()
private var updateFocusScheduled = false
- override fun setNeedsFocusUpdate() {
+ override fun cmp_setNeedsFocusUpdate() {
if (updateFocusScheduled) {
return
}
@@ -809,40 +818,40 @@ private class AccessibilityElement(
}
}
- override fun updateFocusIfNeeded() {
+ override fun cmp_updateFocusIfNeeded() {
UIFocusSystem.focusSystemForEnvironment(environment = this)?.updateFocusIfNeeded()
}
- override fun shouldUpdateFocusInContext(context: UIFocusUpdateContext): Boolean = true
+ override fun cmp_shouldUpdateFocusInContext(context: UIFocusUpdateContext): Boolean = true
- override fun coordinateSpace(): UICoordinateSpaceProtocol =
+ override fun cmp_coordinateSpace(): UICoordinateSpaceProtocol =
if (USE_HIERARCHICAL_COORDINATE_SPACE) {
this
} else {
mediator.view
}
- override fun focusItemsInRect(rect: CValue<CGRect>): List<*> = accessibilityElements?.filter {
+ override fun cmp_focusItemsInRect(rect: CValue<CGRect>): List<*> = accessibilityElements?.filter {
it is UIFocusItemProtocol && CGRectIntersectsRect(it.frame, rect)
} ?: emptyList<Any>()
- override fun isTransparentFocusItem(): Boolean = true
+ override fun cmp_isTransparentFocusItem(): Boolean = true
- override fun drawsFocusRingWhenChildrenFocused(): Boolean =
+ override fun cmp_drawsFocusRingWhenChildrenFocused(): Boolean =
getIfAlive { node.canScroll } ?: false
// Scrolling
- override fun visibleSize(): CValue<CGSize> =
+ override fun cmp_visibleSize(): CValue<CGSize> =
getIfAlive { node.scrollVisibleSize } ?: CGSizeZero.readValue()
- override fun contentSize(): CValue<CGSize> =
+ override fun cmp_contentSize(): CValue<CGSize> =
getIfAlive { node.scrollContentSize } ?: CGSizeZero.readValue()
- override fun contentOffset(): CValue<CGPoint> =
+ override fun cmp_contentOffset(): CValue<CGPoint> =
getIfAlive { node.scrollContentOffset } ?: CGPointZero.readValue()
- override fun setContentOffset(contentOffset: CValue<CGPoint>) = runIfAlive {
+ override fun cmp_setContentOffset(contentOffset: CValue<CGPoint>) = runIfAlive {
val currentContentOffset = contentOffset()
val delta = CGPointMake(
x = contentOffset.useContents { x } - currentContentOffset.useContents { x },
@@ -870,7 +879,7 @@ private class AccessibilityElement(
// UICoordinateSpaceProtocol
@ObjCSignatureOverride
- override fun convertPoint(
+ override fun cmp_convertPoint(
point: CValue<CGPoint>,
toCoordinateSpace: UICoordinateSpaceProtocol
): CValue<CGPoint> {
@@ -883,7 +892,7 @@ private class AccessibilityElement(
}
@ObjCSignatureOverride
- override fun convertPoint(
+ override fun cmp_convertPoint(
point: CValue<CGPoint>,
fromCoordinateSpace: UICoordinateSpaceProtocol
): CValue<CGPoint> {
@@ -896,7 +905,7 @@ private class AccessibilityElement(
}
@ObjCSignatureOverride
- override fun convertRect(
+ override fun cmp_convertRect(
rect: CValue<CGRect>,
toCoordinateSpace: UICoordinateSpaceProtocol
): CValue<CGRect> {
@@ -909,7 +918,7 @@ private class AccessibilityElement(
}
@ObjCSignatureOverride
- override fun convertRect(
+ override fun cmp_convertRect(
rect: CValue<CGRect>,
fromCoordinateSpace: UICoordinateSpaceProtocol
): CValue<CGRect> {
@@ -1394,6 +1403,7 @@ internal class AccessibilityMediator(
view.accessibilityElements = listOf<NSObject>()
cleanUp()
+ root.dispose()
}
private fun cleanUp() {
@@ -1405,6 +1415,10 @@ internal class AccessibilityMediator(
root.element = null
+ // Dispose before dropping the last Kotlin reference to these elements. Once the map is
+ // cleared they are unreachable from Kotlin, so the garbage collector is free to collect
+ // their Kotlin peers, while the accessibility runtime can still message the Objective-C
+ // halves: marking them disposed is what makes those messages inert. See CMP-10615.
for (element in accessibilityElementsMap.values) {
element.dispose()
} |
|
Pierre-Luc Ledoux (@ledouxpl) , thank you for the investigation. I found a better way to flip the condition. Let's see if it helps. |
|
| * Indicates whether this element is still present in the tree. | ||
| */ | ||
| private val isAlive get() = !isDisposed && node.semanticsNode.isValid | ||
| private val isAlive get() = isInitialized && node.semanticsNode.isValid |
There was a problem hiding this comment.
Safe with a null receiver now — isInitialized is a message to nil, which answers NO, so && stops before node.
The catch: this only helps where isAlive is consulted before any Kotlin field is read. These read a field first, so a message arriving after the peer is collected still faults:
bounds()—contentOffset()is safe now, thenfocusFramefaults. This is theaccessibilityFrame→bounds→contentOffsetpath that 3 of our 4 measured crashes came through, so I'd expect them to come back one line later, infocusFrame:conformsToProtocol()— readsscrollableProtocolfirst, and UIKit calls it constantly:frame()andfocusEffectRect()—focusFrame/mediator:setNeedsFocusUpdate()—updateFocusScheduled:coordinateSpace()—mediator, below iOS 18:AccessibilityRoot— none of its overrides consult a flag; they all readmediator:
Doing the check in Objective-C, before the message reaches Kotlin, makes that ordering structural rather than a convention each override has to remember. Patch in my earlier comment.
There was a problem hiding this comment.
true. focusFrame will the the issue then.
The reason why I don't want to use the solution provided by AI is that it will require more refactoring then actually written (getIfAlive must go then). With all these changes it will be no go for 1.12.1 then. So let's see what we can do here.
65aa74a to
4b74bb9
Compare
Fix accessibility elements disposal before the cleanup Fixes https://youtrack.jetbrains.com/issue/CMP-10615 ## Release Notes ### Fixes - iOS - _(prerelease fix)_ Fix crash when iOS reads `AccessibilityElement`'s properties after disposal.
Fix accessibility elements disposal before the cleanup
Fixes https://youtrack.jetbrains.com/issue/CMP-10615
Release Notes
N/A