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

Implement repeat filtering logic in Android Embedder #17509

Merged
merged 13 commits into from
Apr 8, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ class InputConnectionAdaptor extends BaseInputConnection {
private InputMethodManager mImm;
private final Layout mLayout;

// Used to store the last-sent values via updateEditingState to the framework.
// These are then compared against to prevent redundant messages with the same
// data before any valid operations were made to the contents.
private int mPreviousSelectionStart;
Copy link
Member

Choose a reason for hiding this comment

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

Consider moving the selection start/end, composing start/end, and text fields into a class. updateEditingState would construct an instance of the class from the mEditable and call its equals method to check whether it matches the previous instance.

private int mPreviousSelectionEnd;
private int mPreviousComposingStart;
private int mPreviousComposingEnd;
private String mPreviousText;
private boolean mRepeatCheckNeeded = false;

// Used to determine if Samsung-specific hacks should be applied.
private final boolean isSamsung;

Expand Down Expand Up @@ -80,11 +90,41 @@ private void updateEditingState() {
int selectionEnd = Selection.getSelectionEnd(mEditable);
int composingStart = BaseInputConnection.getComposingSpanStart(mEditable);
int composingEnd = BaseInputConnection.getComposingSpanEnd(mEditable);
String text = mEditable.toString();

// Return if this data has already been sent and no meaningful changes have
// occurred to mark this as dirty. This prevents duplicate remote updates of
// the same data, which can break formatters that change the length of the
// contents.
if (mRepeatCheckNeeded
&& selectionStart == mPreviousSelectionStart
&& selectionEnd == mPreviousSelectionEnd
&& composingStart == mPreviousComposingStart
&& composingEnd == mPreviousComposingEnd
&& text.equals(mPreviousText)) {
return;
}

mImm.updateSelection(mFlutterView, selectionStart, selectionEnd, composingStart, composingEnd);

textInputChannel.updateEditingState(
mClient, mEditable.toString(), selectionStart, selectionEnd, composingStart, composingEnd);
mClient, text, selectionStart, selectionEnd, composingStart, composingEnd);

mRepeatCheckNeeded = true;
mPreviousSelectionStart = selectionStart;
mPreviousSelectionEnd = selectionEnd;
mPreviousComposingStart = composingStart;
mPreviousComposingEnd = composingEnd;
mPreviousText = text;
}

// This should be called whenever a change could have been made to
// the value of mEditable, which will make any call of updateEditingState()
// ineligible for repeat checking as we do not want to skip sending real changes
// to the framework.
public void markDirty() {
// Disable updateEditngState's repeat-update check
mRepeatCheckNeeded = false;
}

@Override
Expand All @@ -109,7 +149,7 @@ public boolean endBatchEdit() {
@Override
public boolean commitText(CharSequence text, int newCursorPosition) {
boolean result = super.commitText(text, newCursorPosition);
updateEditingState();
markDirty();
return result;
}

Expand All @@ -118,14 +158,21 @@ public boolean deleteSurroundingText(int beforeLength, int afterLength) {
if (Selection.getSelectionStart(mEditable) == -1) return true;

boolean result = super.deleteSurroundingText(beforeLength, afterLength);
updateEditingState();
markDirty();
return result;
}

@Override
public boolean deleteSurroundingTextInCodePoints(int beforeLength, int afterLength) {
boolean result = super.deleteSurroundingTextInCodePoints(beforeLength, afterLength);
markDirty();
return result;
}

@Override
public boolean setComposingRegion(int start, int end) {
boolean result = super.setComposingRegion(start, end);
updateEditingState();
markDirty();
return result;
}

Expand All @@ -137,7 +184,7 @@ public boolean setComposingText(CharSequence text, int newCursorPosition) {
} else {
result = super.setComposingText(text, newCursorPosition);
}
updateEditingState();
markDirty();
return result;
}

Expand All @@ -159,7 +206,7 @@ public boolean finishComposingText() {
}
}

updateEditingState();
markDirty();
return result;
}

Expand All @@ -173,6 +220,13 @@ public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) {
return extractedText;
}

@Override
public boolean clearMetaKeyStates(int states) {
boolean result = super.clearMetaKeyStates(states);
markDirty();
return result;
}

