Remove FlutterUndoManagerPlugin handlers from undo manager on dealloc (flutter/engine#53553)

When `FlutterUndoManagerPlugin` deallocated, it is supposed to deregister itself from the `NSUndoManager` stack.
However, during dealloc `_undoManagerDelegate.undoManager` is nil (`_undoManagerDelegate` = the engine, and `undoManager` was its view controller's `undoManager`, which was already gone).

Since `_undoManagerDelegate.undoManager` is nil, it doesn't actually call `-[NSUndoManager removeAllActionsWithTarget]`. In the add-to-app scenario, after the view controller pops back to the native view, and "undo" is invoked, the undo action for the `FlutterUndoManagerPlugin` is handled, but it already dealloced so crash.

Fixes https://github.com/flutter/flutter/issues/150408

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
Jenn Magder 2024-06-25 16:54:26 -07:00 committed by GitHub
parent 335e888cf5
commit 53573f860f
2 changed files with 45 additions and 5 deletions

View File

@ -13,6 +13,10 @@ static NSString* const kCanRedo = @"canRedo";
@interface FlutterUndoManagerPlugin ()
@property(nonatomic, weak, readonly) id<FlutterUndoManagerDelegate> undoManagerDelegate;
// When the delegate is `FlutterEngine` this will be the `FlutterViewController`'s undo manager.
// Strongly retain to ensure this target's actions are completely removed during dealloc.
@property(nonatomic) NSUndoManager* undoManager;
@end
@implementation FlutterUndoManagerPlugin
@ -28,13 +32,14 @@ static NSString* const kCanRedo = @"canRedo";
}
- (void)dealloc {
[_undoManagerDelegate.undoManager removeAllActionsWithTarget:self];
[_undoManager removeAllActionsWithTarget:self];
}
- (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
NSString* method = call.method;
id args = call.arguments;
if ([method isEqualToString:kSetUndoStateMethod]) {
self.undoManager = self.undoManagerDelegate.undoManager;
[self setUndoState:args];
result(nil);
} else {
@ -43,11 +48,11 @@ static NSString* const kCanRedo = @"canRedo";
}
- (void)resetUndoManager {
[self.undoManagerDelegate.undoManager removeAllActionsWithTarget:self];
[self.undoManager removeAllActionsWithTarget:self];
}
- (void)registerUndoWithDirection:(FlutterUndoRedoDirection)direction {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
NSUndoManager* undoManager = self.undoManager;
[undoManager beginUndoGrouping];
[undoManager registerUndoWithTarget:self
handler:^(FlutterUndoManagerPlugin* target) {
@ -64,7 +69,7 @@ static NSString* const kCanRedo = @"canRedo";
}
- (void)registerRedo {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
NSUndoManager* undoManager = self.undoManager;
[undoManager beginUndoGrouping];
[undoManager registerUndoWithTarget:self
handler:^(id target) {
@ -76,7 +81,7 @@ static NSString* const kCanRedo = @"canRedo";
}
- (void)setUndoState:(NSDictionary*)dictionary {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
NSUndoManager* undoManager = self.undoManager;
BOOL groupsByEvent = undoManager.groupsByEvent;
undoManager.groupsByEvent = NO;
BOOL canUndo = [dictionary[kCanUndo] boolValue];

View File

@ -27,6 +27,7 @@ FLUTTER_ASSERT_ARC
@property(readonly) NSUInteger undoCount;
@property(readonly) NSUInteger redoCount;
@property(nonatomic, nullable) NSUndoManager* undoManager;
- (instancetype)initWithUndoManager:(NSUndoManager*)undoManager
activeTextInputView:(TextInputViewTest*)activeTextInputView;
@ -164,4 +165,38 @@ FLUTTER_ASSERT_ARC
OCMVerify(never(), [self.activeTextInputView inputDelegate]);
}
- (void)testDeallocRemovesAllUndoManagerActions {
__weak FlutterUndoManagerPlugin* weakUndoManagerPlugin;
// Use a real undo manager.
NSUndoManager* undoManager = [[NSUndoManager alloc] init];
@autoreleasepool {
id activeTextInputView = OCMClassMock([TextInputViewTest class]);
FakeFlutterUndoManagerDelegate* undoManagerDelegate =
[[FakeFlutterUndoManagerDelegate alloc] initWithUndoManager:undoManager
activeTextInputView:activeTextInputView];
FlutterUndoManagerPlugin* undoManagerPlugin =
[[FlutterUndoManagerPlugin alloc] initWithDelegate:undoManagerDelegate];
weakUndoManagerPlugin = undoManagerPlugin;
FlutterMethodCall* setUndoStateCall =
[FlutterMethodCall methodCallWithMethodName:@"UndoManager.setUndoState"
arguments:@{@"canUndo" : @YES, @"canRedo" : @YES}];
[undoManagerPlugin handleMethodCall:setUndoStateCall
result:^(id _Nullable result){
}];
XCTAssertTrue(undoManager.canUndo);
XCTAssertTrue(undoManager.canRedo);
// Fake out the undoManager being nil, which happens when the FlutterViewController deallocs and
// the undo manager can't be fetched from the FlutterEngine delegate.
undoManagerDelegate.undoManager = nil;
}
XCTAssertNil(weakUndoManagerPlugin);
// Regression test for https://github.com/flutter/flutter/issues/150408.
// Undo manager undo and redo stack should be empty after the plugin deallocs.
XCTAssertFalse(undoManager.canUndo);
XCTAssertFalse(undoManager.canRedo);
}
@end