iOS: Eliminate FlutterEngine dealloc notification

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 dealloc.

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 must contain a nil pointer and can thus skip compaction. When
holding weak pointers under ARC, this is no longer the case and so we
work around the issue by storing a nil pointer before calling compact.

See: http://www.openradar.me/15396578

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: flutter/flutter#155943

Author: cbracken

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

@@ -84,7 +84,6 @@ static void IOSPlatformThreadConfigSetter(const fml::Thread::ThreadConfig& confi
 
 #pragma mark - Internal constants
 
-NSString* const kFlutterEngineWillDealloc = @"FlutterEngineWillDealloc";
 NSString* const kFlutterKeyDataChannel = @"flutter/keydata";
 static constexpr int kNumProfilerSamplesPerSec = 5;
 
@@ -282,10 +281,6 @@ - (void)dealloc {
     }
   }];
 
-  [[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.

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

@@ -12,7 +12,7 @@ @implementation FlutterEngineGroupOptions
 
 @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 @@ - (instancetype)initWithName:(NSString*)name project:(nullable FlutterDartProjec
   self = [super init];
   if (self) {
     _name = [name copy];
-    _engines = [[NSMutableArray<NSValue*> alloc] init];
+    _engines = [NSPointerArray weakObjectsPointerArray];
     _project = project;
   }
   return self;
@@ -51,27 +51,26 @@ - (FlutterEngine*)makeEngineWithOptions:(nullable FlutterEngineGroupOptions*)opt
   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
                    libraryURI:libraryURI
                  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 addObject:[NSValue valueWithPointer:(__bridge void*)engine]];
-
-  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-  [center addObserver:self
-             selector:@selector(onEngineWillBeDealloced:)
-                 name:kFlutterEngineWillDealloc
-               object:engine];
+  [self.engines addPointer:(__bridge void*)engine];
 
   return engine;
 }
@@ -82,9 +81,4 @@ - (FlutterEngine*)makeEngine {
   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

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

@@ -37,11 +37,16 @@ - (void)testSpawn {
 }
 
 - (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);
 }

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

@@ -262,23 +262,6 @@ - (void)testSpawn {
   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"];

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;

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 {