diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 854a3ff7f4c1e..ef95a76abad7d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -505,6 +505,9 @@ @implementation FlutterTextInputView { const char* _selectionAffinity; FlutterTextRange* _selectedTextRange; CGRect _cachedFirstRect; + // The view has reached end of life, and is no longer + // allowed to access its textInputDelegate. + BOOL _decommissioned; } @synthesize tokenizer = _tokenizer; @@ -535,6 +538,7 @@ - (instancetype)init { _returnKeyType = UIReturnKeyDone; _secureTextEntry = NO; _accessibilityEnabled = NO; + _decommissioned = NO; if (@available(iOS 11.0, *)) { _smartQuotesType = UITextSmartQuotesTypeYes; _smartDashesType = UITextSmartDashesTypeYes; @@ -545,6 +549,7 @@ - (instancetype)init { } - (void)configureWithDictionary:(NSDictionary*)configuration { + NSAssert(!_decommissioned, @"Attempt to reuse a decommissioned view, for %@", configuration); NSDictionary* inputType = configuration[kKeyboardType]; NSString* keyboardAppearance = configuration[kKeyboardAppearance]; NSDictionary* autofill = configuration[kAutofillProperties]; @@ -596,6 +601,23 @@ - (UITextContentType)textContentType { return _textContentType; } +- (id)textInputDelegate { + return _decommissioned ? nil : _textInputDelegate; +} + +// Declares that the view has reached end of life, and +// is no longer allowed to access its textInputDelegate. +// +// UIKit may retain this view (even after it's been removed +// from the view hierarchy) so that it may outlive the plugin/engine, +// in which case _textInputDelegate will become a dangling pointer. + +// The text input plugin needs to call decommision when it should +// not have access to its FlutterTextInputDelegate any more. +- (void)decommision { + _decommissioned = YES; +} + - (void)dealloc { [_text release]; [_markedText release]; @@ -778,7 +800,8 @@ - (void)replaceRange:(UITextRange*)range withText:(NSString*)text { - (BOOL)shouldChangeTextInRange:(UITextRange*)range replacementText:(NSString*)text { if (self.returnKeyType == UIReturnKeyDefault && [text isEqualToString:@"\n"]) { - [_textInputDelegate performAction:FlutterTextInputActionNewline withClient:_textInputClient]; + [self.textInputDelegate performAction:FlutterTextInputActionNewline + withClient:_textInputClient]; return YES; } @@ -819,7 +842,7 @@ - (BOOL)shouldChangeTextInRange:(UITextRange*)range replacementText:(NSString*)t break; } - [_textInputDelegate performAction:action withClient:_textInputClient]; + [self.textInputDelegate performAction:action withClient:_textInputClient]; return NO; } @@ -1062,9 +1085,9 @@ - (CGRect)firstRectForRange:(UITextRange*)range { return _cachedFirstRect; } - [_textInputDelegate showAutocorrectionPromptRectForStart:start - end:end - withClient:_textInputClient]; + [self.textInputDelegate showAutocorrectionPromptRectForStart:start + end:end + withClient:_textInputClient]; // TODO(cbracken) Implement. return CGRectZero; } @@ -1097,21 +1120,21 @@ - (UITextRange*)characterRangeAtPoint:(CGPoint)point { } - (void)beginFloatingCursorAtPoint:(CGPoint)point { - [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateStart - withClient:_textInputClient - withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}]; + [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateStart + withClient:_textInputClient + withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}]; } - (void)updateFloatingCursorAtPoint:(CGPoint)point { - [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateUpdate - withClient:_textInputClient - withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}]; + [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateUpdate + withClient:_textInputClient + withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}]; } - (void)endFloatingCursor { - [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateEnd - withClient:_textInputClient - withPosition:@{@"X" : @(0), @"Y" : @(0)}]; + [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateEnd + withClient:_textInputClient + withPosition:@{@"X" : @(0), @"Y" : @(0)}]; } #pragma mark - UIKeyInput Overrides @@ -1139,9 +1162,11 @@ - (void)updateEditingState { }; if (_textInputClient == 0 && _autofillId != nil) { - [_textInputDelegate updateEditingClient:_textInputClient withState:state withTag:_autofillId]; + [self.textInputDelegate updateEditingClient:_textInputClient + withState:state + withTag:_autofillId]; } else { - [_textInputDelegate updateEditingClient:_textInputClient withState:state]; + [self.textInputDelegate updateEditingClient:_textInputClient withState:state]; } } @@ -1259,8 +1284,6 @@ - (void)enableActiveViewAccessibility { @end @interface FlutterTextInputPlugin () -@property(nonatomic, strong) FlutterTextInputView* reusableInputView; - // The current password-autofillable input fields that have yet to be saved. @property(nonatomic, readonly) NSMutableDictionary* autofillContext; @@ -1278,11 +1301,11 @@ - (instancetype)init { self = [super init]; if (self) { - _reusableInputView = [[FlutterTextInputView alloc] init]; - _reusableInputView.secureTextEntry = NO; _autofillContext = [[NSMutableDictionary alloc] init]; - _activeView = [_reusableInputView retain]; _inputHider = [[FlutterTextInputViewAccessibilityHider alloc] init]; + // Initialize activeView with a dummy view to keep tests + // passing. + _activeView = [[FlutterTextInputView alloc] init]; } return self; @@ -1291,7 +1314,6 @@ - (instancetype)init { - (void)dealloc { [self hideTextInput]; _activeView.textInputDelegate = nil; - [_reusableInputView release]; [_activeView release]; [_inputHider release]; [_autofillContext release]; @@ -1398,14 +1420,14 @@ - (void)triggerAutofillSave:(BOOL)saveEntries { if (saveEntries) { // Make all the input fields in the autofill context visible, // then remove them to trigger autofill save. - [self cleanUpViewHierarchy:YES clearText:YES]; + [self cleanUpViewHierarchy:YES clearText:YES delayRemoval:NO]; [_autofillContext removeAllObjects]; [self changeInputViewsAutofillVisibility:YES]; } else { [_autofillContext removeAllObjects]; } - [self cleanUpViewHierarchy:YES clearText:!saveEntries]; + [self cleanUpViewHierarchy:YES clearText:!saveEntries delayRemoval:NO]; [self addToInputParentViewIfNeeded:_activeView]; } @@ -1414,9 +1436,11 @@ - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configur // Hide all input views from autofill, only make those in the new configuration visible // to autofill. [self changeInputViewsAutofillVisibility:NO]; + + // Update the current active view. switch (autofillTypeOf(configuration)) { case FlutterAutofillTypeNone: - self.activeView = [self updateAndShowReusableInputView:configuration]; + self.activeView = [self createInputViewWith:configuration]; break; case FlutterAutofillTypeRegular: // If the group does not involve password autofill, only install the @@ -1431,7 +1455,6 @@ - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configur isPasswordRelated:YES]; break; } - [_activeView setTextInputClient:client]; [_activeView reloadInputViews]; @@ -1441,27 +1464,29 @@ - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configur // them to free up resources and reduce the number of input views in the view // hierarchy. // - // This is scheduled on the runloop and delayed by 0.1s so we don't remove the + // The garbage views are decommissioned immediately, but the removeFromSuperview + // call is scheduled on the runloop and delayed by 0.1s so we don't remove the // text fields immediately (which seems to make the keyboard flicker). // See: https://github.com/flutter/flutter/issues/64628. - [self performSelector:@selector(collectGarbageInputViews) withObject:nil afterDelay:0.1]; + [self cleanUpViewHierarchy:NO clearText:YES delayRemoval:YES]; } -// Updates and shows an input field that is not password related and has no autofill -// hints. This method re-configures and reuses an existing instance of input field -// instead of creating a new one. -// Also updates the current autofill context. -- (FlutterTextInputView*)updateAndShowReusableInputView:(NSDictionary*)configuration { +// Creates and shows an input field that is not password related and has no autofill +// hints. This method returns a new FlutterTextInputView instance when called, since +// UIKit uses the identity of `UITextInput` instances (or the identity of the input +// views) to decide whether the IME's internal states should be reset. See: +// https://github.com/flutter/flutter/issues/79031 . +- (FlutterTextInputView*)createInputViewWith:(NSDictionary*)configuration { // It's possible that the configuration of this non-autofillable input view has // an autofill configuration without hints. If it does, remove it from the context. NSString* autofillId = autofillIdFromDictionary(configuration); if (autofillId) { [_autofillContext removeObjectForKey:autofillId]; } - - [_reusableInputView configureWithDictionary:configuration]; - [self addToInputParentViewIfNeeded:_reusableInputView]; - _reusableInputView.textInputDelegate = _textInputDelegate; + FlutterTextInputView* newView = [[FlutterTextInputView alloc] init]; + [newView configureWithDictionary:configuration]; + [self addToInputParentViewIfNeeded:newView]; + newView.textInputDelegate = _textInputDelegate; for (NSDictionary* field in configuration[kAssociatedAutofillFields]) { NSString* autofillId = autofillIdFromDictionary(field); @@ -1469,7 +1494,7 @@ - (FlutterTextInputView*)updateAndShowReusableInputView:(NSDictionary*)configura [_autofillContext removeObjectForKey:autofillId]; } } - return _reusableInputView; + return [newView autorelease]; } - (FlutterTextInputView*)updateAndShowAutofillViews:(NSArray*)fields @@ -1547,11 +1572,21 @@ - (UIView*)keyWindow { return _inputHider.subviews; } -// Removes every installed input field, unless it's in the current autofill -// context. May remove the active view too if includeActiveView is YES. +// Decommisions (See the "decommision" method on FlutterTextInputView) and removes +// every installed input field, unless it's in the current autofill context. +// +// The active view will be decommisioned and removed from its superview too, if +// includeActiveView is YES. // When clearText is YES, the text on the input fields will be set to empty before // they are removed from the view hierarchy, to avoid triggering autofill save. -- (void)cleanUpViewHierarchy:(BOOL)includeActiveView clearText:(BOOL)clearText { +// If delayRemoval is true, removeFromSuperview will be scheduled on the runloop and +// will be delayed by 0.1s so we don't remove the text fields immediately (which seems +// to make the keyboard flicker). +// See: https://github.com/flutter/flutter/issues/64628. + +- (void)cleanUpViewHierarchy:(BOOL)includeActiveView + clearText:(BOOL)clearText + delayRemoval:(BOOL)delayRemoval { for (UIView* view in self.textInputViews) { if ([view isKindOfClass:[FlutterTextInputView class]] && (includeActiveView || view != _activeView)) { @@ -1560,16 +1595,17 @@ - (void)cleanUpViewHierarchy:(BOOL)includeActiveView clearText:(BOOL)clearText { if (clearText) { [inputView replaceRangeLocal:NSMakeRange(0, inputView.text.length) withText:@""]; } - [view removeFromSuperview]; + [inputView decommision]; + if (delayRemoval) { + [inputView performSelector:@selector(removeFromSuperview) withObject:nil afterDelay:0.1]; + } else { + [inputView removeFromSuperview]; + } } } } } -- (void)collectGarbageInputViews { - [self cleanUpViewHierarchy:NO clearText:YES]; -} - // Changes the visibility of every FlutterTextInputView currently in the // view hierarchy. - (void)changeInputViewsAutofillVisibility:(BOOL)newVisibility { @@ -1582,7 +1618,12 @@ - (void)changeInputViewsAutofillVisibility:(BOOL)newVisibility { } // Resets the client id of every FlutterTextInputView in the view hierarchy -// to 0. Called when a new text input connection will be established. +// to 0. +// Called before establishing a new text input connection. +// For views in the current autofill context, they need to +// stay in the view hierachy but should not be allowed to +// send messages (other than autofill related ones) to the +// framework. - (void)resetAllClientIds { for (UIView* view in self.textInputViews) { if ([view isKindOfClass:[FlutterTextInputView class]]) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m index 997daa2a0d48a..5cd8defa9dd68 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m @@ -19,6 +19,7 @@ - (void)setEditableTransform:(NSArray*)matrix; - (void)setTextInputState:(NSDictionary*)state; - (void)setMarkedRect:(CGRect)markedRect; - (void)updateEditingState; +- (void)decommisson; - (BOOL)isVisibleToAutofill; @end @@ -56,7 +57,9 @@ @interface FlutterTextInputPlugin () @property(nonatomic, readonly) NSMutableDictionary* autofillContext; -- (void)collectGarbageInputViews; +- (void)cleanUpViewHierarchy:(BOOL)includeActiveView + clearText:(BOOL)clearText + delayRemoval:(BOOL)delayRemoval; - (NSArray*)textInputViews; @end @@ -252,6 +255,34 @@ - (void)testInputViewCrash { [activeView updateEditingState]; } +- (void)testDoNotReuseInputViews { + NSDictionary* config = self.mutableTemplateCopy; + [self setClientId:123 configuration:config]; + FlutterTextInputView* currentView = textInputPlugin.activeView; + [self setClientId:456 configuration:config]; + + XCTAssertNotNil(currentView); + XCTAssertNotNil(textInputPlugin.activeView); + XCTAssertNotEqual(currentView, textInputPlugin.activeView); +} + +- (void)testNoDanglingEnginePointer { + NSDictionary* config = self.mutableTemplateCopy; + [self setClientId:123 configuration:config]; + + // We'll hold onto the current view and try to access the engine + // after changing the active view. + FlutterTextInputView* currentView = textInputPlugin.activeView; + [self setClientId:456 configuration:config]; + XCTAssertNotNil(currentView); + XCTAssertNotNil(textInputPlugin.activeView); + XCTAssertNotEqual(currentView, textInputPlugin.activeView); + + // Verify that the view can no longer access the engine + // instance. + XCTAssertNil(currentView.textInputDelegate); +} + - (void)ensureOnlyActiveViewCanBecomeFirstResponder { for (FlutterTextInputView* inputView in self.installedInputViews) { XCTAssertEqual(inputView.canBecomeFirstResponder, inputView == textInputPlugin.activeView); @@ -566,7 +597,7 @@ - (void)testAutofillContext { XCTAssertEqual(textInputPlugin.autofillContext.count, 2); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 2); XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); [self ensureOnlyActiveViewCanBecomeFirstResponder]; @@ -589,7 +620,7 @@ - (void)testAutofillContext { XCTAssertEqual(self.viewsVisibleToAutofill.count, 2); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 3); XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); [self ensureOnlyActiveViewCanBecomeFirstResponder]; @@ -609,7 +640,7 @@ - (void)testAutofillContext { XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 4); // Old autofill input fields are still installed and reused. @@ -628,7 +659,7 @@ - (void)testAutofillContext { XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 4); // Old autofill input fields are still installed and reused. @@ -673,7 +704,6 @@ - (void)testCommitAutofillContext { [self ensureOnlyActiveViewCanBecomeFirstResponder]; [self commitAutofillContextAndVerify]; - XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Install the password field again. @@ -682,31 +712,28 @@ - (void)testCommitAutofillContext { [self setClientId:124 configuration:field3]; XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 3); XCTAssertEqual(textInputPlugin.autofillContext.count, 2); XCTAssertNotEqual(textInputPlugin.textInputView, nil); [self ensureOnlyActiveViewCanBecomeFirstResponder]; [self commitAutofillContextAndVerify]; - XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Now switch to an input field that does not autofill. [self setClientId:125 configuration:self.mutableTemplateCopy]; XCTAssertEqual(self.viewsVisibleToAutofill.count, 0); - XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); // The active view should still be installed so it doesn't get // deallocated. - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 0); [self ensureOnlyActiveViewCanBecomeFirstResponder]; [self commitAutofillContextAndVerify]; - XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); [self ensureOnlyActiveViewCanBecomeFirstResponder]; } @@ -798,7 +825,7 @@ - (void)testClearAutofillContextClearsSelection { XCTAssertEqual(self.installedInputViews.count, 2); - [textInputPlugin collectGarbageInputViews]; + [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO]; XCTAssertEqual(self.installedInputViews.count, 1); // Verify the old input view is properly cleaned up.