firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Win32, Keyboard] Fix toggled state synthesization misses up events (flutter/engine#29282)

Browse Source 
This PR fixes a problem where up events might be missed during synthesizing for toggled states.
This commit is contained in:
Tong Mu
2021-10-27 23:14:46 -07:00
committed by GitHub
parent 108bf7c5cc
commit f0825fd030
5 changed files with 137 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc
View File

@@ -204,7 +204,7 @@ void KeyboardKeyChannelHandler::KeyboardHook(
size_t reply_size) {
auto decoded = flutter::JsonMessageCodec::GetInstance().DecodeMessage(
reply, reply_size);
bool handled = (*decoded)[kHandledKey].GetBool();
bool handled = decoded ? (*decoded)[kHandledKey].GetBool() : false;
callback(handled);
});
}

35
engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler_unittests.cc
View File

@@ -110,19 +110,16 @@ TEST(KeyboardKeyChannelHandlerTest, ExtendedKeysAreSentToRedispatch) {
}
TEST(KeyboardKeyChannelHandlerTest, DeadKeysDoNotCrash) {
auto handled_message = CreateResponse(true);
auto unhandled_message = CreateResponse(false);
int received_scancode = 0;
bool received = false;
TestBinaryMessenger messenger(
[&received_scancode, &handled_message, &unhandled_message](
const std::string& channel, const uint8_t* message,
size_t message_size, BinaryReply reply) {
[&received](const std::string& channel, const uint8_t* message,
size_t message_size, BinaryReply reply) {
if (channel == "flutter/keyevent") {
auto message_doc = JsonMessageCodec::GetInstance().DecodeMessage(
message, message_size);
uint32_t character = (*message_doc)[kCharacterCodePointKey].GetUint();
EXPECT_EQ(character, (uint32_t)'^');
received = true;
}
return true;
});
@@ -133,6 +130,30 @@ TEST(KeyboardKeyChannelHandlerTest, DeadKeysDoNotCrash) {
[](bool handled) {});
// EXPECT is done during the callback above.
EXPECT_TRUE(received);
}
TEST(KeyboardKeyChannelHandlerTest, EmptyResponsesDoNotCrash) {
bool received = false;
TestBinaryMessenger messenger(
[&received](const std::string& channel, const uint8_t* message,
size_t message_size, BinaryReply reply) {
if (channel == "flutter/keyevent") {
std::string empty_message = "";
std::vector<uint8_t> empty_response(empty_message.begin(),
empty_message.end());
reply(empty_response.data(), empty_response.size());
received = true;
}
return true;
});
KeyboardKeyChannelHandler handler(&messenger);
handler.KeyboardHook(64, kUnhandledScanCode, WM_KEYDOWN, L'b', false, false,
[](bool handled) {});
// Passes if it does not crash.
EXPECT_TRUE(received);
}
} // namespace testing

64
engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc
View File