// Detect if the keyboard is a Samsung keyboard, where we apply Samsung-specific hacks to
// fix critical bugs that make the keyboard otherwise unusable. See finishComposingText() for
// more details.
Expand All @@ -197,7 +251,7 @@ private boolean isSamsung() {
@Override
public boolean setSelection(int start, int end) {
boolean result = super.setSelection(start, end);
updateEditingState();
markDirty();
return result;
}

Expand All @@ -219,6 +273,7 @@ private static int clampIndexToEditable(int index, Editable editable) {

@Override
public boolean sendKeyEvent(KeyEvent event) {
markDirty();
if (event.getAction() == KeyEvent.ACTION_DOWN) {
if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
int selStart = clampIndexToEditable(Selection.getSelectionStart(mEditable), mEditable);
Expand Down Expand Up @@ -344,6 +399,7 @@ public boolean sendKeyEvent(KeyEvent event) {

@Override
public boolean performContextMenuAction(int id) {
markDirty();
if (id == android.R.id.selectAll) {
setSelection(0, mEditable.length());
return true;
Expand Down Expand Up @@ -397,6 +453,7 @@ public boolean performContextMenuAction(int id) {

@Override
public boolean performEditorAction(int actionCode) {
markDirty();
switch (actionCode) {
case EditorInfo.IME_ACTION_NONE:
textInputChannel.newline(mClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ void setTextInputEditingState(View view, TextInputChannel.TextEditState state) {
}
// Always apply state to selection which handles updating the selection if needed.
applyStateToSelection(state);
InputConnection connection = getLastInputConnection();
if (connection != null && connection instanceof InputConnectionAdaptor) {
((InputConnectionAdaptor) connection).markDirty();
}
// Use updateSelection to update imm on selection if it is not neccessary to restart.
if (!restartAlwaysRequired && !mRestartInputPending) {
mImm.updateSelection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,49 @@ public void testMethod_getExtractedText() {
assertEquals(extractedText.selectionEnd, selStart);
}

@Test
public void inputConnectionAdaptor_RepeatFilter() throws NullPointerException {
View testView = new View(RuntimeEnvironment.application);
FlutterJNI mockFlutterJni = mock(FlutterJNI.class);
DartExecutor dartExecutor = spy(new DartExecutor(mockFlutterJni, mock(AssetManager.class)));
int inputTargetId = 0;
TestTextInputChannel textInputChannel = new TestTextInputChannel(dartExecutor);
Editable mEditable = Editable.Factory.getInstance().newEditable("");
Editable spyEditable = spy(mEditable);
EditorInfo outAttrs = new EditorInfo();
outAttrs.inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_FLAG_MULTI_LINE;

InputConnectionAdaptor inputConnectionAdaptor =
new InputConnectionAdaptor(
testView, inputTargetId, textInputChannel, spyEditable, outAttrs);

inputConnectionAdaptor.beginBatchEdit();
assertEquals(textInputChannel.updateEditingStateInvocations, 0);
inputConnectionAdaptor.setComposingText("I do not fear computers. I fear the lack of them.", 1);
assertEquals(textInputChannel.text, null);
assertEquals(textInputChannel.updateEditingStateInvocations, 0);
inputConnectionAdaptor.endBatchEdit();
assertEquals(textInputChannel.updateEditingStateInvocations, 1);
assertEquals(textInputChannel.text, "I do not fear computers. I fear the lack of them.");

inputConnectionAdaptor.beginBatchEdit();
assertEquals(textInputChannel.updateEditingStateInvocations, 1);
inputConnectionAdaptor.endBatchEdit();
assertEquals(textInputChannel.updateEditingStateInvocations, 1);

inputConnectionAdaptor.beginBatchEdit();
assertEquals(textInputChannel.text, "I do not fear computers. I fear the lack of them.");
assertEquals(textInputChannel.updateEditingStateInvocations, 1);
inputConnectionAdaptor.setSelection(3, 4);
assertEquals(textInputChannel.updateEditingStateInvocations, 1);
assertEquals(textInputChannel.selectionStart, 49);
assertEquals(textInputChannel.selectionEnd, 49);
inputConnectionAdaptor.endBatchEdit();
assertEquals(textInputChannel.updateEditingStateInvocations, 2);
assertEquals(textInputChannel.selectionStart, 3);
assertEquals(textInputChannel.selectionEnd, 4);
}

private static final String SAMPLE_TEXT =
"Lorem ipsum dolor sit amet," + "\nconsectetur adipiscing elit.";

Expand All @@ -285,4 +328,35 @@ private static InputConnectionAdaptor sampleInputConnectionAdaptor(Editable edit
TextInputChannel textInputChannel = mock(TextInputChannel.class);
return new InputConnectionAdaptor(testView, client, textInputChannel, editable, null);
}

private class TestTextInputChannel extends TextInputChannel {
public TestTextInputChannel(DartExecutor dartExecutor) {
super(dartExecutor);
}

public int inputClientId;
public String text;
public int selectionStart;
public int selectionEnd;
public int composingStart;
public int composingEnd;
public int updateEditingStateInvocations = 0;

@Override
public void updateEditingState(
int inputClientId,
String text,
int selectionStart,
int selectionEnd,
int composingStart,
int composingEnd) {
this.inputClientId = inputClientId;
this.text = text;
this.selectionStart = selectionStart;
this.selectionEnd = selectionEnd;
this.composingStart = composingStart;
this.composingEnd = composingEnd;
updateEditingStateInvocations++;
}
}
}