From 3185a9fca1386f5ba062f7adafa4d102f2d7e036 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 3 Apr 2020 10:35:25 -0700 Subject: [PATCH 1/5] Started clearing out the parent of orphaned semantic objects. --- .../framework/Source/accessibility_bridge.mm | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index b07a32989f18e..114512c74070f 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -14,6 +14,8 @@ #include "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h" #include "flutter/shell/platform/darwin/ios/platform_view_ios.h" +FLUTTER_ASSERT_NOT_ARC + namespace { constexpr int32_t kRootNodeId = 0; @@ -199,7 +201,6 @@ - (void)dealloc { for (SemanticsObject* child in _children) { child.parent = nil; } - [_children removeAllObjects]; [_children release]; _parent = nil; _container.get().semanticsObject = nil; @@ -239,6 +240,17 @@ - (BOOL)hasChildren { return [self.children count] != 0; } +- (void)setChildren:(NSMutableArray*)children { + for (SemanticsObject* child in _children) { + child.parent = nil; + } + [_children release]; + _children = [children retain]; + for (SemanticsObject* child in _children) { + child.parent = self; + } +} + #pragma mark - UIAccessibility overrides - (BOOL)isAccessibilityElement { @@ -653,7 +665,7 @@ - (NSInteger)indexOfAccessibilityElement:(id)element { return ((FlutterPlatformViewSemanticsContainer*)element).index; } - NSMutableArray* children = [_semanticsObject children]; + NSArray* children = [_semanticsObject children]; for (size_t i = 0; i < [children count]; i++) { SemanticsObject* child = children[i]; if ((![child hasChildren] && child == element) || @@ -850,6 +862,7 @@ static void ReplaceSemanticsObject(SemanticsObject* oldObject, SemanticsObject* parent = oldObject.parent; [objects removeObjectForKey:nodeId]; newObject.parent = parent; + oldObject.parent = nil; [newObject.parent.children replaceObjectAtIndex:positionInChildlist withObject:newObject]; objects[nodeId] = newObject; } From dd64273dd131b238d8011c12be09092c41eb80c8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 3 Apr 2020 10:53:36 -0700 Subject: [PATCH 2/5] Made "SemanticsObject.children" return an immutable collection. --- .../ios/framework/Source/accessibility_bridge.h | 4 +++- .../ios/framework/Source/accessibility_bridge.mm | 13 +++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h index d6e0bb6446e0c..2c3c9e04b5460 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h @@ -85,13 +85,15 @@ class AccessibilityBridge; * Direct children of this semantics object. Each child's `parent` property must * be equal to this object. */ -@property(nonatomic, strong) NSMutableArray* children; +@property(nonatomic, strong) NSArray* children; /** * Used if this SemanticsObject is for a platform view. */ @property(strong, nonatomic) FlutterPlatformViewSemanticsContainer* platformViewSemanticsContainer; +- (void)replaceChildAtIndex:(NSInteger)index withChild:(SemanticsObject*)child; + - (BOOL)nodeWillCauseLayoutChange:(const flutter::SemanticsNode*)node; #pragma mark - Designated initializers diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index 114512c74070f..a99a62c1efdd6 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -166,6 +166,7 @@ - (instancetype)initWithSemanticsObject:(SemanticsObject*)semanticsObject @implementation SemanticsObject { fml::scoped_nsobject _container; + NSMutableArray* _children; } #pragma mark - Override base class designated initializers @@ -251,6 +252,13 @@ - (void)setChildren:(NSMutableArray*)children { } } +- (void)replaceChildAtIndex:(NSInteger)index withChild:(SemanticsObject*)child { + SemanticsObject* oldChild = _children[index]; + [_children replaceObjectAtIndex:index withObject:child]; + oldChild.parent = nil; + child.parent = self; +} + #pragma mark - UIAccessibility overrides - (BOOL)isAccessibilityElement { @@ -859,11 +867,8 @@ static void ReplaceSemanticsObject(SemanticsObject* oldObject, assert(oldObject.node.id == newObject.node.id); NSNumber* nodeId = @(oldObject.node.id); NSUInteger positionInChildlist = [oldObject.parent.children indexOfObject:oldObject]; - SemanticsObject* parent = oldObject.parent; [objects removeObjectForKey:nodeId]; - newObject.parent = parent; - oldObject.parent = nil; - [newObject.parent.children replaceObjectAtIndex:positionInChildlist withObject:newObject]; + [oldObject.parent replaceChildAtIndex:positionInChildlist withChild:newObject]; objects[nodeId] = newObject; } From 534845dadbb5c0856744052cfcaa067eada961fb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 3 Apr 2020 11:03:06 -0700 Subject: [PATCH 3/5] Made "SemanticsObject.parent" not publicly settable (must go through children editing methods). --- .../framework/Source/accessibility_bridge.h | 2 +- .../framework/Source/accessibility_bridge.mm | 20 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h index 2c3c9e04b5460..772c84eb5ef0e 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h @@ -43,7 +43,7 @@ class AccessibilityBridge; * The parent of this node in the node tree. Will be nil for the root node and * during transient state changes. */ -@property(nonatomic, assign) SemanticsObject* parent; +@property(nonatomic, readonly) SemanticsObject* parent; /** * The accessibility bridge that this semantics object is attached to. This diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index a99a62c1efdd6..3a7c9379fb32e 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -164,6 +164,11 @@ - (instancetype)initWithSemanticsObject:(SemanticsObject*)semanticsObject @end +@interface SemanticsObject () +/** Should only be called in conjunction with setting child/parent relationship. */ +- (void)privateSetParent:(SemanticsObject*)parent; +@end + @implementation SemanticsObject { fml::scoped_nsobject _container; NSMutableArray* _children; @@ -200,7 +205,7 @@ - (instancetype)initWithBridge:(fml::WeakPtr)bridg - (void)dealloc { for (SemanticsObject* child in _children) { - child.parent = nil; + [child privateSetParent:nil]; } [_children release]; _parent = nil; @@ -241,22 +246,26 @@ - (BOOL)hasChildren { return [self.children count] != 0; } +- (void)privateSetParent:(SemanticsObject*)parent { + _parent = parent; +} + - (void)setChildren:(NSMutableArray*)children { for (SemanticsObject* child in _children) { - child.parent = nil; + [child privateSetParent:nil]; } [_children release]; _children = [children retain]; for (SemanticsObject* child in _children) { - child.parent = self; + [child privateSetParent:self]; } } - (void)replaceChildAtIndex:(NSInteger)index withChild:(SemanticsObject*)child { SemanticsObject* oldChild = _children[index]; [_children replaceObjectAtIndex:index withObject:child]; - oldChild.parent = nil; - child.parent = self; + [oldChild privateSetParent:nil]; + [child privateSetParent:self]; } #pragma mark - UIAccessibility overrides @@ -761,7 +770,6 @@ - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction { [[[NSMutableArray alloc] initWithCapacity:newChildCount] autorelease]; for (NSUInteger i = 0; i < newChildCount; ++i) { SemanticsObject* child = GetOrCreateObject(node.childrenInTraversalOrder[i], nodes); - child.parent = object; [newChildren addObject:child]; } object.children = newChildren; From da3ffbf8562299dbfa5b4b2ab42c142b79a5b0af Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 3 Apr 2020 11:14:25 -0700 Subject: [PATCH 4/5] fixed signature on setChildren --- .../darwin/ios/framework/Source/accessibility_bridge.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index 3a7c9379fb32e..f2db354b579ad 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -250,12 +250,12 @@ - (void)privateSetParent:(SemanticsObject*)parent { _parent = parent; } -- (void)setChildren:(NSMutableArray*)children { +- (void)setChildren:(NSArray*)children { for (SemanticsObject* child in _children) { [child privateSetParent:nil]; } [_children release]; - _children = [children retain]; + _children = [[NSMutableArray alloc] initWithArray:children]; for (SemanticsObject* child in _children) { [child privateSetParent:self]; } From c32f9ac7ca4c12448e5a3cecbe823ec07d9ef922 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 3 Apr 2020 12:30:10 -0700 Subject: [PATCH 5/5] replaced removeAllObjects call, changed order of settting the parent to make sure an object isn't deleted beforehand --- .../darwin/ios/framework/Source/accessibility_bridge.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index f2db354b579ad..961f3184f20aa 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -207,6 +207,7 @@ - (void)dealloc { for (SemanticsObject* child in _children) { [child privateSetParent:nil]; } + [_children removeAllObjects]; [_children release]; _parent = nil; _container.get().semanticsObject = nil; @@ -263,9 +264,9 @@ - (void)setChildren:(NSArray*)children { - (void)replaceChildAtIndex:(NSInteger)index withChild:(SemanticsObject*)child { SemanticsObject* oldChild = _children[index]; - [_children replaceObjectAtIndex:index withObject:child]; [oldChild privateSetParent:nil]; [child privateSetParent:self]; + [_children replaceObjectAtIndex:index withObject:child]; } #pragma mark - UIAccessibility overrides