Allow macOS plugins to register as app delegates (flutter#44587)

Adds `addApplicationDelegate:` to the macOS plugin registrar, following the corresponding iOS method, and wires it up to the existing app delegation forwarding that was recently added for use at the application level. (The actual delegate is non-trivially different between iOS and macOS, but that's not resolveable without a complex migration on the iOS side, so the APIs currently diverge after the level of the `addApplicationDelegate:` method itself.)

This doesn't add any new methods to the delegation; those will be added in a follow-up PR.

Also fixes a retain cycle in the termination handler that prevented the new test from working.

Most of flutter/flutter#41471

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

Author: stuartmorgan-g

shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.mm

@@ -14,7 +14,7 @@ @implementation FlutterBinaryMessengerRelay
 - (instancetype)initWithParent:(NSObject<FlutterBinaryMessenger>*)parent {
   self = [super init];
   if (self != nil) {
-    self.parent = parent;
+    _parent = parent;
   }
   return self;
 }

shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h

@@ -10,6 +10,30 @@
 #import "FlutterAppLifecycleDelegate.h"
 #import "FlutterMacros.h"
 
+/**
+ * A protocol to be implemented by the `NSApplicationDelegate` of an application to enable the
+ * Flutter framework and any Flutter plugins to register to receive application life cycle events.
+ *
+ * Implementers should forward all of the `NSApplicationDelegate` methods corresponding to the
+ * handlers in FlutterAppLifecycleDelegate to any registered delegates.
+ */
+FLUTTER_DARWIN_EXPORT
+@protocol FlutterAppLifecycleProvider <NSObject>
+
+/**
+ * Adds an object implementing |FlutterAppLifecycleDelegate| to the list of
+ * delegates to be informed of application lifecycle events.
+ */
+- (void)addApplicationLifecycleDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate;
+
+/**
+ * Removes an object implementing |FlutterAppLifecycleDelegate| to the list of
+ * delegates to be informed of application lifecycle events.
+ */
+- (void)removeApplicationLifecycleDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate;
+
+@end
+
 /**
  * |NSApplicationDelegate| subclass for simple apps that want default behavior.
  *
@@ -20,13 +44,14 @@
  *   * Updates the main Flutter window's title to match the name in the app's Info.plist.
  *     |mainFlutterWindow| must be set before the application finishes launching for this to take
  *     effect.
+ *   * Forwards `NSApplicationDelegate` callbacks to plugins that register for them.
  *
  * App delegates for Flutter applications are *not* required to inherit from
  * this class. Developers of custom app delegate classes should copy and paste
  * code as necessary from FlutterAppDelegate.mm.
  */
 FLUTTER_DARWIN_EXPORT
-@interface FlutterAppDelegate : NSObject <NSApplicationDelegate>
+@interface FlutterAppDelegate : NSObject <NSApplicationDelegate, FlutterAppLifecycleProvider>
 
 /**
  * The application menu in the menu bar.
@@ -39,18 +64,6 @@ FLUTTER_DARWIN_EXPORT
  */
 @property(weak, nonatomic) IBOutlet NSWindow* mainFlutterWindow;
 
-/**
- * Adds an object implementing |FlutterAppLifecycleDelegate| to the list of
- * delegates to be informed of application lifecycle events.
- */
-- (void)addApplicationLifecycleDelegate:(NSObject<FlutterAppLifecycleDelegate>*)delegate;
-
-/**
- * Removes an object implementing |FlutterAppLifecycleDelegate| to the list of
- * delegates to be informed of application lifecycle events.
- */
-- (void)removeApplicationLifecycleDelegate:(NSObject<FlutterAppLifecycleDelegate>*)delegate;
-
 @end
 
 #endif  // FLUTTER_FLUTTERAPPDELEGATE_H_

shell/platform/darwin/macos/framework/Headers/FlutterAppLifecycleDelegate.h

@@ -9,7 +9,6 @@
 #include <Foundation/Foundation.h>
 
 #import "FlutterMacros.h"
-#import "FlutterPluginMacOS.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -18,6 +17,7 @@ NS_ASSUME_NONNULL_BEGIN
  * Protocol for listener of lifecycle events from the NSApplication, typically a
  * FlutterPlugin.
  */
+FLUTTER_DARWIN_EXPORT
 @protocol FlutterAppLifecycleDelegate <NSObject>
 
 @optional

shell/platform/darwin/macos/framework/Headers/FlutterPluginMacOS.h

@@ -4,6 +4,7 @@
 
 #import <Foundation/Foundation.h>
 
+#import "FlutterAppLifecycleDelegate.h"
 #import "FlutterChannels.h"
 #import "FlutterCodecs.h"
 #import "FlutterMacros.h"
@@ -22,7 +23,7 @@ NS_ASSUME_NONNULL_BEGIN
  * expand over time to more closely match the functionality of the iOS FlutterPlugin.
  */
 FLUTTER_DARWIN_EXPORT
-@protocol FlutterPlugin <NSObject>
+@protocol FlutterPlugin <NSObject, FlutterAppLifecycleDelegate>
 
 /**
  * Creates an instance of the plugin to register with |registrar| using the desired

shell/platform/darwin/macos/framework/Headers/FlutterPluginRegistrarMacOS.h

@@ -52,6 +52,13 @@ FLUTTER_DARWIN_EXPORT
 - (void)addMethodCallDelegate:(nonnull id<FlutterPlugin>)delegate
                       channel:(nonnull FlutterMethodChannel*)channel;
 
+/**
+ * Registers the plugin as a receiver of `NSApplicationDelegate` calls.
+ *
+ * @param delegate The receiving object, such as the plugin's main class.
+ */
+- (void)addApplicationDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate;
+
 /**
  * Registers a `FlutterPlatformViewFactory` for creation of platform views.
  *

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

@@ -90,6 +90,11 @@ @interface FlutterEngine () <FlutterBinaryMessenger>
  */
 @property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;
 
+/**
+ * All delegates added via plugin calls to addApplicationDelegate.
+ */
+@property(nonatomic, strong) NSPointerArray* pluginAppDelegates;
+
 - (nullable FlutterViewController*)viewControllerForId:(FlutterViewId)viewId;
 
 /**
@@ -165,7 +170,7 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result;
 #pragma mark -
 
 @implementation FlutterEngineTerminationHandler {
-  FlutterEngine* _engine;
+  __weak FlutterEngine* _engine;
   FlutterTerminationCallback _terminator;
 }
 
@@ -320,6 +325,16 @@ - (void)addMethodCallDelegate:(nonnull id<FlutterPlugin>)delegate
   }];
 }
 
+- (void)addApplicationDelegate:(NSObject<FlutterAppLifecycleDelegate>*)delegate {
+  id<NSApplicationDelegate> appDelegate = [[NSApplication sharedApplication] delegate];
+  if ([appDelegate conformsToProtocol:@protocol(FlutterAppLifecycleProvider)]) {
+    id<FlutterAppLifecycleProvider> lifeCycleProvider =
+        static_cast<id<FlutterAppLifecycleProvider>>(appDelegate);
+    [lifeCycleProvider addApplicationLifecycleDelegate:delegate];
+    [_flutterEngine.pluginAppDelegates addPointer:(__bridge void*)delegate];
+  }
+}
+
 - (void)registerViewFactory:(nonnull NSObject<FlutterPlatformViewFactory>*)factory
                      withId:(nonnull NSString*)factoryId {
   [[_flutterEngine platformViewController] registerViewFactory:factory withId:factoryId];
@@ -421,6 +436,8 @@ - (instancetype)initWithName:(NSString*)labelPrefix
   _visible = NO;
   _project = project ?: [[FlutterDartProject alloc] init];
   _messengerHandlers = [[NSMutableDictionary alloc] init];
+  _binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
+  _pluginAppDelegates = [NSPointerArray weakObjectsPointerArray];
   _currentMessengerConnection = 1;
   _allowHeadlessExecution = allowHeadlessExecution;
   _semanticsEnabled = NO;
@@ -448,12 +465,12 @@ - (instancetype)initWithName:(NSString*)labelPrefix
   [self setUpAccessibilityChannel];
   [self setUpNotificationCenterListeners];
   id<NSApplicationDelegate> appDelegate = [[NSApplication sharedApplication] delegate];
-  const SEL selector = @selector(addApplicationLifecycleDelegate:);
-  if ([appDelegate respondsToSelector:selector]) {
+  if ([appDelegate conformsToProtocol:@protocol(FlutterAppLifecycleProvider)]) {
     _terminationHandler = [[FlutterEngineTerminationHandler alloc] initWithEngine:self
                                                                        terminator:nil];
-    FlutterAppDelegate* flutterAppDelegate = reinterpret_cast<FlutterAppDelegate*>(appDelegate);
-    [flutterAppDelegate addApplicationLifecycleDelegate:self];
+    id<FlutterAppLifecycleProvider> lifecycleProvider =
+        static_cast<id<FlutterAppLifecycleProvider>>(appDelegate);
+    [lifecycleProvider addApplicationLifecycleDelegate:self];
   } else {
     _terminationHandler = nil;
   }
@@ -462,10 +479,20 @@ - (instancetype)initWithName:(NSString*)labelPrefix
 }
 
 - (void)dealloc {
-  FlutterAppDelegate* appDelegate =
-      reinterpret_cast<FlutterAppDelegate*>([[NSApplication sharedApplication] delegate]);
-  if (appDelegate != nil) {
-    [appDelegate removeApplicationLifecycleDelegate:self];
+  id<NSApplicationDelegate> appDelegate = [[NSApplication sharedApplication] delegate];
+  if ([appDelegate conformsToProtocol:@protocol(FlutterAppLifecycleProvider)]) {
+    id<FlutterAppLifecycleProvider> lifecycleProvider =
+        static_cast<id<FlutterAppLifecycleProvider>>(appDelegate);
+    [lifecycleProvider removeApplicationLifecycleDelegate:self];
+
+    // Unregister any plugins that registered as app delegates, since they are not guaranteed to
+    // live after the engine is destroyed, and their delegation registration is intended to be bound
+    // to the engine and its lifetime.
+    for (id<FlutterAppLifecycleDelegate> delegate in _pluginAppDelegates) {
+      if (delegate) {
+        [lifecycleProvider removeApplicationLifecycleDelegate:delegate];
+      }
+    }
   }
   @synchronized(_isResponseValid) {
     [_isResponseValid removeAllObjects];

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

@@ -7,15 +7,19 @@
 
 #include <objc/objc.h>
 
+#include <algorithm>
 #include <functional>
 #include <thread>
+#include <vector>
 
 #include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/lib/ui/window/platform_message.h"
 #include "flutter/shell/platform/common/accessibility_bridge.h"
 #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h"
 #import "flutter/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h"
 #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h"
+#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppLifecycleDelegate.h"
+#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterPluginMacOS.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestUtils.h"
 #include "flutter/shell/platform/embedder/embedder.h"
@@ -59,6 +63,60 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnu
 }
 @end
 
+#pragma mark -
+
+@interface FakeLifecycleProvider : NSObject <FlutterAppLifecycleProvider, NSApplicationDelegate>
+
+@property(nonatomic, strong, readonly) NSPointerArray* registeredDelegates;
+
+// True if the given delegate is currently registered.
+- (BOOL)hasDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate;
+@end
+
+@implementation FakeLifecycleProvider {
+  /**
+   * All currently registered delegates.
+   *
+   * This does not use NSPointerArray or any other weak-pointer
+   * system, because a weak pointer will be nil'd out at the start of dealloc, which will break
+   * queries. E.g., if a delegate is dealloc'd without being unregistered, a weak pointer array
+   * would no longer contain that pointer even though removeApplicationLifecycleDelegate: was never
+   * called, causing tests to pass incorrectly.
+   */
+  std::vector<void*> _delegates;
+}
+
+- (void)addApplicationLifecycleDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate {
+  _delegates.push_back((__bridge void*)delegate);
+}
+
+- (void)removeApplicationLifecycleDelegate:
+    (nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate {
+  auto delegateIndex = std::find(_delegates.begin(), _delegates.end(), (__bridge void*)delegate);
+  NSAssert(delegateIndex != _delegates.end(),
+           @"Attempting to unregister a delegate that was not registered.");
+  _delegates.erase(delegateIndex);
+}
+
+- (BOOL)hasDelegate:(nonnull NSObject<FlutterAppLifecycleDelegate>*)delegate {
+  return std::find(_delegates.begin(), _delegates.end(), (__bridge void*)delegate) !=
+         _delegates.end();
+}
+
+@end
+
+#pragma mark -
+
+@interface FakeAppDelegatePlugin : NSObject <FlutterPlugin>
+@end
+
+@implementation FakeAppDelegatePlugin
++ (void)registerWithRegistrar:(id<FlutterPluginRegistrar>)registrar {
+}
+@end
+
+#pragma mark -
+
 namespace flutter::testing {
 
 TEST_F(FlutterEngineTest, CanLaunch) {
@@ -515,7 +573,7 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnu
                ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
     FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test"
                                                         project:project
-                                         allowHeadlessExecution:true];
+                                         allowHeadlessExecution:YES];
     weakEngine = engine;
     binaryMessenger = engine.binaryMessenger;
   }
@@ -539,7 +597,7 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnu
                ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
     FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test"
                                                         project:project
-                                         allowHeadlessExecution:true];
+                                         allowHeadlessExecution:YES];
     id<FlutterPluginRegistrar> registrar = [engine registrarForPlugin:@"MyPlugin"];
     textureRegistry = registrar.textures;
   }
