From 2cacf194d5107411dee2ebab7f56f1051f3c6a05 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Thu, 29 Jul 2021 14:57:02 +0800 Subject: [PATCH 1/8] fix crash when SemanticsObject dealloc and access the children --- .../darwin/ios/framework/Source/SemanticsObject.mm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index cb74cb4275821..31b89063f1aee 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -406,6 +406,7 @@ - (void)dealloc { } [_children removeAllObjects]; [_children release]; + _children = nil; _parent = nil; _container.get().semanticsObject = nil; [_platformViewSemanticsContainer release]; @@ -429,7 +430,11 @@ - (BOOL)hasChildren { if (_node.IsPlatformViewNode()) { return YES; } - return [self.children count] != 0; + if(_children){ + return [self.children count] != 0; + }else{ + return NO; + } } #pragma mark - Semantic object method From 6a573b9d753ae1078d3ff8f215fdcaf6adc871e6 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Thu, 29 Jul 2021 15:45:49 +0800 Subject: [PATCH 2/8] fix No trailing whitespace found. --- shell/platform/darwin/ios/framework/Source/SemanticsObject.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 31b89063f1aee..e3a1b8a57dcc1 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -430,9 +430,9 @@ - (BOOL)hasChildren { if (_node.IsPlatformViewNode()) { return YES; } - if(_children){ + if (_children) { return [self.children count] != 0; - }else{ + } else { return NO; } } From 50028185c9b315ebcfe2076a440c25f38198a5e8 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Tue, 3 Aug 2021 12:58:53 +0800 Subject: [PATCH 3/8] add SemanticsObjectDeallocated test --- .../ios/framework/Source/SemanticsObjectTest.mm | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index f475a563c1633..cce3ed0479746 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -644,4 +644,15 @@ - (void)testFlutterSwitchSemanticsObjectMatchesUISwitch { XCTAssertEqual(object.accessibilityValue, nativeSwitch.accessibilityValue); } +- (void)testSemanticsObjectDeallocated { + fml::WeakPtrFactory factory( + new flutter::MockAccessibilityBridge()); + fml::WeakPtr bridge = factory.GetWeakPtr(); + SemanticsObject* parent = [[SemanticsObject alloc] initWithBridge:bridge uid:0]; + SemanticsObject* child = [[SemanticsObject alloc] initWithBridge:bridge uid:1]; + parent.children = @[ child ]; + __weak SemanticsObject* weakObject = parent; + parent = nil; + XCTAssertNil(weakObject); +} @end From 37efc179ac91adfca009d79d07af772451d85df0 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Wed, 4 Aug 2021 11:32:46 +0800 Subject: [PATCH 4/8] call [_chilren release] after [super dealloc] --- .../darwin/ios/framework/Source/SemanticsObject.mm | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index e3a1b8a57dcc1..c475959c64e15 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -405,12 +405,15 @@ - (void)dealloc { [child privateSetParent:nil]; } [_children removeAllObjects]; - [_children release]; _children = nil; - _parent = nil; _container.get().semanticsObject = nil; - [_platformViewSemanticsContainer release]; [super dealloc]; + + // Due to a bug in -[UIAccessibilityElementSuperCategory dealloc] + // that calls into self.children, delay releasing ivars until after `[super dealloc]`. + // https://github.com/flutter/flutter/issues/87247 + [_children release]; + [_platformViewSemanticsContainer release]; } #pragma mark - Semantic object property accesser From e62de3de821afa8d2acab592f59f83fc71e036f8 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Thu, 5 Aug 2021 19:40:43 +0800 Subject: [PATCH 5/8] use _inDealloc to prevent accessing _children when [super dealloc] --- .../ios/framework/Source/SemanticsObject.mm | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 5a60578869a9d..bf64ddccdcbac 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -372,6 +372,7 @@ - (void)privateSetParent:(SemanticsObject*)parent; @implementation SemanticsObject { fml::scoped_nsobject _container; NSMutableArray* _children; + BOOL _inDealloc; } #pragma mark - Override base class designated initializers @@ -399,6 +400,7 @@ - (instancetype)initWithBridge:(fml::WeakPtr)br _bridge = bridge; _uid = uid; _children = [[NSMutableArray alloc] init]; + _inDealloc = NO; } return self; @@ -409,15 +411,12 @@ - (void)dealloc { [child privateSetParent:nil]; } [_children removeAllObjects]; - _children = nil; - _container.get().semanticsObject = nil; - [super dealloc]; - - // Due to a bug in -[UIAccessibilityElementSuperCategory dealloc] - // that calls into self.children, delay releasing ivars until after `[super dealloc]`. - // https://github.com/flutter/flutter/issues/87247 [_children release]; + _parent = nil; + _container.get().semanticsObject = nil; [_platformViewSemanticsContainer release]; + _inDealloc = YES; + [super dealloc]; } #pragma mark - Semantic object property accesser @@ -437,11 +436,7 @@ - (BOOL)hasChildren { if (_node.IsPlatformViewNode()) { return YES; } - if (_children) { - return [self.children count] != 0; - } else { - return NO; - } + return [self.children count] != 0; } #pragma mark - Semantic object method @@ -694,6 +689,14 @@ - (void)setAccessibilityContainer:(id)container { } - (id)accessibilityContainer { + if (_inDealloc) { + // In iOS9, `accessibilityContainer` will be called by `[UIAccessibilityElementSuperCategory + // dealloc]` during `[super dealloc]`. And will crash when accessing `_children` which has + // called `[_children release]` in `[SemanticsObject dealloc]`. + // https://github.com/flutter/flutter/issues/87247 + return nil; + } + if (![self isAccessibilityBridgeAlive]) { return nil; } From 9a18ac382eddf69a83b625942f3713f1863d8e66 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley <44207000+xiaoxiaowesley@users.noreply.github.com> Date: Fri, 6 Aug 2021 09:38:40 +0800 Subject: [PATCH 6/8] Update shell/platform/darwin/ios/framework/Source/SemanticsObject.mm Co-authored-by: Jenn Magder --- shell/platform/darwin/ios/framework/Source/SemanticsObject.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index bf64ddccdcbac..a2ff91ff5c641 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -400,7 +400,6 @@ - (instancetype)initWithBridge:(fml::WeakPtr)br _bridge = bridge; _uid = uid; _children = [[NSMutableArray alloc] init]; - _inDealloc = NO; } return self; From 895d366212ba6a59073a93fe2c594a72b272fd58 Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley Date: Fri, 6 Aug 2021 10:15:36 +0800 Subject: [PATCH 7/8] add comment to describe test fail situation --- .../darwin/ios/framework/Source/SemanticsObjectTest.mm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index 7641c8c49b1b6..ce2a19ddfe44b 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -651,6 +651,10 @@ - (void)testSemanticsObjectDeallocated { SemanticsObject* parent = [[SemanticsObject alloc] initWithBridge:bridge uid:0]; SemanticsObject* child = [[SemanticsObject alloc] initWithBridge:bridge uid:1]; parent.children = @[ child ]; + // In some iOS 9 physical devices it will crash when access released properties during + // `[super dealloc]` and this test will no pass + // https://github.com/flutter/flutter/issues/66032 + // https://github.com/flutter/engine/pull/27786 __weak SemanticsObject* weakObject = parent; parent = nil; XCTAssertNil(weakObject); From 13a7b56b786a0604d01eafc12111ac691b80159c Mon Sep 17 00:00:00 2001 From: xiaoxiaowesley <44207000+xiaoxiaowesley@users.noreply.github.com> Date: Fri, 6 Aug 2021 10:25:22 +0800 Subject: [PATCH 8/8] Update shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm Co-authored-by: Jenn Magder --- .../darwin/ios/framework/Source/SemanticsObjectTest.mm | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index ce2a19ddfe44b..f6475beff2a1a 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -651,10 +651,8 @@ - (void)testSemanticsObjectDeallocated { SemanticsObject* parent = [[SemanticsObject alloc] initWithBridge:bridge uid:0]; SemanticsObject* child = [[SemanticsObject alloc] initWithBridge:bridge uid:1]; parent.children = @[ child ]; - // In some iOS 9 physical devices it will crash when access released properties during - // `[super dealloc]` and this test will no pass + // Validate SemanticsObject deallocation does not crash. // https://github.com/flutter/flutter/issues/66032 - // https://github.com/flutter/engine/pull/27786 __weak SemanticsObject* weakObject = parent; parent = nil; XCTAssertNil(weakObject);