From d7d60b451605a9231d7627dfb57e1f4ce99c536a Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Tue, 4 Apr 2023 14:09:38 -0700 Subject: [PATCH] [macOS] Handle termination in FlutterAppDelegate Moves application termination handling to FlutterAppDelegate. Previously, we required macOS applications using Flutter to ensure their main application class was FlutterApplication. Instead, we now do all handling in FlutterAppDelegate and FlutterEngine. There are two termination workflows to consider: * Termination requested from framework side: In this case, then engine receives a `System.exitApplication` method call, and starts the app termination workflow via `[FlutterEngine requestApplicationTermination:exitType:result]`. * Termination requested from macOS (e.g. Cmd-Q): In this case, `FlutterAppDelegate`'s `applicationShouldTerminate:` handler is invoked by AppKit, and the delegate starts the app termination workflow via `[FlutterEngine requestApplicationTermination:exitType:result]`. In either case, at this point, if the request is not cancellable, the app immediately exits. If it is cancellable, the embedder sends a `System.requestAppExit` method channel invocation to the framework, which responds with either `exit` or `cancel`. In the case of `exit` we immediately exit, otherwise we do nothing and the app continues running. This is a minor refactoring of the original approach we took in: https://github.com/flutter/engine/pull/39836 This does not remove the FlutterApplication class, since the framework migration from NSApplication to FlutterApplication still depends on it. A followup patch with replace the migration with a reverse migration will land, then FlutterApplication will be removed. Issue: https://github.com/flutter/flutter/issues/30735 --- ci/licenses_golden/licenses_flutter | 2 - .../framework/Source/FlutterAppDelegate.mm | 25 ++++-- .../framework/Source/FlutterApplication.mm | 82 ------------------- .../Source/FlutterApplication_Internal.h | 27 ------ .../macos/framework/Source/FlutterEngine.mm | 17 ++-- .../framework/Source/FlutterEngineTest.mm | 1 - .../framework/Source/FlutterEngine_Internal.h | 6 +- 7 files changed, 29 insertions(+), 131 deletions(-) delete mode 100644 shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 99c865f4d9fc1..fb265649ad9b8 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2618,7 +2618,6 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/Accessibil ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponder.h + ../../../flutter/LICENSE @@ -5214,7 +5213,6 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/Accessibilit FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm -FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.h FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterBackingStore.mm FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponder.h diff --git a/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm b/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm index 269d58f71a86b..5c5f444c6a196 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm @@ -42,13 +42,6 @@ - (void)applicationWillFinishLaunching:(NSNotification*)notification { } } -// This always returns NSTerminateNow, since by the time we get here, the -// application has already been asked if it should terminate or not, and if not, -// then termination never gets this far. -- (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication*)sender { - return NSTerminateNow; -} - #pragma mark Private Methods - (NSString*)applicationName { @@ -60,4 +53,22 @@ - (NSString*)applicationName { return applicationName; } +- (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnull)sender { + // If the framework has already told us to terminate, terminate immediately. + if ([[self terminationHandler] shouldTerminate]) { + return NSTerminateNow; + } + + // Send a termination request to the framework. + FlutterEngineTerminationHandler* terminationHandler = [self terminationHandler]; + [terminationHandler requestApplicationTermination:sender + exitType:kFlutterAppExitTypeCancelable + result:nil]; + + // Cancel termination to allow the framework to handle the request asynchronously. When the + // termination request returns from the app, if termination is desired, this method will be + // reinvoked with self.terminationHandler.shouldTerminate set to YES. + return NSTerminateCancel; +} + @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm b/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm index 62c3a65338914..8c9797192c23a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterApplication.mm @@ -3,88 +3,6 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterApplication.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" -#include "flutter/shell/platform/embedder/embedder.h" - -// An NSApplication subclass that implements overrides necessary for some -// Flutter features, like application lifecycle handling. @implementation FlutterApplication - -// Initialize NSApplication using the custom subclass. Check whether NSApp was -// already initialized using another class, because that would break some -// things. Warn about the mismatch only once, and only in debug builds. -+ (NSApplication*)sharedApplication { - NSApplication* app = [super sharedApplication]; - - // +sharedApplication initializes the global NSApp, so if we're delivering - // something other than a FlutterApplication, warn the developer once. -#ifndef FLUTTER_RELEASE - static dispatch_once_t onceToken = 0; - dispatch_once(&onceToken, ^{ - if (![app respondsToSelector:@selector(terminateApplication:)]) { - NSLog(@"NSApp should be of type %s, not %s.\n" - "System requests for the application to terminate will not be sent to " - "the Flutter framework, so the framework will be unable to cancel " - "those requests.\n" - "Modify the application's NSPrincipleClass to be %s in the " - "Info.plist to fix this.", - [[self className] UTF8String], [[NSApp className] UTF8String], - [[self className] UTF8String]); - } - }); -#endif // !FLUTTER_RELEASE - return app; -} - -// |terminate| is the entry point for orderly "quit" operations in Cocoa. This -// includes the application menu's Quit menu item and keyboard equivalent, the -// application's dock icon menu's Quit menu item, "quit" (not "force quit") in -// the Activity Monitor, and quits triggered by user logout and system restart -// and shutdown. -// -// We override the normal |terminate| implementation. Our implementation, which -// is specific to the asynchronous nature of Flutter, works by asking the -// application delegate to terminate using its |requestApplicationTermination| -// method instead of going through |applicationShouldTerminate|. -// -// The standard |applicationShouldTerminate| is not used because returning -// NSTerminateLater from that function moves the run loop into a modal dialog -// mode (NSModalPanelRunLoopMode), which stops the main run loop from processing -// messages like, for instance, the response to the method channel call, and -// code paths leading to it must be redirected to |requestApplicationTermination|. -// -// |requestApplicationTermination| differs from the standard -// |applicationShouldTerminate| in that no special event loop is run in the case -// that immediate termination is not possible (e.g., if dialog boxes allowing -// the user to cancel have to be shown, or data needs to be saved). Instead, -// requestApplicationTermination sends a method channel call to the framework asking -// it if it is OK to terminate. When that method channel call returns with a -// result, the application either terminates or continues running. -- (void)terminate:(id)sender { - FlutterAppDelegate* delegate = [self delegate]; - if (!delegate || ![delegate respondsToSelector:@selector(terminationHandler)] || - [delegate terminationHandler] == nil) { - // If there's no termination handler, then just terminate. - [super terminate:sender]; - } - FlutterEngineTerminationHandler* terminationHandler = - [static_cast([self delegate]) terminationHandler]; - [terminationHandler requestApplicationTermination:sender - exitType:kFlutterAppExitTypeCancelable - result:nil]; - // Return, don't exit. The application delegate is responsible for exiting on - // its own by calling |terminateApplication|. -} - -// Starts the regular Cocoa application termination flow, so that plugins will -// get the appropriate notifications after the application has already decided -// to quit. This is called after the application has decided that -// it's OK to terminate. -- (void)terminateApplication:(id)sender { - [super terminate:sender]; -} @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h deleted file mode 100644 index 0a79b225ba144..0000000000000 --- a/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef FLUTTER_FLUTTERAPPLICATION_INTERNAL_H_ -#define FLUTTER_FLUTTERAPPLICATION_INTERNAL_H_ - -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterApplication.h" - -/** - * Define |terminateApplication| for internal use. - */ -@interface FlutterApplication () - -/** - * FlutterApplication's implementation of |terminate| doesn't terminate the - * application: that is left up to the engine, which will call this function if - * it decides that termination request is granted, which will start the regular - * Cocoa flow for terminating the application, calling - * |applicationShouldTerminate|, etc. - * - * @param(sender) The id of the object requesting the termination, or nil. - */ -- (void)terminateApplication:(id)sender; -@end - -#endif // FLUTTER_FLUTTERAPPLICATION_INTERNAL_H_ diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 864049ff90452..913b9a64f3ae5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -13,9 +13,7 @@ #include "flutter/shell/platform/embedder/embedder.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h" -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterApplication.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterApplication_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin.h" @@ -176,15 +174,11 @@ - (instancetype)initWithEngine:(FlutterEngine*)engine _terminator = terminator ? terminator : ^(id sender) { // Default to actually terminating the application. The terminator exists to // allow tests to override it so that an actual exit doesn't occur. - FlutterApplication* flutterApp = [FlutterApplication sharedApplication]; - if (flutterApp && [flutterApp respondsToSelector:@selector(terminateApplication:)]) { - [[FlutterApplication sharedApplication] terminateApplication:sender]; - } else if (flutterApp) { - [flutterApp terminate:sender]; - } + NSApplication* flutterApp = [NSApplication sharedApplication]; + [flutterApp terminate:sender]; }; FlutterAppDelegate* appDelegate = - (FlutterAppDelegate*)[[FlutterApplication sharedApplication] delegate]; + (FlutterAppDelegate*)[[NSApplication sharedApplication] delegate]; appDelegate.terminationHandler = self; return self; } @@ -202,7 +196,7 @@ - (void)handleRequestAppExitMethodCall:(NSDictionary*)arguments FlutterAppExitType exitType = [type isEqualTo:@"cancelable"] ? kFlutterAppExitTypeCancelable : kFlutterAppExitTypeRequired; - [self requestApplicationTermination:[FlutterApplication sharedApplication] + [self requestApplicationTermination:[NSApplication sharedApplication] exitType:exitType result:result]; } @@ -212,6 +206,7 @@ - (void)handleRequestAppExitMethodCall:(NSDictionary*)arguments - (void)requestApplicationTermination:(id)sender exitType:(FlutterAppExitType)type result:(nullable FlutterResult)result { + _shouldTerminate = YES; switch (type) { case kFlutterAppExitTypeCancelable: { FlutterJSONMethodCodec* codec = [FlutterJSONMethodCodec sharedInstance]; @@ -238,6 +233,8 @@ - (void)requestApplicationTermination:(id)sender NSDictionary* replyArgs = (NSDictionary*)decoded_reply; if ([replyArgs[@"response"] isEqual:@"exit"]) { _terminator(sender); + } else if ([replyArgs[@"response"] isEqual:@"cancel"]) { + _shouldTerminate = NO; } if (result != nil) { result(replyArgs); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 36c7d745cdf41..af2cc140f909f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -14,7 +14,6 @@ #include "flutter/shell/platform/common/accessibility_bridge.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h" -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterApplication.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" diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 9b705a030690b..13c47f239ab69 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -8,7 +8,6 @@ #include -#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterApplication.h" #import "flutter/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h" @@ -52,11 +51,14 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { * messages through the platform channel managed by the engine. */ @interface FlutterEngineTerminationHandler : NSObject + +@property(nonatomic, readonly) BOOL shouldTerminate; + - (instancetype)initWithEngine:(FlutterEngine*)engine terminator:(nullable FlutterTerminationCallback)terminator; - (void)handleRequestAppExitMethodCall:(NSDictionary*)data result:(FlutterResult)result; -- (void)requestApplicationTermination:(FlutterApplication*)sender +- (void)requestApplicationTermination:(NSApplication*)sender exitType:(FlutterAppExitType)type result:(nullable FlutterResult)result; @end