@@ -956,6 +1014,42 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnu
   [mockApplication stopMocking];
 }
 
+TEST_F(FlutterEngineTest, ForwardsPluginDelegateRegistration) {
+  id<NSApplicationDelegate> previousDelegate = [[NSApplication sharedApplication] delegate];
+  FakeLifecycleProvider* fakeAppDelegate = [[FakeLifecycleProvider alloc] init];
+  [NSApplication sharedApplication].delegate = fakeAppDelegate;
+
+  FakeAppDelegatePlugin* plugin = [[FakeAppDelegatePlugin alloc] init];
+  FlutterEngine* engine = CreateMockFlutterEngine(nil);
+
+  [[engine registrarForPlugin:@"TestPlugin"] addApplicationDelegate:plugin];
+
+  EXPECT_TRUE([fakeAppDelegate hasDelegate:plugin]);
+
+  [NSApplication sharedApplication].delegate = previousDelegate;
+}
+
+TEST_F(FlutterEngineTest, UnregistersPluginsOnEngineDestruction) {
+  id<NSApplicationDelegate> previousDelegate = [[NSApplication sharedApplication] delegate];
+  FakeLifecycleProvider* fakeAppDelegate = [[FakeLifecycleProvider alloc] init];
+  [NSApplication sharedApplication].delegate = fakeAppDelegate;
+
+  FakeAppDelegatePlugin* plugin = [[FakeAppDelegatePlugin alloc] init];
+
+  @autoreleasepool {
+    FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil];
+
+    [[engine registrarForPlugin:@"TestPlugin"] addApplicationDelegate:plugin];
+    EXPECT_TRUE([fakeAppDelegate hasDelegate:plugin]);
+  }
+
+  // When the engine is released, it should unregister any plugins it had
+  // registered on its behalf.
+  EXPECT_FALSE([fakeAppDelegate hasDelegate:plugin]);
+
+  [NSApplication sharedApplication].delegate = previousDelegate;
+}
+
 }  // namespace flutter::testing
 
 // NOLINTEND(clang-analyzer-core.StackAddressEscape)