From 6b250c5725314cd44e20056bacabc3336aece24a Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 3 Sep 2020 13:55:02 -0700 Subject: [PATCH] Fix iOS text field input keyboard flickering & crash (flutter/engine#20805) --- .../Source/FlutterTextInputPlugin.mm | 75 +++++++++----- .../Source/FlutterTextInputPluginTest.m | 98 +++++++++++++++++-- 2 files changed, 140 insertions(+), 33 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 77b65e63554..ca276ab9904 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -1121,7 +1121,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { - (void)showTextInput { _activeView.textInputDelegate = _textInputDelegate; - [self addToKeyWindowIfNeeded:_activeView]; + [self addToInputParentViewIfNeeded:_activeView]; [_activeView becomeFirstResponder]; } @@ -1143,10 +1143,11 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { } [self cleanUpViewHierarchy:YES clearText:!saveEntries]; - [self addToKeyWindowIfNeeded:_activeView]; + [self addToInputParentViewIfNeeded:_activeView]; } - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configuration { + [self resetAllClientIds]; // Hide all input views from autofill, only make those in the new configuration visible // to autofill. [self changeInputViewsAutofillVisibility:NO]; @@ -1168,11 +1169,19 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { break; } - // Clean up views that should no longer be in the view hierarchy according to the - // updated autofill context. - [self cleanUpViewHierarchy:NO clearText:YES]; [_activeView setTextInputClient:client]; [_activeView reloadInputViews]; + + // Clean up views that no longer need to be in the view hierarchy, according to + // the current autofill context. The "garbage" input views are already made + // invisible to autofill and they can't `becomeFirstResponder`, we only remove + // 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 + // 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]; } // Updates and shows an input field that is not password related and has no autofill @@ -1188,7 +1197,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { } [_reusableInputView configureWithDictionary:configuration]; - [self addToKeyWindowIfNeeded:_reusableInputView]; + [self addToInputParentViewIfNeeded:_reusableInputView]; _reusableInputView.textInputDelegate = _textInputDelegate; for (NSDictionary* field in configuration[kAssociatedAutofillFields]) { @@ -1253,7 +1262,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { inputView = needsPasswordAutofill ? [FlutterSecureTextInputView alloc] : [FlutterTextInputView alloc]; inputView = [[inputView init] autorelease]; - [self addToKeyWindowIfNeeded:inputView]; + [self addToInputParentViewIfNeeded:inputView]; } inputView.textInputDelegate = _textInputDelegate; @@ -1261,23 +1270,27 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { return inputView; } -// Removes every installed input field, unless it's in the current autofill -// context. May remove the active view 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 autofill save . -- (void)cleanUpViewHierarchy:(BOOL)includeActiveView clearText:(BOOL)clearText { +// The UIView to add FlutterTextInputViews to. +- (UIView*)textInputParentView { UIWindow* keyWindow = [UIApplication sharedApplication].keyWindow; NSAssert(keyWindow != nullptr, @"The application must have a key window since the keyboard client " @"must be part of the responder chain to function"); + return keyWindow; +} - for (UIView* view in keyWindow.subviews) { +// Removes every installed input field, unless it's in the current autofill +// context. May remove the active view 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 { + for (UIView* view in self.textInputParentView.subviews) { if ([view isKindOfClass:[FlutterTextInputView class]] && (includeActiveView || view != _activeView)) { FlutterTextInputView* inputView = (FlutterTextInputView*)view; if (_autofillContext[inputView.autofillId] != view) { if (clearText) { - inputView.text.string = @""; + [inputView replaceRangeLocal:NSMakeRange(0, inputView.text.length) withText:@""]; } [view removeFromSuperview]; } @@ -1285,13 +1298,14 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { } } -- (void)changeInputViewsAutofillVisibility:(BOOL)newVisibility { - UIWindow* keyWindow = [UIApplication sharedApplication].keyWindow; - NSAssert(keyWindow != nullptr, - @"The application must have a key window since the keyboard client " - @"must be part of the responder chain to function"); +- (void)collectGarbageInputViews { + [self cleanUpViewHierarchy:NO clearText:YES]; +} - for (UIView* view in keyWindow.subviews) { +// Changes the visibility of every FlutterTextInputView currently in the +// view hierarchy. +- (void)changeInputViewsAutofillVisibility:(BOOL)newVisibility { + for (UIView* view in self.textInputParentView.subviews) { if ([view isKindOfClass:[FlutterTextInputView class]]) { FlutterTextInputView* inputView = (FlutterTextInputView*)view; inputView.isVisibleToAutofill = newVisibility; @@ -1299,14 +1313,21 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) { } } -- (void)addToKeyWindowIfNeeded:(FlutterTextInputView*)inputView { - UIWindow* keyWindow = [UIApplication sharedApplication].keyWindow; - NSAssert(keyWindow != nullptr, - @"The application must have a key window since the keyboard client " - @"must be part of the responder chain to function"); +// Resets the client id of every FlutterTextInputView in the view hierarchy +// to 0. Called when a new text input connection will be established. +- (void)resetAllClientIds { + for (UIView* view in self.textInputParentView.subviews) { + if ([view isKindOfClass:[FlutterTextInputView class]]) { + FlutterTextInputView* inputView = (FlutterTextInputView*)view; + [inputView setTextInputClient:0]; + } + } +} - if (inputView.window != keyWindow) { - [keyWindow addSubview:inputView]; +- (void)addToInputParentViewIfNeeded:(FlutterTextInputView*)inputView { + UIView* parentView = self.textInputParentView; + if (inputView.superview != parentView) { + [parentView addSubview:inputView]; } } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m index a6d10993de7..f62f665fbc0 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m @@ -27,6 +27,9 @@ FLUTTER_ASSERT_ARC @property(nonatomic, assign) FlutterTextInputView* activeView; @property(nonatomic, readonly) NSMutableDictionary* autofillContext; + +- (void)collectGarbageInputViews; +- (UIView*)textInputParentView; @end @interface FlutterTextInputPluginTest : XCTestCase @@ -81,12 +84,7 @@ FLUTTER_ASSERT_ARC } - (NSArray*)installedInputViews { - UIWindow* keyWindow = - [[[UIApplication sharedApplication] windows] - filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"isKeyWindow == YES"]] - .firstObject; - - return [keyWindow.subviews + return [textInputPlugin.textInputParentView.subviews filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"self isKindOfClass: %@", [FlutterTextInputView class]]]; } @@ -407,6 +405,12 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(textInputPlugin.autofillContext.count, 0); } +- (void)ensureOnlyActiveViewCanBecomeFirstResponder { + for (FlutterTextInputView* inputView in self.installedInputViews) { + XCTAssertEqual(inputView.canBecomeFirstResponder, inputView == textInputPlugin.activeView); + } +} + #pragma mark - Autofill - Tests - (void)testAutofillContext { @@ -434,8 +438,11 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(self.viewsVisibleToAutofill.count, 2); XCTAssertEqual(textInputPlugin.autofillContext.count, 2); + + [textInputPlugin collectGarbageInputViews]; XCTAssertEqual(self.installedInputViews.count, 2); XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // The configuration changes. NSMutableDictionary* field3 = self.mutablePasswordTemplateCopy; @@ -454,8 +461,11 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(self.viewsVisibleToAutofill.count, 2); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); + + [textInputPlugin collectGarbageInputViews]; XCTAssertEqual(self.installedInputViews.count, 3); XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Old autofill input fields are still installed and reused. for (NSString* key in oldContext.allKeys) { @@ -467,9 +477,12 @@ FLUTTER_ASSERT_ARC oldContext = textInputPlugin.autofillContext; [self setClientId:124 configuration:config]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); + + [textInputPlugin collectGarbageInputViews]; XCTAssertEqual(self.installedInputViews.count, 4); // Old autofill input fields are still installed and reused. @@ -478,6 +491,7 @@ FLUTTER_ASSERT_ARC } // The active view should change. XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Switch to a similar password field, the previous field should be reused. oldContext = textInputPlugin.autofillContext; @@ -486,6 +500,8 @@ FLUTTER_ASSERT_ARC // Reuse the input view instance from the last time. XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 3); + + [textInputPlugin collectGarbageInputViews]; XCTAssertEqual(self.installedInputViews.count, 4); // Old autofill input fields are still installed and reused. @@ -493,6 +509,7 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(oldContext[key], textInputPlugin.autofillContext[key]); } XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; } - (void)testCommitAutofillContext { @@ -526,21 +543,27 @@ FLUTTER_ASSERT_ARC [self setClientId:123 configuration:config]; XCTAssertEqual(self.viewsVisibleToAutofill.count, 2); XCTAssertEqual(textInputPlugin.autofillContext.count, 2); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; [self commitAutofillContextAndVerify]; XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Install the password field again. [self setClientId:123 configuration:config]; // Switch to a regular autofill group. [self setClientId:124 configuration:field3]; XCTAssertEqual(self.viewsVisibleToAutofill.count, 1); + + [textInputPlugin collectGarbageInputViews]; 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]; @@ -549,11 +572,15 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); // The active view should still be installed so it doesn't get // deallocated. + + [textInputPlugin collectGarbageInputViews]; XCTAssertEqual(self.installedInputViews.count, 1); XCTAssertEqual(textInputPlugin.autofillContext.count, 0); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; [self commitAutofillContextAndVerify]; XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView); + [self ensureOnlyActiveViewCanBecomeFirstResponder]; } - (void)testAutofillInputViews { @@ -577,6 +604,7 @@ FLUTTER_ASSERT_ARC [config setValue:@[ field1, field2 ] forKey:@"fields"]; [self setClientId:123 configuration:config]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Find all the FlutterTextInputViews we created. NSArray* inputFields = self.installedInputViews; @@ -589,6 +617,7 @@ FLUTTER_ASSERT_ARC FlutterTextInputView* inactiveView = inputFields[1]; [inactiveView replaceRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(0, 0)] withText:@"Autofilled!"]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; // Verify behavior. OCMVerify([engine updateEditingClient:0 withState:[OCMArg isNotNil] withTag:@"field2"]); @@ -610,4 +639,61 @@ FLUTTER_ASSERT_ARC XCTAssertNotEqual([inputView performSelector:@selector(font)], nil); } +- (void)testClearAutofillContextClearsSelection { + NSMutableDictionary* regularField = self.mutableTemplateCopy; + NSDictionary* editingValue = @{ + @"text" : @"REGULAR_TEXT_FIELD", + @"composingBase" : @0, + @"composingExtent" : @3, + @"selectionBase" : @1, + @"selectionExtent" : @4 + }; + [regularField setValue:@{ + @"uniqueIdentifier" : @"field2", + @"hints" : @[ @"hint2" ], + @"editingValue" : editingValue, + } + forKey:@"autofill"]; + [regularField addEntriesFromDictionary:editingValue]; + [self setClientId:123 configuration:regularField]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; + XCTAssertEqual(self.installedInputViews.count, 1); + + FlutterTextInputView* oldInputView = self.installedInputViews[0]; + XCTAssert([oldInputView.text isEqualToString:@"REGULAR_TEXT_FIELD"]); + FlutterTextRange* selectionRange = (FlutterTextRange*)oldInputView.selectedTextRange; + XCTAssert(NSEqualRanges(selectionRange.range, NSMakeRange(1, 3))); + + // Replace the original password field with new one. This should remove + // the old password field, but not immediately. + [self setClientId:124 configuration:self.mutablePasswordTemplateCopy]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; + + XCTAssertEqual(self.installedInputViews.count, 2); + + [textInputPlugin collectGarbageInputViews]; + XCTAssertEqual(self.installedInputViews.count, 1); + + // Verify the old input view is properly cleaned up. + XCTAssert([oldInputView.text isEqualToString:@""]); + selectionRange = (FlutterTextRange*)oldInputView.selectedTextRange; + XCTAssert(NSEqualRanges(selectionRange.range, NSMakeRange(0, 0))); +} + +- (void)testGarbageInputViewsAreNotRemovedImmediately { + // Add a password field that should autofill. + [self setClientId:123 configuration:self.mutablePasswordTemplateCopy]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; + + XCTAssertEqual(self.installedInputViews.count, 1); + // Add an input field that doesn't autofill. This should remove the password + // field, but not immediately. + [self setClientId:124 configuration:self.mutableTemplateCopy]; + [self ensureOnlyActiveViewCanBecomeFirstResponder]; + + XCTAssertEqual(self.installedInputViews.count, 2); + + [self commitAutofillContextAndVerify]; +} + @end