iOS: Eliminate FlutterEngine dealloc notification (flutter/engine#56650)

FlutterEngineGroup keeps an array of all live FlutterEngine instances it has created such that after the first engine, it can spawn further engines using the first.

Previously we manually managed this array by adding engines to it upon creation, then having [FlutterEngine dealloc] emit a notification such that FlutterEngineGroup can listen for it, and remove instances from the array upon reception of the notification.

Instead, we now use an NSPointerArray of of weak pointers such that pointers are automatically nilled out by ARC after the last strong reference is collected. This eliminates the need for the manual notification during dealloc.

Unfortunately, NSPointerArray is "clever" and assumes that if the array has not been mutated to store a nil pointer since its last compact call, it must not contain a nil pointer and can thus skip compaction. Under ARC, weak pointers are automatically nilled out when the last strong reference is released, so the above heuristic is no longer valid. We work around the issue by storing a nil pointer before calling compact.

See http://www.openradar.me/15396578 for the radar tracking this bug.

I'm not thrilled with the fact that we're replacing one sort of TODO with another, but the code is much simpler and avoids relying on a trip through the notification center, which seems objectively better.

Issue: https://github.com/flutter/flutter/issues/155943
Issue: https://github.com/flutter/flutter/issues/137801

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

@@ -84,7 +84,6 @@ NSString* const FlutterDefaultInitialRoute = nil;
 
 #pragma mark - Internal constants
 
-NSString* const kFlutterEngineWillDealloc = @"FlutterEngineWillDealloc";
 NSString* const kFlutterKeyDataChannel = @"flutter/keydata";
 static constexpr int kNumProfilerSamplesPerSec = 5;
 
@@ -282,10 +281,6 @@ static constexpr int kNumProfilerSamplesPerSec = 5;
     }
   }];
 
-  [[NSNotificationCenter defaultCenter] postNotificationName:kFlutterEngineWillDealloc
-                                                      object:self
-                                                    userInfo:nil];
-
   // nil out weak references.
   // TODO(cbracken): https://github.com/flutter/flutter/issues/156222
   // Ensure that FlutterEngineRegistrar is using weak pointers, then eliminate this code.

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineGroup.mm

@@ -12,7 +12,7 @@ FLUTTER_ASSERT_ARC
 
 @interface FlutterEngineGroup ()
 @property(nonatomic, copy) NSString* name;
-@property(nonatomic, strong) NSMutableArray<NSValue*>* engines;
+@property(nonatomic, strong) NSPointerArray* engines;
 @property(nonatomic, copy) FlutterDartProject* project;
 @property(nonatomic, assign) NSUInteger enginesCreatedCount;
 @end
@@ -23,7 +23,7 @@ FLUTTER_ASSERT_ARC
   self = [super init];
   if (self) {
     _name = [name copy];
-    _engines = [[NSMutableArray<NSValue*> alloc] init];
+    _engines = [NSPointerArray weakObjectsPointerArray];
     _project = project;
   }
   return self;
@@ -51,6 +51,12 @@ FLUTTER_ASSERT_ARC
   NSArray<NSString*>* entrypointArgs = options.entrypointArgs;
 
   FlutterEngine* engine;
+  // NSPointerArray is clever and assumes that unless a mutation operation has occurred on it that
+  // has set one of its values to nil, nothing could have changed and it can skip compaction.
+  // That's reasonable behaviour on a regular NSPointerArray but not for a weakObjectPointerArray.
+  // As a workaround, we mutate it first. See: http://www.openradar.me/15396578
+  [self.engines addPointer:nil];
+  [self.engines compact];
   if (self.engines.count <= 0) {
     engine = [self makeEngine];
     [engine runWithEntrypoint:entrypoint
@@ -58,20 +64,13 @@ FLUTTER_ASSERT_ARC
                  initialRoute:initialRoute
                entrypointArgs:entrypointArgs];
   } else {
-    FlutterEngine* spawner = (FlutterEngine*)[self.engines[0] pointerValue];
+    FlutterEngine* spawner = (__bridge FlutterEngine*)[self.engines pointerAtIndex:0];
     engine = [spawner spawnWithEntrypoint:entrypoint
                                libraryURI:libraryURI
                              initialRoute:initialRoute
                            entrypointArgs:entrypointArgs];
   }
-  // TODO(cbracken): https://github.com/flutter/flutter/issues/155943
+  [self.engines addPointer:(__bridge void*)engine];
-  [self.engines addObject:[NSValue valueWithPointer:(__bridge void*)engine]];
-
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center addObserver:self
-             selector:@selector(onEngineWillBeDealloced:)
-                 name:kFlutterEngineWillDealloc
-               object:engine];
 
   return engine;
 }
@@ -82,9 +81,4 @@ FLUTTER_ASSERT_ARC
   return [[FlutterEngine alloc] initWithName:engineName project:self.project];
 }
 
-- (void)onEngineWillBeDealloced:(NSNotification*)notification {
-  // TODO(cbracken): https://github.com/flutter/flutter/issues/155943
-  [self.engines removeObject:[NSValue valueWithPointer:(__bridge void*)notification.object]];
-}
-
 @end

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineGroupTest.mm

@@ -37,11 +37,16 @@ FLUTTER_ASSERT_ARC
 }
 
 - (void)testDeleteLastEngine {
+  __weak FlutterEngine* weakSpawner;
   FlutterEngineGroup* group = [[FlutterEngineGroup alloc] initWithName:@"foo" project:nil];
   @autoreleasepool {
     FlutterEngine* spawner = [group makeEngineWithEntrypoint:nil libraryURI:nil];
+    weakSpawner = spawner;
     XCTAssertNotNil(spawner);
+    XCTAssertNotNil(weakSpawner);
   }
+  XCTAssertNil(weakSpawner);
+
   FlutterEngine* spawnee = [group makeEngineWithEntrypoint:nil libraryURI:nil];
   XCTAssertNotNil(spawnee);
 }

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

@@ -262,23 +262,6 @@ FLUTTER_ASSERT_ARC
   XCTAssertNotNil(spawn);
 }
 
-- (void)testDeallocNotification {
-  XCTestExpectation* deallocNotification = [self expectationWithDescription:@"deallocNotification"];
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  id<NSObject> observer;
-  @autoreleasepool {
-    FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar"];
-    observer = [center addObserverForName:kFlutterEngineWillDealloc
-                                   object:engine
-                                    queue:[NSOperationQueue mainQueue]
-                               usingBlock:^(NSNotification* note) {
-                                 [deallocNotification fulfill];
-                               }];
-  }
-  [self waitForExpectations:@[ deallocNotification ]];
-  [center removeObserver:observer];
-}
-
 - (void)testSetHandlerAfterRun {
   FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar"];
   XCTestExpectation* gotMessage = [self expectationWithDescription:@"gotMessage"];

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h

@@ -29,8 +29,6 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-extern NSString* const kFlutterEngineWillDealloc;
-
 @interface FlutterEngine () <FlutterViewEngineDelegate>
 
 - (flutter::Shell&)shell;

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Test.h

@@ -11,8 +11,6 @@
 #import "flutter/shell/platform/darwin/ios/rendering_api_selection.h"
 #include "flutter/shell/platform/embedder/embedder.h"
 
-extern NSString* const kFlutterEngineWillDealloc;
-
 @class FlutterBinaryMessengerRelay;
 
 namespace flutter {