Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and migrate FlutterUndoManagerPlugin to ARC #52234

Merged
merged 4 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions shell/platform/darwin/ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ source_set("flutter_framework_source_arc") {
"framework/Source/FlutterTextureRegistryRelay.mm",
"framework/Source/FlutterUIPressProxy.h",
"framework/Source/FlutterUIPressProxy.mm",
"framework/Source/FlutterUndoManagerDelegate.h",
"framework/Source/FlutterUndoManagerPlugin.h",
"framework/Source/FlutterUndoManagerPlugin.mm",
"framework/Source/KeyCodeMap.g.mm",
"framework/Source/KeyCodeMap_Internal.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.h",
Expand Down Expand Up @@ -157,9 +160,6 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterPluginAppLifeCycleDelegate.mm",
"framework/Source/FlutterSemanticsScrollView.h",
"framework/Source/FlutterSemanticsScrollView.mm",
"framework/Source/FlutterUndoManagerDelegate.h",
"framework/Source/FlutterUndoManagerPlugin.h",
"framework/Source/FlutterUndoManagerPlugin.mm",
"framework/Source/FlutterView.h",
"framework/Source/FlutterView.mm",
"framework/Source/FlutterViewController.mm",
Expand Down
13 changes: 9 additions & 4 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ - (void)setViewController:(FlutterViewController*)viewController {
[self maybeSetupPlatformViewChannels];
[self updateDisplays];
_textInputPlugin.get().viewController = viewController;
_undoManagerPlugin.get().viewController = viewController;

if (viewController) {
__block FlutterEngine* blockSelf = self;
Expand Down Expand Up @@ -465,7 +464,6 @@ - (void)setFlutterViewControllerWillDeallocObserver:(id<NSObject>)observer {
- (void)notifyViewControllerDeallocated {
[[self lifecycleChannel] sendMessage:@"AppLifecycleState.detached"];
_textInputPlugin.get().viewController = nil;
_undoManagerPlugin.get().viewController = nil;
if (!_allowHeadlessExecution) {
[self destroyContext];
} else if (_shell) {
Expand Down Expand Up @@ -1189,12 +1187,19 @@ - (void)flutterTextInputView:(FlutterTextInputView*)textInputView

#pragma mark - Undo Manager Delegate

- (void)flutterUndoManagerPlugin:(FlutterUndoManagerPlugin*)undoManagerPlugin
handleUndoWithDirection:(FlutterUndoRedoDirection)direction {
- (void)handleUndoWithDirection:(FlutterUndoRedoDirection)direction {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undoManagerPlugin was unused.

NSString* action = (direction == FlutterUndoRedoDirectionUndo) ? @"undo" : @"redo";
[_undoManagerChannel.get() invokeMethod:@"UndoManagerClient.handleUndo" arguments:@[ action ]];
}

- (UIView<UITextInput>*)activeTextInputView {
return [[self textInputPlugin] textInputView];
}

- (NSUndoManager*)undoManager {
return self.viewController.undoManager;
}
Comment on lines +1195 to +1201
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two new properties to conform to the delegate, to reduce how much the plugin needs to understand this ownership model.


#pragma mark - Screenshot Delegate

- (flutter::Rasterizer::Screenshot)takeScreenshot:(flutter::Rasterizer::ScreenshotType)type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,29 @@ typedef NS_ENUM(NSInteger, FlutterUndoRedoDirection) {
// NOLINTEND(readability-identifier-naming)
};

@class FlutterUndoManagerPlugin;

/**
* Protocol for undo manager changes from the `FlutterUndoManagerPlugin`, typically a
* `FlutterEngine`.
*/
@protocol FlutterUndoManagerDelegate <NSObject>
- (void)flutterUndoManagerPlugin:(FlutterUndoManagerPlugin*)undoManagerPlugin
handleUndoWithDirection:(FlutterUndoRedoDirection)direction;

/**
* The `NSUndoManager` that should be managed by the `FlutterUndoManagerPlugin`.
* When the delegate is `FlutterEngine` this will be the `FlutterViewController`'s undo manager.
*/
@property(nonatomic, readonly, nullable) NSUndoManager* undoManager;

/**
* Used to notify the active view when undo manager state (can redo/can undo)
* changes, in order to force keyboards to update undo/redo buttons.
*/
@property(nonatomic, readonly, nullable) UIView<UITextInput>* activeTextInputView;

/**
* Pass changes to the framework through the undo manager channel.
*/
- (void)handleUndoWithDirection:(FlutterUndoRedoDirection)direction;

@end
NS_ASSUME_NONNULL_END

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@

#import <UIKit/UIKit.h>

#import "flutter/fml/memory/weak_ptr.h"
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h"
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUndoManagerDelegate.h"

@interface FlutterUndoManagerPlugin : NSObject

@property(nonatomic, assign) FlutterViewController* viewController;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@
// found in the LICENSE file.

#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterUndoManagerPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h"

#import <Foundation/Foundation.h>
#import <UIKit/UIKit.h>

#include "flutter/fml/logging.h"

#pragma mark - UndoManager channel method names.
static NSString* const kSetUndoStateMethod = @"UndoManager.setUndoState";
Expand All @@ -17,24 +11,24 @@
static NSString* const kCanUndo = @"canUndo";
static NSString* const kCanRedo = @"canRedo";

@implementation FlutterUndoManagerPlugin {
id<FlutterUndoManagerDelegate> _undoManagerDelegate;
}
@interface FlutterUndoManagerPlugin ()
@property(nonatomic, weak, readonly) id<FlutterUndoManagerDelegate> undoManagerDelegate;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From below:

// `_undoManagerDelegate` is a weak reference because it should retain FlutterUndoManagerPlugin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also have a declaration comment (maybe a short summary of what kinds of things are delegated).

@end

@implementation FlutterUndoManagerPlugin

- (instancetype)initWithDelegate:(id<FlutterUndoManagerDelegate>)undoManagerDelegate {
self = [super init];

if (self) {
// `_undoManagerDelegate` is a weak reference because it should retain FlutterUndoManagerPlugin.
_undoManagerDelegate = undoManagerDelegate;
}

return self;
}

- (void)dealloc {
[self resetUndoManager];
[super dealloc];
[_undoManagerDelegate.undoManager removeAllActionsWithTarget:self];
}

- (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
Expand All @@ -48,46 +42,43 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
}
}

- (NSUndoManager*)undoManager {
return _viewController.undoManager;
}

- (void)resetUndoManager API_AVAILABLE(ios(9.0)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed API_AVAILABLE(ios(9.0)) since the min is iOS 12. Must have missed that in a cleanup.

[[self undoManager] removeAllActionsWithTarget:self];
- (void)resetUndoManager {
[self.undoManagerDelegate.undoManager removeAllActionsWithTarget:self];
}

- (void)registerUndoWithDirection:(FlutterUndoRedoDirection)direction API_AVAILABLE(ios(9.0)) {
[[self undoManager] beginUndoGrouping];
[[self undoManager] registerUndoWithTarget:self
handler:^(id target) {
// Register undo with opposite direction.
FlutterUndoRedoDirection newDirection =
(direction == FlutterUndoRedoDirectionRedo)
? FlutterUndoRedoDirectionUndo
: FlutterUndoRedoDirectionRedo;
[target registerUndoWithDirection:newDirection];
// Invoke method on delegate.
[_undoManagerDelegate flutterUndoManagerPlugin:self
handleUndoWithDirection:direction];
}];
[[self undoManager] endUndoGrouping];
- (void)registerUndoWithDirection:(FlutterUndoRedoDirection)direction {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
[undoManager beginUndoGrouping];
[undoManager registerUndoWithTarget:self
handler:^(FlutterUndoManagerPlugin* target) {
// Register undo with opposite direction.
FlutterUndoRedoDirection newDirection =
(direction == FlutterUndoRedoDirectionRedo)
? FlutterUndoRedoDirectionUndo
: FlutterUndoRedoDirectionRedo;
[target registerUndoWithDirection:newDirection];
// Invoke method on delegate.
[target.undoManagerDelegate handleUndoWithDirection:direction];
}];
[undoManager endUndoGrouping];
}

- (void)registerRedo API_AVAILABLE(ios(9.0)) {
[[self undoManager] beginUndoGrouping];
[[self undoManager]
registerUndoWithTarget:self
handler:^(id target) {
// Register undo with opposite direction.
[target registerUndoWithDirection:FlutterUndoRedoDirectionRedo];
}];
[[self undoManager] endUndoGrouping];
[[self undoManager] undo];
- (void)registerRedo {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
[undoManager beginUndoGrouping];
[undoManager registerUndoWithTarget:self
handler:^(id target) {
// Register undo with opposite direction.
[target registerUndoWithDirection:FlutterUndoRedoDirectionRedo];
}];
[undoManager endUndoGrouping];
[undoManager undo];
}

- (void)setUndoState:(NSDictionary*)dictionary API_AVAILABLE(ios(9.0)) {
BOOL groupsByEvent = [self undoManager].groupsByEvent;
[self undoManager].groupsByEvent = NO;
- (void)setUndoState:(NSDictionary*)dictionary {
NSUndoManager* undoManager = self.undoManagerDelegate.undoManager;
BOOL groupsByEvent = undoManager.groupsByEvent;
undoManager.groupsByEvent = NO;
BOOL canUndo = [dictionary[kCanUndo] boolValue];
BOOL canRedo = [dictionary[kCanRedo] boolValue];

Expand All @@ -99,16 +90,15 @@ - (void)setUndoState:(NSDictionary*)dictionary API_AVAILABLE(ios(9.0)) {
if (canRedo) {
[self registerRedo];
}

if (_viewController.engine.textInputPlugin.textInputView != nil) {
UIView<UITextInput>* textInputView = self.undoManagerDelegate.activeTextInputView;
if (textInputView != nil) {
// This is needed to notify the iPadOS keyboard that it needs to update the
// state of the UIBarButtons. Otherwise, the state changes to NSUndoManager
// will not show up until the next keystroke (or other trigger).
UITextInputAssistantItem* assistantItem =
_viewController.engine.textInputPlugin.textInputView.inputAssistantItem;
UITextInputAssistantItem* assistantItem = textInputView.inputAssistantItem;
assistantItem.leadingBarButtonGroups = assistantItem.leadingBarButtonGroups;
}
[self undoManager].groupsByEvent = groupsByEvent;
undoManager.groupsByEvent = groupsByEvent;
}

@end
Loading