Skip to content

Commit

Permalink
Make android more lenient when it comes to out-of-order key event res…
Browse files Browse the repository at this point in the history 
…ponses (#23604)

Relaxes enforcement of key events being handled in order, to match similar code in the Linux and Windows implementations.
  • Loading branch information
@gspencergoog
gspencergoog committed Jan 21, 2021
1 parent 1474d08 commit 3da13fc
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 31 deletions.
49 changes: 21 additions & 28 deletions shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java
View file
Expand Up @@ -14,6 +14,7 @@
import io.flutter.plugin.editing.TextInputPlugin;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Iterator;

/**
* A class to process key events from Android, passing them to the framework as messages using
Expand Down Expand Up @@ -93,11 +94,11 @@ public boolean onKeyEvent(@NonNull KeyEvent keyEvent) {
// case the theory is wrong.
return false;
}
if (eventResponder.isHeadEvent(keyEvent)) {
// If the keyEvent is at the head of the queue of pending events we've seen,
// and has the same id, then we know that this is a re-dispatched keyEvent, and
// we shouldn't respond to it, but we should remove it from tracking now.
eventResponder.removeHeadEvent();
if (isPendingEvent(keyEvent)) {
// If the keyEvent is in the queue of pending events we've seen, and has
// the same id, then we know that this is a re-dispatched keyEvent, and we
// shouldn't respond to it, but we should remove it from tracking now.
eventResponder.removePendingEvent(keyEvent);
return false;
}

Expand All @@ -122,8 +123,8 @@ public boolean onKeyEvent(@NonNull KeyEvent keyEvent) {
* @param event the event to check for being the current event.
* @return
*/
public boolean isCurrentEvent(@NonNull KeyEvent event) {
return eventResponder.isHeadEvent(event);
public boolean isPendingEvent(@NonNull KeyEvent event) {
return eventResponder.findPendingEvent(event) != null;
}

/**
Expand Down Expand Up @@ -199,27 +200,19 @@ public EventResponder(@NonNull View view, @NonNull TextInputPlugin textInputPlug
}

/** Removes the first pending event from the cache of pending events. */
private KeyEvent removeHeadEvent() {
return pendingEvents.removeFirst();
private void removePendingEvent(KeyEvent event) {
pendingEvents.remove(event);
}

private KeyEvent checkIsHeadEvent(KeyEvent event) {
if (pendingEvents.size() == 0) {
throw new AssertionError(
"Event response received when no events are in the queue. Received event " + event);
}
if (pendingEvents.getFirst() != event) {
throw new AssertionError(
"Event response received out of order. Should have seen event "
+ pendingEvents.getFirst()
+ " first. Instead, received "
+ event);
private KeyEvent findPendingEvent(KeyEvent event) {
Iterator<KeyEvent> iter = pendingEvents.iterator();
while (iter.hasNext()) {
KeyEvent item = iter.next();
if (item == event) {
return item;
}
}
return pendingEvents.getFirst();
}

private boolean isHeadEvent(KeyEvent event) {
return pendingEvents.size() > 0 && pendingEvents.getFirst() == event;
return null;
}

/**
Expand All @@ -229,7 +222,7 @@ private boolean isHeadEvent(KeyEvent event) {
*/
@Override
public void onKeyEventHandled(KeyEvent event) {
removeHeadEvent();
removePendingEvent(event);
}

/**
Expand All @@ -240,7 +233,7 @@ public void onKeyEventHandled(KeyEvent event) {
*/
@Override
public void onKeyEventNotHandled(KeyEvent event) {
redispatchKeyEvent(checkIsHeadEvent(event));
redispatchKeyEvent(findPendingEvent(event));
}

/** Adds an Android key event to the event responder to wait for a response. */
Expand Down Expand Up @@ -269,7 +262,7 @@ private void redispatchKeyEvent(KeyEvent event) {
&& textInputPlugin.getLastInputConnection() != null
&& textInputPlugin.getLastInputConnection().sendKeyEvent(event)) {
// The event was handled, so we can remove it from the queue.
removeHeadEvent();
removePendingEvent(event);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java
View file
Expand Up @@ -299,7 +299,7 @@ public boolean sendKeyEvent(KeyEvent event) {
// already know about (i.e. when events arrive here from a soft keyboard and
// not a hardware keyboard), to avoid a loop.
if (keyProcessor != null
&& !keyProcessor.isCurrentEvent(event)
&& !keyProcessor.isPendingEvent(event)
&& keyProcessor.onKeyEvent(event)) {
return true;
}
Expand Down
123 changes: 123 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java
View file
Expand Up @@ -74,6 +74,54 @@ public void destroyTest() {
.setEventResponseHandler(isNull(KeyEventChannel.EventResponseHandler.class));
}

public void removesPendingEventsWhenKeyDownHandled() {
FlutterEngine flutterEngine = mockFlutterEngine();
KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
View fakeView = mock(View.class);
View fakeRootView = mock(View.class);
when(fakeView.getRootView())
.then(
new Answer<View>() {
@Override
public View answer(InvocationOnMock invocation) throws Throwable {
return fakeRootView;
}
});

ArgumentCaptor<KeyEventChannel.EventResponseHandler> handlerCaptor =
ArgumentCaptor.forClass(KeyEventChannel.EventResponseHandler.class);
verify(fakeKeyEventChannel).setEventResponseHandler(handlerCaptor.capture());
AndroidKeyProcessor processor =
new AndroidKeyProcessor(fakeView, fakeKeyEventChannel, mock(TextInputPlugin.class));
ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> eventCaptor =
ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);

boolean result = processor.onKeyEvent(fakeKeyEvent);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
assertEquals(true, result);

// Capture the FlutterKeyEvent so we can find out its event ID to use when
// faking our response.
verify(fakeKeyEventChannel, times(1)).keyDown(eventCaptor.capture());
boolean[] dispatchResult = {true};
when(fakeView.dispatchKeyEvent(any(KeyEvent.class)))
.then(
new Answer<Boolean>() {
@Override
public Boolean answer(InvocationOnMock invocation) throws Throwable {
KeyEvent event = (KeyEvent) invocation.getArguments()[0];
assertEquals(fakeKeyEvent, event);
dispatchResult[0] = processor.onKeyEvent(event);
return dispatchResult[0];
}
});

// Fake a response from the framework.
handlerCaptor.getValue().onKeyEventHandled(eventCaptor.getValue().event);
assertEquals(false, processor.isPendingEvent(fakeKeyEvent));
}

public void synthesizesEventsWhenKeyDownNotHandled() {
FlutterEngine flutterEngine = mockFlutterEngine();
KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
Expand All @@ -98,6 +146,7 @@ public View answer(InvocationOnMock invocation) throws Throwable {
FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);

boolean result = processor.onKeyEvent(fakeKeyEvent);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
assertEquals(true, result);

// Capture the FlutterKeyEvent so we can find out its event ID to use when
Expand All @@ -118,6 +167,7 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable {

// Fake a response from the framework.
handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
assertEquals(false, dispatchResult[0]);
verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
Expand Down Expand Up @@ -148,6 +198,7 @@ public View answer(InvocationOnMock invocation) throws Throwable {
FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_UP, 65);

boolean result = processor.onKeyEvent(fakeKeyEvent);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
assertEquals(true, result);

// Capture the FlutterKeyEvent so we can find out its event ID to use when
Expand All @@ -168,12 +219,84 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable {

// Fake a response from the framework.
handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
assertEquals(false, dispatchResult[0]);
verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent);
}

public void respondsCorrectlyWhenEventsAreReturnedOutOfOrder() {
FlutterEngine flutterEngine = mockFlutterEngine();
KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
View fakeView = mock(View.class);
View fakeRootView = mock(View.class);
when(fakeView.getRootView())
.then(
new Answer<View>() {
@Override
public View answer(InvocationOnMock invocation) throws Throwable {
return fakeRootView;
}
});

ArgumentCaptor<KeyEventChannel.EventResponseHandler> handlerCaptor =
ArgumentCaptor.forClass(KeyEventChannel.EventResponseHandler.class);
verify(fakeKeyEventChannel).setEventResponseHandler(handlerCaptor.capture());
AndroidKeyProcessor processor =
new AndroidKeyProcessor(fakeView, fakeKeyEventChannel, mock(TextInputPlugin.class));
ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> event1Captor =
ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> event2Captor =
ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
FakeKeyEvent fakeKeyEvent1 = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);
FakeKeyEvent fakeKeyEvent2 = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 20);

boolean result1 = processor.onKeyEvent(fakeKeyEvent1);
boolean result2 = processor.onKeyEvent(fakeKeyEvent2);
assertEquals(true, processor.isPendingEvent(fakeKeyEvent1));
assertEquals(true, processor.isPendingEvent(fakeKeyEvent2));
assertEquals(true, result1);
assertEquals(true, result2);

// Capture the FlutterKeyEvent so we can find out its event ID to use when
// faking our response.
verify(fakeKeyEventChannel, times(1)).keyDown(event1Captor.capture());
verify(fakeKeyEventChannel, times(1)).keyDown(event2Captor.capture());
boolean[] dispatchResult = {true, true};
when(fakeView.dispatchKeyEvent(any(KeyEvent.class)))
.then(
new Answer<Boolean>() {
@Override
public Boolean answer(InvocationOnMock invocation) throws Throwable {
KeyEvent event = (KeyEvent) invocation.getArguments()[0];
assertEquals(true, fakeKeyEvent1 == event || fakeKeyEvent2 == event);
if (fakeKeyEvent1 == event) {
dispatchResult[0] = processor.onKeyEvent(fakeKeyEvent1);
return dispatchResult[0];
} else {
dispatchResult[1] = processor.onKeyEvent(fakeKeyEvent2);
return dispatchResult[1];
}
}
});

assertEquals(true, processor.isPendingEvent(fakeKeyEvent1));
assertEquals(true, processor.isPendingEvent(fakeKeyEvent2));

// Fake a "handled" response from the framework, but do it in reverse order.
handlerCaptor.getValue().onKeyEventNotHandled(event2Captor.getValue().event);
handlerCaptor.getValue().onKeyEventNotHandled(event1Captor.getValue().event);

verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent1);
verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent2);
assertEquals(false, dispatchResult[0]);
assertEquals(false, dispatchResult[1]);
verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent1);
verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent2);
}

@NonNull
private FlutterEngine mockFlutterEngine() {
// Mock FlutterEngine and all of its required direct calls.
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java
View file
Expand Up @@ -1033,7 +1033,7 @@ public void testCursorAnchorInfo() {
public void testSendKeyEvent_sendSoftKeyEvents() {
ListenableEditingState editable = sampleEditable(5, 5);
AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class);
when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(true);
when(mockKeyProcessor.isPendingEvent(any())).thenReturn(true);
InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor);

KeyEvent shiftKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SHIFT_LEFT);
Expand All @@ -1047,7 +1047,7 @@ public void testSendKeyEvent_sendSoftKeyEvents() {
public void testSendKeyEvent_sendHardwareKeyEvents() {
ListenableEditingState editable = sampleEditable(5, 5);
AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class);
when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(false);
when(mockKeyProcessor.isPendingEvent(any())).thenReturn(false);
when(mockKeyProcessor.onKeyEvent(any())).thenReturn(true);
InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor);

Expand Down

0 comments on commit 3da13fc

Please sign in to comment.