From 1789d2ddeeb2033a43bebce3975317622a332270 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 21 Jul 2022 12:54:55 -0700 Subject: [PATCH 1/3] Impl and test --- .../android/KeyEmbedderResponder.java | 39 ++++++++++----- .../android/KeyboardManagerTest.java | 50 +++++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java index 989ee9997d6ba..fc2488782c6f8 100644 --- a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java +++ b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java @@ -10,7 +10,9 @@ import io.flutter.embedding.android.KeyboardMap.PressingGoal; import io.flutter.embedding.android.KeyboardMap.TogglingGoal; import io.flutter.plugin.common.BinaryMessenger; +import java.util.ArrayList; import java.util.HashMap; +import java.util.concurrent.Callable; /** * A {@link KeyboardManager.Responder} of {@link KeyboardManager} that handles events by sending @@ -120,17 +122,25 @@ void updatePressingState(@NonNull Long physicalKey, @Nullable Long logicalKey) { // dispatches synthesized events so that the state of these keys matches the true state taking // the current event in consideration. // + // Events that should be synthesized before the main event are synthesized + // immediately, while events that should be syntehsized after the main event are appended to + // postSynthsize. + // // Although Android KeyEvent defined bitmasks for sided modifiers (SHIFT_LEFT_ON and // SHIFT_RIGHT_ON), // this function only uses the unsided modifiers (SHIFT_ON), due to the weird behaviors observed // on ChromeOS, where right modifiers produce events with UNSIDED | LEFT_SIDE meta state bits. void synchronizePressingKey( - PressingGoal goal, boolean truePressed, long eventLogicalKey, KeyEvent event) { + PressingGoal goal, + boolean truePressed, + long eventLogicalKey, + KeyEvent event, + ArrayList> postSynchronize) { // During an incoming event, there might be a synthesized Flutter event for each key of each // pressing goal, followed by an eventual main Flutter event. // - // NowState ----------------> PreEventState --------------> TrueState - // Synchronization Event + // NowState ----------------> PreEventState --------------> -------------->TrueState + // PreSynchronize Event PostSynchronize // // The goal of the synchronization algorithm is to derive a pre-event state that can satisfy the // true state (`truePressed`) after the event, and that requires as few synthesized events based @@ -148,10 +158,10 @@ void synchronizePressingKey( preEventStates[keyIdx] = false; postEventAnyPressed = true; if (!truePressed) { - throw new AssertionError( - String.format( - "Unexpected metaState 0 for key 0x%x during an ACTION_down event.", - eventLogicalKey)); + postSynchronize.add( + () -> { + synthesizeEvent(false, key.logicalKey, key.physicalKey, event.getEventTime()); + }); } break; case kUp: @@ -165,10 +175,10 @@ void synchronizePressingKey( // synthesize a down event here, or there will be a down event *and* a repeat event, // both of which have printable characters. Obviously don't synthesize up events either. if (!truePressed) { - throw new AssertionError( - String.format( - "Unexpected metaState 0 for key 0x%x during an ACTION_down repeat event.", - eventLogicalKey)); + postSynchronize.add( + () -> { + synthesizeEvent(false, key.logicalKey, key.physicalKey, event.getEventTime()); + }); } preEventStates[keyIdx] = nowStates[keyIdx]; postEventAnyPressed = true; @@ -260,8 +270,10 @@ private boolean handleEventImpl( final Long physicalKey = getPhysicalKey(event); final Long logicalKey = getLogicalKey(event); + final ArrayList> postSynchronizeEvents = new ArrayList<>(); for (final PressingGoal goal : KeyboardMap.pressingGoals) { - synchronizePressingKey(goal, (event.getMetaState() & goal.mask) != 0, logicalKey, event); + synchronizePressingKey( + goal, (event.getMetaState() & goal.mask) != 0, logicalKey, event, postSynchronizeEvents); } for (final TogglingGoal goal : togglingGoals.values()) { @@ -329,6 +341,9 @@ private boolean handleEventImpl( output.synthesized = false; sendKeyEvent(output, onKeyEventHandledCallback); + for (final Callable postSyncEvent : postSynchronizeEvents) { + postSyncEvent(); + } return true; } diff --git a/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java b/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java index 9210b2edd698e..66df565ed8c34 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java @@ -1346,6 +1346,56 @@ public void synchronizeOtherModifiers() { calls.clear(); } + // Regression test for https://github.com/flutter/flutter/issues/108124 + @Test + public void synchronizeModifiersForConflictingMetaState() { + // Test if ShiftLeft can be correctly synchronized during down events of + // ShiftLeft that has 0 for its metaState. + final KeyboardTester tester = new KeyboardTester(); + final ArrayList calls = new ArrayList<>(); + // Even though the event is for ShiftRight, we still set SHIFT | SHIFT_LEFT here. + // See the comment in synchronizePressingKey for the reason. + final int SHIFT_LEFT_ON = META_SHIFT_LEFT_ON | META_SHIFT_ON; + + tester.recordEmbedderCallsTo(calls); + tester.respondToTextInputWith(true); // Suppress redispatching + + // Test: Down event when the current state is 0. + assertEquals( + true, + tester.keyboardManager.handleEvent( + new FakeKeyEvent(ACTION_DOWN, SCAN_SHIFT_LEFT, KEYCODE_SHIFT_LEFT, 0, '\0', 0))); + assertEquals(calls.size(), 2); + assertEmbedderEventEquals( + calls.get(0).keyData, Type.kDown, PHYSICAL_SHIFT_LEFT, LOGICAL_SHIFT_LEFT, null, false); + assertEmbedderEventEquals( + calls.get(1).keyData, Type.kUp, PHYSICAL_SHIFT_LEFT, LOGICAL_SHIFT_LEFT, null, true); + calls.clear(); + + // A normal down event. + assertEquals( + true, + tester.keyboardManager.handleEvent( + new FakeKeyEvent( + ACTION_DOWN, SCAN_SHIFT_LEFT, KEYCODE_SHIFT_LEFT, 0, '\0', SHIFT_LEFT_ON))); + assertEquals(calls.size(), 1); + assertEmbedderEventEquals( + calls.get(0).keyData, Type.kDown, PHYSICAL_SHIFT_LEFT, LOGICAL_SHIFT_LEFT, null, false); + calls.clear(); + + // Test: Repeat event when the current state is 0. + assertEquals( + true, + tester.keyboardManager.handleEvent( + new FakeKeyEvent(ACTION_DOWN, SCAN_SHIFT_LEFT, KEYCODE_SHIFT_LEFT, 1, '\0', 0))); + assertEquals(calls.size(), 2); + assertEmbedderEventEquals( + calls.get(0).keyData, Type.kRepeat, PHYSICAL_SHIFT_LEFT, LOGICAL_SHIFT_LEFT, null, false); + assertEmbedderEventEquals( + calls.get(1).keyData, Type.kUp, PHYSICAL_SHIFT_LEFT, LOGICAL_SHIFT_LEFT, null, true); + calls.clear(); + } + @Test public void normalCapsLockEvents() { final KeyboardTester tester = new KeyboardTester(); From df28366e7ce958ff05cd67e91e17f51a61a285c5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 21 Jul 2022 16:19:59 -0700 Subject: [PATCH 2/3] Format --- .../android/KeyEmbedderResponder.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java index fc2488782c6f8..1fccb08cb861a 100644 --- a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java +++ b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java @@ -12,7 +12,6 @@ import io.flutter.plugin.common.BinaryMessenger; import java.util.ArrayList; import java.util.HashMap; -import java.util.concurrent.Callable; /** * A {@link KeyboardManager.Responder} of {@link KeyboardManager} that handles events by sending @@ -135,7 +134,7 @@ void synchronizePressingKey( boolean truePressed, long eventLogicalKey, KeyEvent event, - ArrayList> postSynchronize) { + ArrayList postSynchronize) { // During an incoming event, there might be a synthesized Flutter event for each key of each // pressing goal, followed by an eventual main Flutter event. // @@ -151,6 +150,7 @@ void synchronizePressingKey( // 1. Find the current states of all keys. // 2. Derive the pre-event state of the event key (if applicable.) for (int keyIdx = 0; keyIdx < goal.keys.length; keyIdx += 1) { + final KeyboardMap.KeyPair key = goal.keys[keyIdx]; nowStates[keyIdx] = pressingRecords.containsKey(goal.keys[keyIdx].physicalKey); if (goal.keys[keyIdx].logicalKey == eventLogicalKey) { switch (getEventType(event)) { @@ -159,9 +159,9 @@ void synchronizePressingKey( postEventAnyPressed = true; if (!truePressed) { postSynchronize.add( - () -> { - synthesizeEvent(false, key.logicalKey, key.physicalKey, event.getEventTime()); - }); + () -> + synthesizeEvent( + false, key.logicalKey, key.physicalKey, event.getEventTime())); } break; case kUp: @@ -176,9 +176,9 @@ void synchronizePressingKey( // both of which have printable characters. Obviously don't synthesize up events either. if (!truePressed) { postSynchronize.add( - () -> { - synthesizeEvent(false, key.logicalKey, key.physicalKey, event.getEventTime()); - }); + () -> + synthesizeEvent( + false, key.logicalKey, key.physicalKey, event.getEventTime())); } preEventStates[keyIdx] = nowStates[keyIdx]; postEventAnyPressed = true; @@ -270,7 +270,7 @@ private boolean handleEventImpl( final Long physicalKey = getPhysicalKey(event); final Long logicalKey = getLogicalKey(event); - final ArrayList> postSynchronizeEvents = new ArrayList<>(); + final ArrayList postSynchronizeEvents = new ArrayList<>(); for (final PressingGoal goal : KeyboardMap.pressingGoals) { synchronizePressingKey( goal, (event.getMetaState() & goal.mask) != 0, logicalKey, event, postSynchronizeEvents); @@ -341,8 +341,8 @@ private boolean handleEventImpl( output.synthesized = false; sendKeyEvent(output, onKeyEventHandledCallback); - for (final Callable postSyncEvent : postSynchronizeEvents) { - postSyncEvent(); + for (final Runnable postSyncEvent : postSynchronizeEvents) { + postSyncEvent.run(); } return true; } From 255e9b023dddf45f955804d4bbc840e16d3c5a3e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 21 Jul 2022 17:09:42 -0700 Subject: [PATCH 3/3] Fix comment --- .../io/flutter/embedding/android/KeyEmbedderResponder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java index 1fccb08cb861a..7d97f5cb46b9c 100644 --- a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java +++ b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java @@ -123,7 +123,7 @@ void updatePressingState(@NonNull Long physicalKey, @Nullable Long logicalKey) { // // Events that should be synthesized before the main event are synthesized // immediately, while events that should be syntehsized after the main event are appended to - // postSynthsize. + // `postSynchronize`. // // Although Android KeyEvent defined bitmasks for sided modifiers (SHIFT_LEFT_ON and // SHIFT_RIGHT_ON),