@@ -179,9 +179,9 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
FlutterKeyEventType type;
// The resulting event's `logical_key`.
uint64_t result_logical_key;
// The next value of pressingRecords_[physical_key] (or to remove it).
uint64_t next_logical_record;
bool next_has_record = true;
// What pressingRecords_[physical_key] should be after the KeyboardHookImpl
// returns (0 if the entry should be removed).
uint64_t eventual_logical_record;
char character_bytes[kCharacterCacheSize];
character = _UndeadChar(character);
@@ -193,7 +193,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
type = kFlutterKeyEventTypeRepeat;
assert(had_record);
ConvertUtf32ToUtf8_(character_bytes, character);
next_logical_record = last_logical_record;
eventual_logical_record = last_logical_record;
result_logical_key = last_logical_record;
} else {
// A non-repeated key has been pressed that has the exact physical key
@@ -208,7 +208,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
type = kFlutterKeyEventTypeDown;
assert(!had_record);
ConvertUtf32ToUtf8_(character_bytes, character);
next_logical_record = logical_key;
eventual_logical_record = logical_key;
result_logical_key = logical_key;
}
} else { // isPhysicalDown is false
@@ -224,22 +224,37 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
assert(had_record);
// Up events never have character.
character_bytes[0] = '\0';
next_has_record = false;
eventual_logical_record = 0;
result_logical_key = last_logical_record;
}
}
UpdateLastSeenCritialKey(key, physical_key, result_logical_key);
SynchronizeCritialToggledStates(type == kFlutterKeyEventTypeDown ? key : 0);
// Synchronize the toggled states of critical keys (such as whether CapsLocks
// is enabled). Toggled states can only be changed upon a down event, so if
// the recorded toggled state does not match the true state, this function
// will synthesize (an up event if the key is recorded pressed, then) a down
// event.
//
// After this function, all critical keys will have their toggled state
// updated to the true state, while the critical keys whose toggled state have
// been changed will be pressed regardless of their true pressed state.
// Updating the pressed state will be done by SynchronizeCritialPressedStates.
SynchronizeCritialToggledStates(key, type == kFlutterKeyEventTypeDown);
// Synchronize the pressed states of critical keys (such as whether CapsLocks
// is pressed).
//
// After this function, all critical keys except for the target key will have
// their toggled state and pressed state matched with their true states. The
// target key's pressed state will be updated immediately after this.
SynchronizeCritialPressedStates(key, type != kFlutterKeyEventTypeRepeat);
if (next_has_record) {
pressingRecords_[physical_key] = next_logical_record;
if (eventual_logical_record != 0) {
pressingRecords_[physical_key] = eventual_logical_record;
} else {
pressingRecords_.erase(last_logical_record_iter);
}
SynchronizeCritialPressedStates();
if (result_logical_key == VK_PROCESSKEY) {
// VK_PROCESSKEY means that the key press is used by an IME. These key
// presses are considered handled and not sent to Flutter. These events must
@@ -322,7 +337,8 @@ void KeyboardKeyEmbedderHandler::UpdateLastSeenCritialKey(
}
void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
int toggle_virtual_key) {
int this_virtual_key,
bool is_down_event) {
// TODO(dkwingsmt) consider adding support for synchronizing key state for UWP
// https://github.com/flutter/flutter/issues/70202
#ifdef WINUWP
@@ -341,7 +357,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
if (key_info.check_toggled) {
SHORT state = get_key_state_(virtual_key);
bool should_toggled = state & kStateMaskToggled;
if (virtual_key == toggle_virtual_key) {
if (virtual_key == this_virtual_key && is_down_event) {
key_info.toggled_on = !key_info.toggled_on;
}
if (key_info.toggled_on != should_toggled) {
@@ -352,10 +368,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
kFlutterKeyEventTypeUp, key_info.physical_key,
key_info.logical_key, empty_character),
nullptr, nullptr);
} else {
// This key will always be pressed in the following synthesized event.
pressingRecords_[key_info.physical_key] = key_info.logical_key;
}
// Synchronizing toggle state always ends with the key being pressed.
pressingRecords_[key_info.physical_key] = key_info.logical_key;
SendEvent(SynthesizeSimpleEvent(kFlutterKeyEventTypeDown,
key_info.physical_key,
key_info.logical_key, empty_character),
@@ -367,7 +382,9 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
#endif
}
void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates() {
void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates(
int this_virtual_key,
bool pressed_state_will_change) {
// TODO(dkwingsmt) consider adding support for synchronizing key state for UWP
// https://github.com/flutter/flutter/issues/70202
#ifdef WINUWP
@@ -386,16 +403,19 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates() {
auto recorded_pressed_iter = pressingRecords_.find(key_info.physical_key);
bool recorded_pressed = recorded_pressed_iter != pressingRecords_.end();
bool should_pressed = state & kStateMaskPressed;
if (virtual_key == this_virtual_key && pressed_state_will_change) {
should_pressed = !should_pressed;
}
if (recorded_pressed != should_pressed) {
if (should_pressed) {
pressingRecords_[key_info.physical_key] = key_info.logical_key;
} else {
if (recorded_pressed) {
pressingRecords_.erase(recorded_pressed_iter);
} else {
pressingRecords_[key_info.physical_key] = key_info.logical_key;
}
const char* empty_character = "";
SendEvent(
SynthesizeSimpleEvent(should_pressed ? kFlutterKeyEventTypeDown
: kFlutterKeyEventTypeUp,
SynthesizeSimpleEvent(recorded_pressed ? kFlutterKeyEventTypeUp
: kFlutterKeyEventTypeDown,
key_info.physical_key, key_info.logical_key,
empty_character),
nullptr, nullptr);

4
engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.h
View File

@@ -107,10 +107,10 @@ class KeyboardKeyEmbedderHandler
uint64_t logical_key);
// Check each key's state from |get_key_state_| and synthesize events
// if their toggling states have been desynchronized.
void SynchronizeCritialToggledStates(int this_virtual_key);
void SynchronizeCritialToggledStates(int virtual_key, bool is_down);
// Check each key's state from |get_key_state_| and synthesize events
// if their pressing states have been desynchronized.
void SynchronizeCritialPressedStates();
void SynchronizeCritialPressedStates(int virtual_key, bool was_down);
// Wraps perform_send_event_ with state tracking. Use this instead of
// |perform_send_event_| to send events to the framework.

71
engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler_unittests.cc
View File

@@ -323,8 +323,9 @@ TEST(KeyboardKeyEmbedderHandlerTest, ImeEventsAreIgnored) {
EXPECT_EQ(last_handled, true);
}
// Test if modifier keys that are told apart by the extended bit
// can be identified.
// Test if modifier keys that are told apart by the extended bit can be
// identified. (Their physical keys must be searched with the extended bit
// considered.)
TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) {
TestKeystate key_state;
std::vector<TestFlutterKeyEvent> results;
@@ -343,7 +344,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) {
last_handled = false;
key_state.Set(VK_LCONTROL, true);
handler->KeyboardHook(
VK_CONTROL, kScanCodeControl, WM_KEYDOWN, 0, false, false,
VK_LCONTROL, kScanCodeControl, WM_KEYDOWN, 0, false, false,
[&last_handled](bool handled) { last_handled = handled; });
EXPECT_EQ(last_handled, false);
EXPECT_EQ(results.size(), 1);
@@ -362,7 +363,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) {
last_handled = false;
key_state.Set(VK_RCONTROL, true);
handler->KeyboardHook(
VK_CONTROL, kScanCodeControl, WM_KEYDOWN, 0, true, true,
VK_RCONTROL, kScanCodeControl, WM_KEYDOWN, 0, true, true,
[&last_handled](bool handled) { last_handled = handled; });
EXPECT_EQ(last_handled, false);
EXPECT_EQ(results.size(), 1);
@@ -381,7 +382,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) {
last_handled = false;
key_state.Set(VK_LCONTROL, false);
handler->KeyboardHook(
VK_CONTROL, kScanCodeControl, WM_KEYUP, 0, false, true,
VK_LCONTROL, kScanCodeControl, WM_KEYUP, 0, false, true,
[&last_handled](bool handled) { last_handled = handled; });
EXPECT_EQ(last_handled, false);
EXPECT_EQ(results.size(), 1);
@@ -400,7 +401,7 @@ TEST(KeyboardKeyEmbedderHandlerTest, ModifierKeysByExtendedBit) {
last_handled = false;
key_state.Set(VK_RCONTROL, false);
handler->KeyboardHook(
VK_CONTROL, kScanCodeControl, WM_KEYUP, 0, true, true,
VK_RCONTROL, kScanCodeControl, WM_KEYUP, 0, true, true,
[&last_handled](bool handled) { last_handled = handled; });
EXPECT_EQ(last_handled, false);
EXPECT_EQ(results.size(), 1);
@@ -852,7 +853,8 @@ TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledState) {
event->callback(false, event->user_data);
}
TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledStateByItself) {
TEST(KeyboardKeyEmbedderHandlerTest,
SynthesizeForDesyncToggledStateByItselfsUp) {
TestKeystate key_state;
std::vector<TestFlutterKeyEvent> results;
TestFlutterKeyEvent* event;
@@ -911,6 +913,61 @@ TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeForDesyncToggledStateByItself) {
EXPECT_EQ(last_handled, true);
}
TEST(KeyboardKeyEmbedderHandlerTest,
SynthesizeForDesyncToggledStateByItselfsDown) {
TestKeystate key_state;
std::vector<TestFlutterKeyEvent> results;
TestFlutterKeyEvent* event;
bool last_handled = false;
// NumLock is started up and disabled
key_state.Set(VK_NUMLOCK, false, false);
std::unique_ptr<KeyboardKeyEmbedderHandler> handler =
std::make_unique<KeyboardKeyEmbedderHandler>(
[&results](const FlutterKeyEvent& event,
FlutterKeyEventCallback callback, void* user_data) {
results.emplace_back(event, callback, user_data);
},
key_state.Getter());
// NumLock is toggled somewhere else
// key_state.Set(VK_NUMLOCK, false, true);
// NumLock is pressed
key_state.Set(VK_NUMLOCK, true, false);
handler->KeyboardHook(
VK_NUMLOCK, kScanCodeNumLock, WM_KEYDOWN, 0, true, false,
[&last_handled](bool handled) { last_handled = handled; });
EXPECT_EQ(last_handled, false);
EXPECT_EQ(results.size(), 3);
event = &results[0];
EXPECT_EQ(event->type, kFlutterKeyEventTypeDown);
EXPECT_EQ(event->physical, kPhysicalNumLock);
EXPECT_EQ(event->logical, kLogicalNumLock);
EXPECT_STREQ(event->character, "");
EXPECT_EQ(event->synthesized, true);
EXPECT_EQ(event->callback, nullptr);
event = &results[1];
EXPECT_EQ(event->type, kFlutterKeyEventTypeUp);
EXPECT_EQ(event->physical, kPhysicalNumLock);
EXPECT_EQ(event->logical, kLogicalNumLock);
EXPECT_STREQ(event->character, "");
EXPECT_EQ(event->synthesized, true);
EXPECT_EQ(event->callback, nullptr);
event = &results[2];
EXPECT_EQ(event->type, kFlutterKeyEventTypeDown);
EXPECT_EQ(event->physical, kPhysicalNumLock);
EXPECT_EQ(event->logical, kLogicalNumLock);
EXPECT_STREQ(event->character, "");
EXPECT_EQ(event->synthesized, false);
last_handled = false;
event->callback(true, event->user_data);
EXPECT_EQ(last_handled, true);
}
TEST(KeyboardKeyEmbedderHandlerTest, SynthesizeWithInitialTogglingState) {
TestKeystate key_state;
std::vector<TestFlutterKeyEvent> results;