From 5fa39ff1423caa4691a769f5701b9deabc1dd56e Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 18 Jul 2023 10:31:43 -0700 Subject: [PATCH] Check FlutterAppDelegate selector support before calling (flutter/engine#43425) ## Description This adds checks for the app delegate to make sure that it supports the Flutter-specific selectors before calling them, so that a non `FlutterAppDelegate` can be used for the `NSApplicationDelegate` on `NSApp`. ## Related Issues - https://github.com/flutter/flutter/issues/124829 - https://github.com/flutter/flutter/issues/127476 ## Tests - Added a test to make sure things don't crash if the app delegate isn't a `FlutterAppDelegate`. --- .../macos/framework/Source/FlutterEngine.mm | 39 +++++++++++++------ .../framework/Source/FlutterEngineTest.mm | 26 +++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index caab9c1852..f6764167de 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -178,9 +178,11 @@ constexpr char kTextPlainFormat[] = "text/plain"; // allow tests to override it so that an actual exit doesn't occur. [[NSApplication sharedApplication] terminate:sender]; }; - FlutterAppDelegate* appDelegate = - (FlutterAppDelegate*)[[NSApplication sharedApplication] delegate]; - appDelegate.terminationHandler = self; + id appDelegate = [[NSApplication sharedApplication] delegate]; + if ([appDelegate respondsToSelector:@selector(setTerminationHandler:)]) { + FlutterAppDelegate* flutterAppDelegate = reinterpret_cast(appDelegate); + flutterAppDelegate.terminationHandler = self; + } return self; } @@ -420,10 +422,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi _semanticsEnabled = NO; _isResponseValid = [[NSMutableArray alloc] initWithCapacity:1]; [_isResponseValid addObject:@YES]; - _terminationHandler = [[FlutterEngineTerminationHandler alloc] initWithEngine:self - terminator:nil]; // kFlutterImplicitViewId is reserved for the implicit view. - // All IDs above it are for regular views. _nextViewId = kFlutterImplicitViewId + 1; _embedderAPI.struct_size = sizeof(FlutterEngineProcTable); @@ -443,9 +442,16 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi [self setUpPlatformViewChannel]; [self setUpAccessibilityChannel]; [self setUpNotificationCenterListeners]; - FlutterAppDelegate* appDelegate = - reinterpret_cast([[NSApplication sharedApplication] delegate]); - [appDelegate addApplicationLifecycleDelegate:self]; + id appDelegate = [[NSApplication sharedApplication] delegate]; + const SEL selector = @selector(addApplicationLifecycleDelegate:); + if ([appDelegate respondsToSelector:selector]) { + _terminationHandler = [[FlutterEngineTerminationHandler alloc] initWithEngine:self + terminator:nil]; + FlutterAppDelegate* flutterAppDelegate = reinterpret_cast(appDelegate); + [flutterAppDelegate addApplicationLifecycleDelegate:self]; + } else { + _terminationHandler = nil; + } return self; } @@ -1097,9 +1103,20 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi } else if ([call.method isEqualToString:@"Clipboard.hasStrings"]) { result(@{@"value" : @([self clipboardHasStrings])}); } else if ([call.method isEqualToString:@"System.exitApplication"]) { - [[self terminationHandler] handleRequestAppExitMethodCall:call.arguments result:result]; + if ([self terminationHandler] == nil) { + // If the termination handler isn't set, then either we haven't + // initialized it yet, or (more likely) the NSApp delegate isn't a + // FlutterAppDelegate, so it can't cancel requests to exit. So, in that + // case, just terminate when requested. + [NSApp terminate:self]; + result(nil); + } else { + [[self terminationHandler] handleRequestAppExitMethodCall:call.arguments result:result]; + } } else if ([call.method isEqualToString:@"System.initializationComplete"]) { - [self terminationHandler].acceptingRequests = YES; + if ([self terminationHandler] != nil) { + [self terminationHandler].acceptingRequests = YES; + } result(nil); } else { result(FlutterMethodNotImplemented); diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 4fccfa0ded..fea57a9adf 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -47,6 +47,16 @@ constexpr int64_t kImplicitViewId = 0ll; @end +@interface PlainAppDelegate : NSObject +@end + +@implementation PlainAppDelegate +- (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnull)sender { + // Always cancel, so that the test doesn't exit. + return NSTerminateCancel; +} +@end + namespace flutter::testing { TEST_F(FlutterEngineTest, CanLaunch) { @@ -782,6 +792,22 @@ TEST_F(FlutterEngineTest, HandlesTerminationRequest) { EXPECT_TRUE(triedToTerminate); } +TEST_F(FlutterEngineTest, IgnoresTerminationRequestIfNotFlutterAppDelegate) { + id previousDelegate = [[NSApplication sharedApplication] delegate]; + id plainDelegate = [[PlainAppDelegate alloc] init]; + [NSApplication sharedApplication].delegate = plainDelegate; + + // Creating the engine shouldn't fail here, even though the delegate isn't a + // FlutterAppDelegate. + CreateMockFlutterEngine(nil); + + // Asking to terminate the app should cancel. + EXPECT_EQ([[[NSApplication sharedApplication] delegate] applicationShouldTerminate:NSApp], + NSTerminateCancel); + + [NSApplication sharedApplication].delegate = previousDelegate; +} + TEST_F(FlutterEngineTest, HandleAccessibilityEvent) { __block BOOL announced = NO; id engineMock = CreateMockFlutterEngine(nil);