From 8464034a8950287f290df348d5a4f02fe57daae8 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 4 May 2023 16:26:04 -0700 Subject: [PATCH 1/3] Wait for binding to be ready before requesting exits from framework --- .../macos/framework/Source/FlutterAppDelegate.mm | 2 +- .../darwin/macos/framework/Source/FlutterEngine.mm | 9 +++++++-- .../darwin/macos/framework/Source/FlutterEngineTest.mm | 10 ++++++++++ .../macos/framework/Source/FlutterEngine_Internal.h | 1 + 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm b/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm index 5c5f444c6a196..e934c0a6b663e 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate.mm @@ -55,7 +55,7 @@ - (NSString*)applicationName { - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnull)sender { // If the framework has already told us to terminate, terminate immediately. - if ([[self terminationHandler] shouldTerminate]) { + if ([self terminationHandler] == nil || [[self terminationHandler] shouldTerminate]) { return NSTerminateNow; } diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 91aa17e903a33..fca1bf75e158a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -168,6 +168,7 @@ @implementation FlutterEngineTerminationHandler { - (instancetype)initWithEngine:(FlutterEngine*)engine terminator:(FlutterTerminationCallback)terminator { self = [super init]; + _bindingIsReady = NO; _engine = engine; _terminator = terminator ? terminator : ^(id sender) { // Default to actually terminating the application. The terminator exists to @@ -205,6 +206,11 @@ - (void)requestApplicationTermination:(id)sender exitType:(FlutterAppExitType)type result:(nullable FlutterResult)result { _shouldTerminate = YES; + if (![self bindingIsReady]) { + // Until the binding has signaled that it is ready to handle application + // termination requests, the app will just terminate when asked. + type = kFlutterAppExitTypeRequired; + } switch (type) { case kFlutterAppExitTypeCancelable: { FlutterJSONMethodCodec* codec = [FlutterJSONMethodCodec sharedInstance]; @@ -1032,8 +1038,7 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { } else if ([call.method isEqualToString:@"System.exitApplication"]) { [[self terminationHandler] handleRequestAppExitMethodCall:call.arguments result:result]; } else if ([call.method isEqualToString:@"System.initializationComplete"]) { - // TODO(gspencergoog): Handle this message to enable exit message listening. - // https://github.com/flutter/flutter/issues/126033 + [self terminationHandler].bindingIsReady = YES; result(nil); } else { result(FlutterMethodNotImplemented); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 92e87100dd0de..0a3fe0c7f1a29 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -744,6 +744,16 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable [FlutterMethodCall methodCallWithMethodName:@"System.exitApplication" arguments:@{@"type" : @"cancelable"}]; + // Always terminate when the binding isn't ready (which is the default). + triedToTerminate = FALSE; + calledAfterTerminate = @""; + nextResponse = @"cancel"; + [engineMock handleMethodCall:methodExitApplication result:appExitResult]; + EXPECT_STREQ([calledAfterTerminate UTF8String], ""); + EXPECT_TRUE(triedToTerminate); + + // Once the binding is ready, handle the request. + terminationHandler.bindingIsReady = YES; triedToTerminate = FALSE; calledAfterTerminate = @""; nextResponse = @"exit"; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index e291094eaa342..e6e797975f98d 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -53,6 +53,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { @interface FlutterEngineTerminationHandler : NSObject @property(nonatomic, readonly) BOOL shouldTerminate; +@property(nonatomic, readwrite) BOOL bindingIsReady; - (instancetype)initWithEngine:(FlutterEngine*)engine terminator:(nullable FlutterTerminationCallback)terminator; From f3af486fe805ce81dacf575ee1b2e51496fa3243 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 4 May 2023 16:48:45 -0700 Subject: [PATCH 2/3] Rename to acceptingRequests --- .../darwin/macos/framework/Source/FlutterEngine.mm | 8 ++++---- .../darwin/macos/framework/Source/FlutterEngineTest.mm | 2 +- .../macos/framework/Source/FlutterEngine_Internal.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index fca1bf75e158a..d13917d610eb5 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -168,7 +168,7 @@ @implementation FlutterEngineTerminationHandler { - (instancetype)initWithEngine:(FlutterEngine*)engine terminator:(FlutterTerminationCallback)terminator { self = [super init]; - _bindingIsReady = NO; + _acceptingRequests = NO; _engine = engine; _terminator = terminator ? terminator : ^(id sender) { // Default to actually terminating the application. The terminator exists to @@ -206,8 +206,8 @@ - (void)requestApplicationTermination:(id)sender exitType:(FlutterAppExitType)type result:(nullable FlutterResult)result { _shouldTerminate = YES; - if (![self bindingIsReady]) { - // Until the binding has signaled that it is ready to handle application + if (![self acceptingRequests]) { + // Until the Dart application has signaled that it is ready to handle // termination requests, the app will just terminate when asked. type = kFlutterAppExitTypeRequired; } @@ -1038,7 +1038,7 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { } else if ([call.method isEqualToString:@"System.exitApplication"]) { [[self terminationHandler] handleRequestAppExitMethodCall:call.arguments result:result]; } else if ([call.method isEqualToString:@"System.initializationComplete"]) { - [self terminationHandler].bindingIsReady = YES; + [self terminationHandler].acceptingRequests = YES; result(nil); } else { result(FlutterMethodNotImplemented); diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 0a3fe0c7f1a29..63bd6a055875a 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -753,7 +753,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable EXPECT_TRUE(triedToTerminate); // Once the binding is ready, handle the request. - terminationHandler.bindingIsReady = YES; + terminationHandler.acceptingRequests = YES; triedToTerminate = FALSE; calledAfterTerminate = @""; nextResponse = @"exit"; diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index e6e797975f98d..31c076dc1785f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -53,7 +53,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { @interface FlutterEngineTerminationHandler : NSObject @property(nonatomic, readonly) BOOL shouldTerminate; -@property(nonatomic, readwrite) BOOL bindingIsReady; +@property(nonatomic, readwrite) BOOL acceptingRequests; - (instancetype)initWithEngine:(FlutterEngine*)engine terminator:(nullable FlutterTerminationCallback)terminator; From 917809f2063256629186297b9f6416e59ee53c89 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 4 May 2023 16:50:19 -0700 Subject: [PATCH 3/3] NO means NO --- .../macos/framework/Source/FlutterEngineTest.mm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index 63bd6a055875a..1bc60e4c533b4 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -702,7 +702,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable TEST_F(FlutterEngineTest, HandlesTerminationRequest) { id engineMock = CreateMockFlutterEngine(nil); __block NSString* nextResponse = @"exit"; - __block BOOL triedToTerminate = FALSE; + __block BOOL triedToTerminate = NO; FlutterEngineTerminationHandler* terminationHandler = [[FlutterEngineTerminationHandler alloc] initWithEngine:engineMock terminator:^(id sender) { @@ -745,7 +745,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable arguments:@{@"type" : @"cancelable"}]; // Always terminate when the binding isn't ready (which is the default). - triedToTerminate = FALSE; + triedToTerminate = NO; calledAfterTerminate = @""; nextResponse = @"cancel"; [engineMock handleMethodCall:methodExitApplication result:appExitResult]; @@ -754,14 +754,14 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable // Once the binding is ready, handle the request. terminationHandler.acceptingRequests = YES; - triedToTerminate = FALSE; + triedToTerminate = NO; calledAfterTerminate = @""; nextResponse = @"exit"; [engineMock handleMethodCall:methodExitApplication result:appExitResult]; EXPECT_STREQ([calledAfterTerminate UTF8String], "exit"); EXPECT_TRUE(triedToTerminate); - triedToTerminate = FALSE; + triedToTerminate = NO; calledAfterTerminate = @""; nextResponse = @"cancel"; [engineMock handleMethodCall:methodExitApplication result:appExitResult]; @@ -769,7 +769,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable EXPECT_FALSE(triedToTerminate); // Check that it doesn't crash on error. - triedToTerminate = FALSE; + triedToTerminate = NO; calledAfterTerminate = @""; nextResponse = @"error"; [engineMock handleMethodCall:methodExitApplication result:appExitResult]; @@ -778,7 +778,7 @@ - (nonnull NSView*)createWithViewIdentifier:(int64_t)viewId arguments:(nullable } TEST_F(FlutterEngineTest, HandleAccessibilityEvent) { - __block BOOL announced = FALSE; + __block BOOL announced = NO; id engineMock = CreateMockFlutterEngine(nil); OCMStub([engineMock announceAccessibilityMessage:[OCMArg any]