Retain the parent of a SemanticsObject and implement proper dealloc (f…

…lutter#3740)

* Retain the parent of a SemanticsObject

This fixes the last known crash in iOS accessibility.

* rework memory management

* review comments

shell/platform/darwin/ios/framework/Source/accessibility_bridge.h

@@ -10,6 +10,7 @@
 #include <unordered_set>
 #include <vector>
 
+#include "flutter/fml/platform/darwin/scoped_nsobject.h"
 #include "flutter/lib/ui/semantics/semantics_node.h"
 #include "flutter/shell/platform/darwin/ios/framework/Source/FlutterView.h"
 #include "lib/ftl/macros.h"
@@ -31,7 +32,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, strong) SemanticsObject* parent;
 
 - (instancetype)init __attribute__((unavailable("Use initWithBridge instead")));
 - (instancetype)initWithBridge:(shell::AccessibilityBridge*)bridge
@@ -54,12 +55,12 @@ class AccessibilityBridge final {
 
  private:
   SemanticsObject* GetOrCreateObject(int32_t id);
-  void VisitObjectsRecursively(SemanticsObject* object, std::unordered_set<int>* visited_objects);
+  void VisitObjectsRecursivelyAndRemove(SemanticsObject* object, NSMutableArray<NSNumber*>* doomed_uids);
   void ReleaseObjects(std::unordered_map<int, SemanticsObject*>& objects);
 
   UIView* view_;
   PlatformViewIOS* platform_view_;
-  std::unordered_map<int, SemanticsObject*> objects_;
+  fml::scoped_nsobject<NSMutableDictionary<NSNumber*, SemanticsObject*>> objects_;
 
   FTL_DISALLOW_COPY_AND_ASSIGN(AccessibilityBridge);
 };

shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm

@@ -79,10 +79,11 @@ - (void)setSemanticsNode:(const blink::SemanticsNode*)node {
   return &_children;
 }
 
-- (void)neuter {
+- (void)dealloc {
   _bridge = nullptr;
   _children.clear();
-  self.parent = nil;
+  [_parent release];
+  [super dealloc];
 }
 
 #pragma mark - UIAccessibility overrides
@@ -218,12 +219,10 @@ - (BOOL)accessibilityPerformMagicTap {
 namespace shell {
 
 AccessibilityBridge::AccessibilityBridge(UIView* view, PlatformViewIOS* platform_view)
-    : view_(view), platform_view_(platform_view) {}
+    : view_(view), platform_view_(platform_view), objects_([[NSMutableDictionary alloc] init]) {}
 
 AccessibilityBridge::~AccessibilityBridge() {
   view_.accessibilityElements = nil;
-  ReleaseObjects(objects_);
-  objects_.clear();
 }
 
 void AccessibilityBridge::UpdateSemantics(std::vector<blink::SemanticsNode> nodes) {
@@ -240,7 +239,7 @@ - (BOOL)accessibilityPerformMagicTap {
     }
   }
 
-  SemanticsObject* root = objects_[kRootNodeId];
+  SemanticsObject* root = objects_.get()[@(kRootNodeId)];
 
   if (root) {
     if (!view_.accessibilityElements) {
@@ -249,30 +248,24 @@ - (BOOL)accessibilityPerformMagicTap {
   } else {
     view_.accessibilityElements = nil;
   }
-
-  std::unordered_set<int> visited_objects;
+
+  NSMutableArray<NSNumber*>* doomed_uids =
+    [NSMutableArray arrayWithArray: [objects_.get() allKeys]];
   if (root)
-    VisitObjectsRecursively(root, &visited_objects);
-
-  std::unordered_map<int, SemanticsObject*> doomed_objects;
-  doomed_objects.swap(objects_);
-  for (int uid : visited_objects) {
-    auto it = doomed_objects.find(uid);
-    objects_.insert(*it);
-    doomed_objects.erase(it);
-    // TODO(abarth): Use extract once we're at C++17.
-  }
+    VisitObjectsRecursivelyAndRemove(root, doomed_uids);
 
-  SemanticsObject* doomed_focused_object = nil;
-  for (const auto& entry : doomed_objects) {
-    SemanticsObject* object = entry.second;
+  bool focused_object_doomed = false;
+  for (NSNumber* uid in doomed_uids) {
+    SemanticsObject* object = objects_.get()[uid];
     if ([object accessibilityElementIsFocused]) {
-      doomed_focused_object = object;
+      focused_object_doomed = true;
       break;
     }
   }
 
-  if (doomed_focused_object != nil) {
+  [objects_ removeObjectsForKeys: doomed_uids];
+
+  if (focused_object_doomed) {
     // Previously focused element is no longer in the tree.
     // Passing `nil` as argument to let iOS figure out what to focus next.
     // TODO(goderbauer): Figure out which element should be focused next and post
@@ -282,37 +275,26 @@ - (BOOL)accessibilityPerformMagicTap {
     // Passing `nil` as argument to keep focus where it is.
     UIAccessibilityPostNotification(UIAccessibilityLayoutChangedNotification, nil);
   }
-
-  ReleaseObjects(doomed_objects);
 }
 
 void AccessibilityBridge::DispatchSemanticsAction(int32_t uid, blink::SemanticsAction action) {
   platform_view_->DispatchSemanticsAction(uid, action);
 }
 
 SemanticsObject* AccessibilityBridge::GetOrCreateObject(int32_t uid) {
-  SemanticsObject* object = objects_[uid];
+  SemanticsObject* object = objects_.get()[@(uid)];
   if (!object) {
-    object = [[SemanticsObject alloc] initWithBridge:this uid:uid];
-    objects_[uid] = object;
+    object = [[[SemanticsObject alloc] initWithBridge:this uid:uid] autorelease];
+    objects_.get()[@(uid)] = object;
   }
   return object;
 }
 
-void AccessibilityBridge::VisitObjectsRecursively(SemanticsObject* object,
-                                                  std::unordered_set<int>* visited_objects) {
-  visited_objects->insert(object.uid);
+void AccessibilityBridge::VisitObjectsRecursivelyAndRemove(SemanticsObject* object,
+                                                  NSMutableArray<NSNumber*>* doomed_uids) {
+  [doomed_uids removeObject: @(object.uid)];
   for (SemanticsObject* child : *[object children])
-    VisitObjectsRecursively(child, visited_objects);
-}
-
-void AccessibilityBridge::ReleaseObjects(std::unordered_map<int, SemanticsObject*>& objects) {
-  for (const auto& entry : objects) {
-    SemanticsObject* object = entry.second;
-    [object neuter];
-    [object release];
-  }
-  objects.clear();
+    VisitObjectsRecursivelyAndRemove(child, doomed_uids);
 }
 
 }  // namespace shell