Modify assert keyboard condition on Windows to account for violation of invariant (flutter/engine#36129)

* Skip assert on critical keys

* Comment motivation

* Update shell/platform/windows/keyboard_key_embedder_handler.cc

Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

* Unit test keyboard change

* Format

* Ditto

* Update shell/platform/windows/keyboard_unittests.cc

Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

* Update shell/platform/windows/keyboard_key_embedder_handler.cc

Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>

* Discard repeat keyup messages

* Initial

* Post synchronization

* Explain assertion

* Reassess pressing records

* Formatting

* Update shell/platform/windows/keyboard_key_embedder_handler.cc

Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>

* One space

* Update shell/platform/windows/keyboard_key_embedder_handler.cc

Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>

* Comment spacing

Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>
This commit is contained in:
yaakovschectman 2022-09-20 17:00:55 -04:00 committed by GitHub
parent 24451cf084
commit 74ec47aa10
3 changed files with 41 additions and 5 deletions

View File

@ -164,8 +164,8 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
action == WM_SYSKEYDOWN || action == WM_SYSKEYUP);
auto last_logical_record_iter = pressingRecords_.find(physical_key);
const bool had_record = last_logical_record_iter != pressingRecords_.end();
const uint64_t last_logical_record =
bool had_record = last_logical_record_iter != pressingRecords_.end();
uint64_t last_logical_record =
had_record ? last_logical_record_iter->second : 0;
// The logical key for the current "tap sequence".
@ -208,6 +208,12 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
SynchronizeCritialPressedStates(key, physical_key, is_event_down,
event_key_can_be_repeat);
// Reassess the last logical record in case pressingRecords_ was modified
// by the above synchronization methods.
last_logical_record_iter = pressingRecords_.find(physical_key);
had_record = last_logical_record_iter != pressingRecords_.end();
last_logical_record = had_record ? last_logical_record_iter->second : 0;
// The resulting event's `type`.
FlutterKeyEventType type;
character = UndeadChar(character);
@ -264,8 +270,14 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
pressingRecords_[physical_key] = eventual_logical_record;
} else {
auto record_iter = pressingRecords_.find(physical_key);
assert(record_iter != pressingRecords_.end());
pressingRecords_.erase(record_iter);
// Assert this in debug mode. But in cases that it doesn't satisfy
// (such as due to a bug), make sure the `erase` is only called
// with a valid value to avoid crashing.
if (record_iter != pressingRecords_.end()) {
pressingRecords_.erase(record_iter);
} else {
assert(false);
}
}
FlutterKeyEvent key_data{
@ -299,6 +311,14 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
pending_responses_[response_id] = std::move(pending_ptr);
SendEvent(key_data, KeyboardKeyEmbedderHandler::HandleResponse,
reinterpret_cast<void*>(pending_responses_[response_id].get()));
// Post-event synchronization. It is useful in cases where the true pressing
// state does not match the event type. For example, a CapsLock down event is
// received despite that GetKeyState says that CapsLock is not pressed. In
// such case, post-event synchronization will synthesize a CapsLock up event
// after the main event.
SynchronizeCritialPressedStates(key, physical_key, is_event_down,
event_key_can_be_repeat);
}
void KeyboardKeyEmbedderHandler::KeyboardHook(

View File

@ -942,7 +942,10 @@ TEST(KeyboardKeyEmbedderHandlerTest,
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);
// 4 total events should be fired:
// Pre-synchronization toggle, pre-sync press,
// main event, and post-sync press.
EXPECT_EQ(results.size(), 4);
event = &results[0];
EXPECT_EQ(event->type, kFlutterKeyEventTypeDown);
EXPECT_EQ(event->physical, kPhysicalNumLock);

View File

@ -2374,5 +2374,18 @@ TEST(KeyboardTest, VietnameseTelexAddDiacriticWithSlowTrueResponse) {
VietnameseTelexAddDiacriticWithSlowResponse(true);
}
// Ensure that the scancode-less key events issued by Narrator
// when toggling caps lock don't violate assert statements.
TEST(KeyboardTest, DoubleCapsLock) {
KeyboardTester tester;
tester.Responding(false);
tester.InjectKeyboardChanges(std::vector<KeyboardChange>{
WmKeyDownInfo{VK_CAPITAL, 0, kNotExtended}.Build(),
WmKeyUpInfo{VK_CAPITAL, 0, kNotExtended}.Build()});
clear_key_calls();
}
} // namespace testing
} // namespace flutter