[Windows, Keyboard] Fix crash on Up-only IME event #33746
Conversation
dkwingsmt
force-pushed
the
win-key-fix-up-only-ime-synth
branch
from
7ad9e01 to
c1fd021
Compare
| @@ -114,10 +114,13 @@ 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 virtual_key, bool is_down); | |||
| void SynchronizeCritialToggledStates(int this_virtual_key, | |||
There was a problem hiding this comment.
I don't think adding "this_" helps explain anything.
There was a problem hiding this comment.
You're right. It is the leftover of an intermediate state during development. I've changed them to be event_virtual_key and alike, which should be more helpful.
dkwingsmt
added
the
waiting for tree to go green
|
This pull request is not suitable for automatic merging in its current state.
|
fluttergithubbot
removed
the
waiting for tree to go green
…wingsmt/engine into win-key-fix-up-only-ime-synth
|
Not sure how the presubmit is running for 12 hours. I can't get to the status page either. cc @godofredoc |
@drewroengoogle @yusuf-goog can you please take a look to identify if the task never run or if it was github not updating the check? some instructions on how to triage the problem can be found here: go/flutter-luci-playbook#debug-builds-not-being-scheduled |
If I'm understanding this correctly, it does look like this run did fail: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20iOS%20Engine/32658/overview |
|
Kicking off a retry to see if that reports back. |
@dkwingsmt to validate the first build failed and after clicking the re-run button the check was not updated, am I correct? @yusuf-goog @drewroengoogle any 500s in the reset-try api? |
|
@godofredoc I found that the test failed with this error last night and clicked rerun. I don't remember if the check is updated after that (I tend to saying that it did become a green dot but don't count me on that.) |
* Impl and test * Format and some doc * Rewrite algorithm * Format * Simplify * Fix * Fix params
This PR fixes flutter/flutter#104169, where a specific combination of environments leads to the following key events:
Besides fixing the exact part, this PR also reorganized much of the synthesization algorithm, adopting the terminology and process from the latest version used in Android.
Cause
The previous algorithm always looks for the critical key by virtual key, and thus fails to consider this event as the target key, synthesizing a key up event. However, the "main event" stage uses the pressing records before the synthesization, therefore also dispatching a key up event.
What is changed
The most important parts of the change are as follows:
The synthesization algorithm now looks for the critical key by physical key if the key is pressed. In this way, the extra key up event are no longer synthesized. https://github.com/flutter/engine/pull/33746/files#diff-d948a303b1d747955e0f2b1e73bd03b8b4d103fd1dfda7df97dc620603e08c8aR431-R442
The information for the main event is computed after synthesization. In this way, even if the synthesization is incorrect, the main-event stage won't exacerbate the problem. https://github.com/flutter/engine/pull/33746/files#diff-d948a303b1d747955e0f2b1e73bd03b8b4d103fd1dfda7df97dc620603e08c8aR171-R208
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.