From 92b50c226878671e634ffe16b63218e1cbd44ffc Mon Sep 17 00:00:00 2001 From: LongCat is Looong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 18 Jun 2020 21:31:28 -0700 Subject: [PATCH 1/3] remove redundant framework sync from iOS text input plugin --- .../Source/FlutterTextInputPlugin.mm | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 20e36c4595d85..68d9c079b0e0d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -325,7 +325,8 @@ - (void)setTextInputClient:(int)client { _textInputClient = client; } -- (void)setTextInputState:(NSDictionary*)state { +// Return true if the new input state needs to be synced back to the framework. +- (BOOL)setTextInputState:(NSDictionary*)state { NSString* newText = state[@"text"]; BOOL textChanged = ![self.text isEqualToString:newText]; if (textChanged) { @@ -356,8 +357,7 @@ - (void)setTextInputState:(NSDictionary*)state { selectedRange.length != oldSelectedRange.length) { needsEditingStateUpdate = YES; [self.inputDelegate selectionWillChange:self]; - [self setSelectedTextRange:[FlutterTextRange rangeWithNSRange:selectedRange] - updateEditingState:NO]; + [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:selectedRange]]; _selectionAffinity = _kTextAffinityDownstream; if ([state[@"selectionAffinity"] isEqualToString:@(_kTextAffinityUpstream)]) _selectionAffinity = _kTextAffinityUpstream; @@ -367,10 +367,9 @@ - (void)setTextInputState:(NSDictionary*)state { if (textChanged) { [self.inputDelegate textDidChange:self]; } - if (needsEditingStateUpdate) { - // For consistency with Android behavior, send an update to the framework. - [self updateEditingState]; - } + + // For consistency with Android behavior, send an update to the framework if anything changed. + return needsEditingStateUpdate; } - (NSRange)clampSelection:(NSRange)range forText:(NSString*)text { @@ -401,11 +400,7 @@ - (UITextRange*)selectedTextRange { return [[_selectedTextRange copy] autorelease]; } -- (void)setSelectedTextRange:(UITextRange*)selectedTextRange { - [self setSelectedTextRange:selectedTextRange updateEditingState:YES]; -} - -- (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState:(BOOL)update { +- (void)_setSelectedTextRange:(UITextRange*)selectedTextRange { if (_selectedTextRange != selectedTextRange) { UITextRange* oldSelectedRange = _selectedTextRange; if (self.hasText) { @@ -416,12 +411,14 @@ - (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState: _selectedTextRange = [selectedTextRange copy]; } [oldSelectedRange release]; - - if (update) - [self updateEditingState]; } } +- (void)setSelectedTextRange:(UITextRange*)selectedTextRange { + [self _setSelectedTextRange:selectedTextRange]; + [self updateEditingState]; +} + - (id)insertDictationResultPlaceholder { return @""; } @@ -440,26 +437,29 @@ - (NSString*)textInRange:(UITextRange*)range { return [self.text substringWithRange:textRange]; } -- (void)replaceRange:(UITextRange*)range withText:(NSString*)text { - NSRange replaceRange = ((FlutterTextRange*)range).range; +- (void)_replaceRange:(NSRange)range withText:(NSString*)text { NSRange selectedRange = _selectedTextRange.range; + // Adjust the text selection: // * reduce the length by the intersection length // * adjust the location by newLength - oldLength + intersectionLength - NSRange intersectionRange = NSIntersectionRange(replaceRange, selectedRange); - if (replaceRange.location <= selectedRange.location) - selectedRange.location += text.length - replaceRange.length; + NSRange intersectionRange = NSIntersectionRange(range, selectedRange); + if (range.location <= selectedRange.location) + selectedRange.location += text.length - range.length; if (intersectionRange.location != NSNotFound) { selectedRange.location += intersectionRange.length; selectedRange.length -= intersectionRange.length; } - [self.text replaceCharactersInRange:[self clampSelection:replaceRange forText:self.text] + [self.text replaceCharactersInRange:[self clampSelection:range forText:self.text] withString:text]; - [self setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange - forText:self.text]] - updateEditingState:NO]; + [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange + forText:self.text]]]; +} +- (void)replaceRange:(UITextRange*)range withText:(NSString*)text { + NSRange replaceRange = ((FlutterTextRange*)range).range; + [self _replaceRange:replaceRange withText:text]; [self updateEditingState]; } @@ -522,11 +522,11 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte if (markedTextRange.length > 0) { // Replace text in the marked range with the new text. - [self replaceRange:self.markedTextRange withText:markedText]; + [self _replaceRange:markedTextRange withText:markedText]; markedTextRange.length = markedText.length; } else { // Replace text in the selected range with the new text. - [self replaceRange:_selectedTextRange withText:markedText]; + [self _replaceRange:selectedRange withText:markedText]; markedTextRange = NSMakeRange(selectedRange.location, markedText.length); } @@ -535,9 +535,9 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte NSUInteger selectionLocation = markedSelectedRange.location + markedTextRange.location; selectedRange = NSMakeRange(selectionLocation, markedSelectedRange.length); - [self setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange - forText:self.text]] - updateEditingState:YES]; + [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange + forText:self.text]]]; + [self updateEditingState]; } - (void)unmarkText { @@ -1002,7 +1002,9 @@ + (void)setupInputView:(FlutterTextInputView*)inputView } - (void)setTextInputEditingState:(NSDictionary*)state { - [_activeView setTextInputState:state]; + if ([_activeView setTextInputState:state]) { + [_activeView updateEditingState]; + } } - (void)clearTextInputClient { From 535959d609e84b7dc68c4692075f5a652c899502 Mon Sep 17 00:00:00 2001 From: LongCat is Looong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 19 Jun 2020 09:50:52 -0700 Subject: [PATCH 2/3] add tests --- .../Source/FlutterTextInputPluginTest.m | 94 ++++++++++--------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m index 4f20c4d9da23e..2a33beedbab4b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m @@ -17,13 +17,27 @@ @interface FlutterTextInputView () - (void)setTextInputState:(NSDictionary*)state; @end -@implementation FlutterTextInputPluginTest -- (void)testSecureInput { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputPlugin* textInputPlugin = [[FlutterTextInputPlugin alloc] init]; +@implementation FlutterTextInputPluginTest { + id engine; + FlutterTextInputPlugin* textInputPlugin; +} + +- (void)setUp { + [super setUp]; + + engine = OCMClassMock([FlutterEngine class]); + textInputPlugin = [[FlutterTextInputPlugin alloc] init]; textInputPlugin.textInputDelegate = engine; +} +- (void)tearDown { + [engine stopMocking]; + [[[[textInputPlugin textInputView] superview] subviews] + makeObjectsPerformSelector:@selector(removeFromSuperview)]; + [super tearDown]; +} + +- (void)testSecureInput { NSDictionary* config = @{ @"inputType" : @{@"name" : @"TextInuptType.text"}, @"keyboardAppearance" : @"Brightness.light", @@ -61,17 +75,9 @@ - (void)testSecureInput { // The one FlutterTextInputView we inserted into the view hierarchy should be the text input // plugin's active text input view. XCTAssertEqual(inputView, textInputPlugin.textInputView); - - // Clean up. - [engine stopMocking]; - [[[[textInputPlugin textInputView] superview] subviews] - makeObjectsPerformSelector:@selector(removeFromSuperview)]; } - (void)testTextChangesTriggerUpdateEditingClient { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; @@ -86,15 +92,9 @@ - (void)testTextChangesTriggerUpdateEditingClient { // Don't send anything if there's nothing new. [inputView setTextInputState:@{@"text" : @"AFTER"}]; OCMReject([engine updateEditingClient:0 withState:[OCMArg any]]); - - // Clean up. - [engine stopMocking]; } - (void)testSelectionChangeTriggersUpdateEditingClient { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; @@ -118,15 +118,9 @@ - (void)testSelectionChangeTriggersUpdateEditingClient { [inputView setTextInputState:@{@"text" : @"SELECTION", @"selectionBase" : @1, @"selectionExtent" : @2}]; OCMReject([engine updateEditingClient:0 withState:[OCMArg any]]); - - // Clean up. - [engine stopMocking]; } - (void)testComposingChangeTriggersUpdateEditingClient { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; inputView.textInputDelegate = engine; @@ -151,17 +145,9 @@ - (void)testComposingChangeTriggersUpdateEditingClient { [inputView setTextInputState:@{@"text" : @"COMPOSING", @"composingBase" : @1, @"composingExtent" : @2}]; OCMReject([engine updateEditingClient:0 withState:[OCMArg any]]); - - // Clean up. - [engine stopMocking]; } - (void)testAutofillInputViews { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputPlugin* textInputPlugin = [[FlutterTextInputPlugin alloc] init]; - textInputPlugin.textInputDelegate = engine; - NSDictionary* template = @{ @"inputType" : @{@"name" : @"TextInuptType.text"}, @"keyboardAppearance" : @"Brightness.light", @@ -214,26 +200,15 @@ - (void)testAutofillInputViews { // Verify behavior. OCMVerify([engine updateEditingClient:0 withState:[OCMArg isNotNil] withTag:@"field2"]); - - // Clean up. - [engine stopMocking]; - [[[[textInputPlugin textInputView] superview] subviews] - makeObjectsPerformSelector:@selector(removeFromSuperview)]; } - (void)testAutocorrectionPromptRectAppears { - // Setup test. - id engine = OCMClassMock([FlutterEngine class]); - FlutterTextInputView* inputView = [[FlutterTextInputView alloc] initWithFrame:CGRectZero]; inputView.textInputDelegate = engine; [inputView firstRectForRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(0, 1)]]; // Verify behavior. OCMVerify([engine showAutocorrectionPromptRectForStart:0 end:1 withClient:0]); - - // Clean up mocks - [engine stopMocking]; } - (void)testTextRangeFromPositionMatchesUITextViewBehavior { @@ -248,4 +223,35 @@ - (void)testTextRangeFromPositionMatchesUITextViewBehavior { XCTAssertEqual(range.location, 0); XCTAssertEqual(range.length, 2); } + +- (void)testUITextInputCallsUpdateEditingStateOnce { + FlutterTextInputView* inputView = [[FlutterTextInputView alloc] init]; + inputView.textInputDelegate = engine; + + __block int updateCount = 0; + OCMStub([engine updateEditingClient:0 withState:[OCMArg isNotNil]]) + .andDo(^(NSInvocation* invocation) { + updateCount++; + }); + + [inputView insertText:@"text to insert"]; + // Update the framework exactly once. + XCTAssertEqual(updateCount, 1); + + [inputView deleteBackward]; + XCTAssertEqual(updateCount, 2); + + inputView.selectedTextRange = [FlutterTextRange rangeWithNSRange:NSMakeRange(0, 1)]; + XCTAssertEqual(updateCount, 3); + + [inputView replaceRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(0, 1)] + withText:@"replace text"]; + XCTAssertEqual(updateCount, 4); + + [inputView setMarkedText:@"marked text" selectedRange:NSMakeRange(0, 1)]; + XCTAssertEqual(updateCount, 5); + + [inputView unmarkText]; + XCTAssertEqual(updateCount, 6); +} @end From ac89f3e03e9d848395c1b9ace10cf617e10f3667 Mon Sep 17 00:00:00 2001 From: LongCat is Looong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 19 Jun 2020 13:18:07 -0700 Subject: [PATCH 3/3] renaming private methods --- .../Source/FlutterTextInputPlugin.mm | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 68d9c079b0e0d..dc85fbee70692 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -357,7 +357,7 @@ - (BOOL)setTextInputState:(NSDictionary*)state { selectedRange.length != oldSelectedRange.length) { needsEditingStateUpdate = YES; [self.inputDelegate selectionWillChange:self]; - [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:selectedRange]]; + [self setSelectedTextRangeLocal:[FlutterTextRange rangeWithNSRange:selectedRange]]; _selectionAffinity = _kTextAffinityDownstream; if ([state[@"selectionAffinity"] isEqualToString:@(_kTextAffinityUpstream)]) _selectionAffinity = _kTextAffinityUpstream; @@ -400,7 +400,8 @@ - (UITextRange*)selectedTextRange { return [[_selectedTextRange copy] autorelease]; } -- (void)_setSelectedTextRange:(UITextRange*)selectedTextRange { +// Change the range of selected text, without notifying the framework. +- (void)setSelectedTextRangeLocal:(UITextRange*)selectedTextRange { if (_selectedTextRange != selectedTextRange) { UITextRange* oldSelectedRange = _selectedTextRange; if (self.hasText) { @@ -415,7 +416,7 @@ - (void)_setSelectedTextRange:(UITextRange*)selectedTextRange { } - (void)setSelectedTextRange:(UITextRange*)selectedTextRange { - [self _setSelectedTextRange:selectedTextRange]; + [self setSelectedTextRangeLocal:selectedTextRange]; [self updateEditingState]; } @@ -437,7 +438,9 @@ - (NSString*)textInRange:(UITextRange*)range { return [self.text substringWithRange:textRange]; } -- (void)_replaceRange:(NSRange)range withText:(NSString*)text { +// Replace the text within the specified range with the given text, +// without notifying the framework. +- (void)replaceRangeLocal:(NSRange)range withText:(NSString*)text { NSRange selectedRange = _selectedTextRange.range; // Adjust the text selection: @@ -453,13 +456,14 @@ - (void)_replaceRange:(NSRange)range withText:(NSString*)text { [self.text replaceCharactersInRange:[self clampSelection:range forText:self.text] withString:text]; - [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange - forText:self.text]]]; + [self setSelectedTextRangeLocal:[FlutterTextRange + rangeWithNSRange:[self clampSelection:selectedRange + forText:self.text]]]; } - (void)replaceRange:(UITextRange*)range withText:(NSString*)text { NSRange replaceRange = ((FlutterTextRange*)range).range; - [self _replaceRange:replaceRange withText:text]; + [self replaceRangeLocal:replaceRange withText:text]; [self updateEditingState]; } @@ -522,11 +526,11 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte if (markedTextRange.length > 0) { // Replace text in the marked range with the new text. - [self _replaceRange:markedTextRange withText:markedText]; + [self replaceRangeLocal:markedTextRange withText:markedText]; markedTextRange.length = markedText.length; } else { // Replace text in the selected range with the new text. - [self _replaceRange:selectedRange withText:markedText]; + [self replaceRangeLocal:selectedRange withText:markedText]; markedTextRange = NSMakeRange(selectedRange.location, markedText.length); } @@ -535,8 +539,9 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte NSUInteger selectionLocation = markedSelectedRange.location + markedTextRange.location; selectedRange = NSMakeRange(selectionLocation, markedSelectedRange.length); - [self _setSelectedTextRange:[FlutterTextRange rangeWithNSRange:[self clampSelection:selectedRange - forText:self.text]]]; + [self setSelectedTextRangeLocal:[FlutterTextRange + rangeWithNSRange:[self clampSelection:selectedRange + forText:self.text]]]; [self updateEditingState]; }