Fix invalid access of weakly owned ptr in iOS a11y bridge (flutter#29456

)
nt4f04uNd · Nov 2, 2021 · d57627c · d57627c
1 parent 3e6c690
commit d57627c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 5 deletions.
diff --git a/shell/platform/darwin/ios/BUILD.gn b/shell/platform/darwin/ios/BUILD.gn
@@ -256,6 +256,7 @@ shared_library("ios_test_flutter") {
     "framework/Source/connection_collection_test.mm",
   ]
   deps = [
+    ":flutter_framework",
     ":flutter_framework_source",
     ":ios_gpu_configuration",
     ":ios_test_flutter_mrc",

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h
@@ -20,6 +20,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @interface FlutterSemanticsScrollView : UIScrollView
 
+@property(nonatomic, assign, nullable) SemanticsObject* semanticsObject;
+
 - (instancetype)init NS_UNAVAILABLE;
 - (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
 - (instancetype)initWithCoder:(NSCoder*)coder NS_UNAVAILABLE;

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm
@@ -6,10 +6,6 @@
 
 #import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h"
 
-@interface FlutterSemanticsScrollView ()
-@property(nonatomic, assign) SemanticsObject* semanticsObject;
-@end
-
 @implementation FlutterSemanticsScrollView
 
 - (instancetype)initWithSemanticsObject:(SemanticsObject*)semanticsObject {
@@ -40,7 +36,7 @@ - (BOOL)isAccessibilityElement {
       self.contentSize.height > self.frame.size.height) {
     // In SwitchControl or VoiceControl, the isAccessibilityElement must return YES
     // in order to use scroll actions.
-    return !_semanticsObject.bridge->isVoiceOverRunning();
+    return ![_semanticsObject bridge]->isVoiceOverRunning();
   } else {
     return NO;
   }

diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm
@@ -167,6 +167,7 @@ - (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)br
 
 - (void)dealloc {
   [_scrollView removeFromSuperview];
+  _scrollView.semanticsObject = nil;
   [_scrollView release];
   [super dealloc];
 }

diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
@@ -1573,4 +1573,42 @@ - (void)testAccessibilityMessageAfterDeletion {
   OCMVerify([messenger cleanUpConnection:connection]);
   [engine stopMocking];
 }
+
+- (void)testFlutterSemanticsScrollViewManagedObjectLifecycleCorrectly {
+  flutter::MockDelegate mock_delegate;
+  auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+  auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+      /*delegate=*/mock_delegate,
+      /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+      /*platform_views_controller=*/nil,
+      /*task_runners=*/runners);
+  id mockFlutterView = OCMClassMock([FlutterView class]);
+  id mockFlutterViewController = OCMClassMock([FlutterViewController class]);
+  OCMStub([mockFlutterViewController view]).andReturn(mockFlutterView);
+
+  auto ios_delegate = std::make_unique<flutter::MockIosDelegate>();
+  __block auto bridge =
+      std::make_unique<flutter::AccessibilityBridge>(/*view_controller=*/mockFlutterViewController,
+                                                     /*platform_view=*/platform_view.get(),
+                                                     /*platform_views_controller=*/nil,
+                                                     /*ios_delegate=*/std::move(ios_delegate));
+
+  FlutterSemanticsScrollView* flutterSemanticsScrollView;
+  @autoreleasepool {
+    FlutterScrollableSemanticsObject* semanticsObject =
+        [[[FlutterScrollableSemanticsObject alloc] initWithBridge:bridge->GetWeakPtr()
+                                                              uid:1234] autorelease];
+
+    flutterSemanticsScrollView = semanticsObject.nativeAccessibility;
+  }
+  XCTAssertTrue(flutterSemanticsScrollView);
+  // If the _semanticsObject is not a weak pointer this (or any other method on
+  // flutterSemanticsScrollView) will cause an EXC_BAD_ACCESS.
+  XCTAssertFalse([flutterSemanticsScrollView isAccessibilityElement]);
+}
 @end