Conversation
56f9932 to
d27484a
Compare
|
@kerberjg if there is not a general Google Flutter App that can be used to validate, we need a Flutter test case in tree. If there is a general Google Flutter App that can be used, it needs to be documented somewhere. |
|
@jwinarske 👍 added validation instructions for |
jwinarske
left a comment
There was a problem hiding this comment.
When events arrive via FlutterEngineSendKeyEvent, the framework's KeyEventManager enters a transit mode (keyDataThenRawKeyData) on the first handleKeyData call. In that mode the new event is buffered into _keyEventsSinceLastMessage and only dispatched to HardwareKeyboard when a follow-up flutter/keyevent message arrives — that raw message is what triggers the buffer drain. If we never send the
legacy message, the buffer never drains, so HardwareKeyboard.instance.addHandler never fires.
Always send the legacy channel message (so the buffer drains) but make sure both paths are deterministically ordered for each keystroke so KeyEventManager doesn't see drift.
This should pass all green and not generate any error output:
https://github.com/toyota-connected/ivi-homescreen/tree/v2.0/test/integration/keyboard_test
- Add FlutterEngineSendKeyEvent support for proper key handling - Pass xkb_modifiers through key event pipeline for Shift+Arrow selection - (compatibility fixes)
d27484a to
d01e7c8
Compare
aa1e4a1 to
e136230
Compare
|
@jwinarske Fixed message ordering and validated against the integration test ✅ Just pending the above question on the Linux code events EDIT: Also gotta fix formatting. It passes clang-format-18 on my end, I'll have a look at the CI config |
Critical / functional bugs
shell/platform/homescreen/text_input_plugin.cc:153-178 (commit 2c90280) TextInputModel stores text as std::u16string and selection_ holds UTF-16 code-unit indices — confirmed in third_party/flutter/shell/platform/common/text_input_model.cc:262-296 (text_range() returns text_.length() which is UTF-16, GetText() calls fml::Utf16ToUtf8). The new code does: const std::string text = active_model_->GetText(); // UTF-8 PreviousLinePosition/NextLinePosition walk a UTF-8 string and return byte offsets, then feed them back to SetSelection as if they were UTF-16 indices. For ASCII this happens to match; for any multi-byte UTF-8 text the cursor jumps to the wrong place, can land mid-surrogate-pair, or Fix: convert columns/positions in UTF-16 (operate directly on the model's internal u16string by exposing a helper, or scan \n in UTF-16 via a UTF-8→UTF-16 mapping table).
Same file, lines 128-152. SetSelection(TextRange(sel.base(), sel.extent() ± 1)) advances by exactly one UTF-16 unit. The model's own MoveCursorBack/MoveCursorForward handle surrogate pairs (count = IsTrailingSurrogate(...) ? 2 : 1); the new shift-arrow code bypasses that, so a Reliability
shell/platform/homescreen/key_mapping.cc:446-453 (commit ced007f) const uint8_t evdev8 = evdev & 0xff; // silently truncates evdev defines codes well past 255 (gamepad buttons BTN_DPAD_UP=0x220, KEY_BUTTONCONFIG=0x240, etc.). With this mask, e.g. evdev 285 = 256+29 is reported as LEFTCTRL. Should bail out for evdev >= 256 and use the Linux-plane fallback (0x00600000 | evdev).
shell/platform/homescreen/key_event_handler.cc:159-205 The async work captures raw pointers with no lifetime guard:
If a view is torn down, the engine destroyed, the channel destroyed, or the TextInputPlugin (now owned by m_state->text_input_plugin) destroyed before the callback drains, you get use-after-free. At minimum, KeyEventHandler should null out engine_/text_input_ on shutdown and
Same path: cb_data is new'd and only freed inside OnKeyEventResponse. If the engine is torn down with key events in flight, the callback never fires and each pending key event leaks. Bound is small (per-keystroke) but worth knowing during shutdown.
key_event_handler.cc:127-143. If a key-down arrives and the user loses keyboard focus before key-up (e.g. Wayland keyboard_leave or the window deactivates), the entry sticks around. The next press of the same physical key is then mis-classified as kFlutterKeyEventTypeRepeat instead of
shell/view/flutter_view.cc:178-181 (commit c7595f5):
text_input_plugin.cc:117 (shift = (modifiers & 0x1) != 0) and key_event_handler.cc (ctrl_or_alt = (data->modifiers & 0x0c) != 0). The values from xkb_state_serialize_mods(..., XKB_STATE_MODS_EFFECTIVE) are bit indices into the keymap's modifier list, which are usually Shift=0, Ctrl=2,
key_event_handler.cc:181-185:
key_event_handler.h says key events go via "embedder API + flutter/keyevent channel"; in practice (after e136230) the channel send now happens inside the asio post, sequenced after SendKeyEvent. Worth a one-liner update so future maintainers don't get confused. Performance
Every key event allocates a KeyEventCallbackData, a std::string, posts a lambda, builds a rapidjson document, and walks two unordered_maps. Negligible at human typing rates but eliminates the ability to get clean key-event timing under stress (e.g. autorepeat at 60+ Hz). A small object
key_event_handler.h:64 — KeyboardHook is non-const, so mutable is unnecessary. Cosmetic. Net assessmentItems 1, 2, 3, 4, 6 are real correctness/safety regressions worth fixing before this lands on v2.0. The rest are tightening. The HID plane fix (0x00700000 → 0x00070000) and the event-ordering fix in e136230 are good catches. |
a67b2fe to
94e9b29
Compare
- Fix UTF-16 offset bug - Fix Shift+Arrow surrogate-pair bug - Fix hard-coded shift_mask - Fix evdev >= 0xff aliasing - Add alive_ guard to prevent leaks - Update docs
67a7324 to
3dab352
Compare
|
@jwinarske Tested and ready for review! |
There was a problem hiding this comment.
Reliability
H1 — TextInputPlugin is destroyed before KeyEventHandler
shell/platform/homescreen/flutter_desktop_view_controller_state.h:34-39
std::vector<std::unique_ptr<flutter::KeyboardHookHandler>> keyboard_hook_handlers; // contains KeyEventHandler
std::unique_ptr<flutter::TextInputPlugin> text_input_plugin;C++ destroys members in reverse declaration order, so text_input_plugin
is destroyed first, then keyboard_hook_handlers. Between those two
destructor calls, KeyEventHandler is still alive and text_input_ is a
dangling pointer.
shell/platform/homescreen/key_event_handler.cc:206-208 snapshots
text_input_ into the heap-allocated KeyEventCallbackData at the moment
KeyboardHook runs. The lifetime sentinel alive_ only signals when the
handler itself is gone — it does not cover the case where
TextInputPlugin has been destroyed but KeyEventHandler has not.
Window: any key event delivered to KeyboardHook() after ~TextInputPlugin
and before ~KeyEventHandler queues an asio post that calls
cb_data->text_input->GetSelectedText() / ->KeyboardHook() on freed
memory.
Fix: declare text_input_plugin before keyboard_hook_handlers (smallest
change), or have KeyEventHandler clear text_input_ from a ~TextInputPlugin
hook.
H2 — Display::m_view_controller_state is a raw pointer that is never cleared
shell/wayland/display.h:367 — FlutterDesktopViewControllerState* m_view_controller_state{}. The new keyboard plumbing adds two more
dereferences:
display.cc:589—FocusLostCallback(d->m_view_controller_state)display.cc:612—KeymapChangedCallback(d->m_view_controller_state, …)
If the controller state is destroyed before the Display, every Wayland
keyboard callback (key, leave, keymap) UAFs. The two new callbacks check
non-null first; the existing KeyCallback at display.cc:658, 717 does
not. Make the policy uniform and clear the pointer on view teardown.
H3 — TextInputPlugin is touched from multiple threads with no synchronization
| Entry | Thread |
|---|---|
KeyboardHook (clipboard path on the asio strand from key_event_handler.cc:231) |
platform strand |
OnKeyEventResponse → text_input_->KeyboardHook (key_event_handler.cc:101) |
thread chosen by Flutter engine |
FocusLost (synchronously from display.cc:589) |
Wayland event loop |
KeymapChanged (synchronously from display.cc:612) |
Wayland event loop |
HandleMethodCall (channel handler) |
platform thread |
State touched: active_model_, unicode_state_, unicode_hex_,
compose_base_/extent_utf16_, shift_mask_, ctrl_mask_, client_id_,
input_type_, input_action_. No mutex, no atomics, no strand routing.
A focus-leave arriving while a keystroke is in flight on the strand will
mutate unicode_state_ and unicode_hex_ concurrently with the strand
lambda that reads/writes them. Realistic on Wayland because
keyboard_handle_leave is dispatched from a different thread than the
platform task runner.
Fix: route FocusLost and KeymapChanged through asio::post to the
platform strand so all TextInputPlugin state touches are serialized.
H4 — OnKeyEventResponse runs the fallback inline, off-strand
key_event_handler.cc:76-105. The Flutter embedder invokes the callback
on its own thread, then this code calls data->text_input->KeyboardHook(…)
directly. TextInputPlugin::KeyboardHook is also entered from the strand
for clipboard handling — concurrent access to active_model_ and the
Unicode state machine is possible.
The alive_ sentinel only protects against UAF of the handler; it does not
serialize access to TextInputPlugin.
Fix: post the fallback text_input_->KeyboardHook(…) back onto the same
strand instead of calling it inline.
M1 — cb_data->text_input snapshot doesn’t survive H1 window
key_event_handler.cc:247. The strand lambda checks the handler’s alive_
sentinel and then calls methods on cb_data->text_input. Between sentinel
check and dereference, the snapshot is fine; it’s the snapshot itself
that is dangling under H1’s teardown ordering. Fixing H1 closes this.
M2 — SendKeyEvent failure leaks cb_data
key_event_handler.cc:275. eng->SendKeyEvent(...) is called for its
side effect; the return code is dropped. The Flutter embedder API only
invokes the callback on kSuccess. On any other return value, cb_data
leaks and the fallback to TextInputPlugin never fires.
Fix:
auto rc = eng->SendKeyEvent(flutter_event, OnKeyEventResponse, cb_data);
if (rc != kSuccess) {
// call OnKeyEventResponse(false, cb_data) ourselves, or delete cb_data
}M3 — KeyEventHandler::~KeyEventHandler does not drain the strand
The destructor sets alive_->store(false) and returns. Lambdas already
queued on the strand check the sentinel and free cb_data on bail — that
part is correct. But the destructor relies on the engine and the strand
outliving the handler. Today they do; a future refactor that swaps that
order silently regresses this. Worth a comment in the destructor pinning
the invariant.
M4 — Backspace count drift in Ctrl+Shift+U state machine
text_input_plugin.cc:469-473 and :500-504:
const size_t remove_count = 1 + unicode_hex_.size();
for (size_t i = 0; i < remove_count; ++i) {
active_model_->Backspace();
}This assumes the model still contains exactly the inserted 'u' + hex…
characters at the cursor. If the framework calls
TextInput.setEditingState while the user is mid-Unicode-input, the
state machine isn’t reset (the only reset path is kClearClientMethod /
kSetClientMethod) and the backspace loop will delete user-typed
characters.
Fix: in kSetEditingStateMethod also reset unicode_state_,
unicode_hex_, and the compose extents.
M5 — Ctrl+V while in Unicode-input mode pastes through the model
key_event_handler.cc:259-263 runs cb_data->text_input->PasteText(...)
on Ctrl+V before the strand dispatches the key to TextInput. While in
the Unicode hex-pending state, this inserts arbitrary text mid-composition.
The next Backspace loop in Commit/Cancel will eat from that pasted text
instead of from the composing region.
Fix: short-circuit clipboard shortcuts when
text_input_->IsUnicodeInputActive() is true.
M6 — keyboard_handle_leave has a stray ; and orders trace after callback
display.cc:587-593:
if (d->m_view_controller_state) {
FocusLostCallback(d->m_view_controller_state);
}
SPDLOG_TRACE("- Display::keyboard_handle_leave()");
; // <-- stray empty statementCosmetic. Remove the stray ;. Also consider tracing first then calling
into the controller, so the leave event is observable even if the
callback throws.
L1 — engine_ and text_input_ are unsynchronized non-atomic raw pointers
key_event_handler.cc:78-79, 198, 230. SetEngine /
SetTextInputPlugin are called once during view init on the platform
thread before any Wayland keyboard input is delivered, so the race is
benign in practice. Mark them std::atomic<T*> to keep TSan quiet and to
document the threading contract.
L2 — keyboard_repeat_func reads m_xkb_state racing with keyboard_handle_keymap
display.cc:711-716. The repeat timer reads d->m_xkb_state while
keyboard_handle_keymap (display.cc:610-614) is allowed to
xkb_state_unref and replace it. Pre-existing, but the new modifier
serialize on line 712 widens the window slightly. Disarm the timer in
keyboard_handle_keymap before swapping.
L3 — FlutterView::Initialize uses dynamic_cast + assert
shell/view/flutter_view.cc:181-184. The cast assumes
keyboard_hook_handlers[0] is a KeyEventHandler. Today the constructor
guarantees this, but a future push-order change silently breaks key
dispatch in release builds (assert compiled out). Hold the
KeyEventHandler* directly when you create it.
L4 — Utf8PosToUtf16Index trusts well-formed UTF-8
text_input_plugin.cc:54-74. The loop walks bytes by lead-byte length
without validating continuation bytes. Safe today because
TextInputModel::GetText() always returns valid UTF-8 (it round-trips via
fml::Utf16ToUtf8). Add a min(i + seq_len, utf8.size()) clamp if a
future caller can pass externally sourced text.
L5 — CommitUnicodeInput silently drops malformed hex
text_input_plugin.cc:478-490. stoul throws → catch(...) → codepoint = 0 → validation fails → no insertion. The composing region vanishes
with no user feedback. Add SPDLOG_DEBUG so it’s observable.
L6 — keyboard_handle_key ignores the no-repeat else branch
display.cc:665-672. The previous else { SPDLOG_DEBUG("key does not repeat: …"); } is now commented out:
// else {
// SPDLOG_DEBUG("key does not repeat: 0x{:x}", xkb_scancode);
// }Either delete or restore. Commented-out code rots — pick one.
Summary
| Sev | Count |
|---|---|
| High | 4 |
| Medium | 6 |
| Low | 6 |
Top three to fix before merge:
- H1 — reorder members in
flutter_desktop_view_controller_state.hso
text_input_pluginis destroyed afterkeyboard_hook_handlers. - H3 / H4 — funnel
FocusLost,KeymapChanged, and the
OnKeyEventResponsefallback through the platform strand. - H2 — clear
Display::m_view_controller_stateon view shutdown and
apply the null check uniformly to all three callbacks.
Security
H1 — Untyped JSON access in TextInput.setClient arguments
shell/platform/homescreen/text_input_plugin.cc:543:
client_id_ = client_id_json.GetInt();client_id_json is checked for IsNull() only. RapidJSON’s GetInt()
asserts (or returns indeterminate value in NDEBUG builds, depending on
build flags) when the underlying type is not int — e.g., a string or a
double. A malicious Flutter bundle that calls
TextInput.setClient([{}, …]) or supplies a string in slot 0 can crash
the homescreen.
Same pattern at :595-596 for selectionBase / selectionExtent after
the IsNull check — only IsInt then GetInt would be safe.
Mitigation: add IsInt() (and IsString() where strings are read) checks
before the typed accessor and return Error("Bad Arguments", ...) on
mismatch. Same hardening should be applied to the other places that
assume types after only checking IsNull (lines 532-538, 547-548, 558).
Severity is high in any deployment where Flutter bundle integrity is not
end-to-end verified, low otherwise. ivi-homescreen does load arbitrary
asset bundles via -b, so this is reachable.
H2 — setEditingState does not validate selectionBase/selectionExtent against text length
text_input_plugin.cc:595-602:
int base = selection_base->value.GetInt();
int extent = selection_extent->value.GetInt();
if (base == -1 && extent == -1) {
base = extent = 0;
}
active_model_->SetText(text->value.GetString());
active_model_->SetSelection(
TextRange(static_cast<size_t>(base), static_cast<size_t>(extent)));A negative non--1 value (e.g., -2) becomes a huge size_t. A value
larger than the text’s UTF-16 length is also accepted. TextRange
internals will then drive arrow-key navigation with out-of-range indices.
TextInputModel::SetSelection does have a clamp internally for some
callers, but the values are also fed back through the round-trip channel
in SendStateUpdate and into our up/down arrow code at
text_input_plugin.cc:289-294, which uses
active_model_->selection().extent() as a UTF-16 index passed to
TextRange(new_u16_pos). If the engine accepts the bogus extent, our
later math is undefined.
Mitigation: clamp base/extent to [0, text_utf16_length], reject
negatives other than the documented -1 sentinel.
M1 — In-process clipboard discloses across security contexts
key_event_handler.cc:100, 234, 247-263. clipboard_ is a
std::string member of KeyEventHandler, scoped per FlutterView.
Anything copied/cut from one text field is plain UTF-8 in the process
heap, kept until overwritten by another copy or until the handler is
destroyed. Two implications:
- No clearing on focus loss / app sleep / lock. Sensitive text
(passwords pasted into a hidden field, OTP codes, etc.) lingers in
process memory indefinitely. The TODO at
key_event_handler.h:99notes "integrate with the system clipboard"
but doesn’t mention sanitization. Consider zeroing onFocusLost
for known-sensitive input types (password input type once we honour
it, or after N seconds of idle). - Cross-bundle leak. If multiple Flutter bundles share the same
FlutterViewlifetime (today they don’t, but the architecture
permits it), the clipboard is shared without an authorization check.
Mitigation: zero clipboard_ in ~KeyEventHandler (currently relies on
std::string destructor — it does not zero), and offer a "clear on
focus loss" knob for sensitive input types.
M2 — Utf32ToUtf8 is correct but lacks surrogate validation
key_event_handler.cc:52-70. The function happily encodes UTF-32
values in [0xD800, 0xDFFF] (lone surrogates) into 3-byte UTF-8
sequences. RFC 3629 forbids these. The input source is
xkb_keysym_to_utf32, which never returns a surrogate, so this is
not currently exploitable — but the helper is generic (Utf32ToUtf8
is its name, not "encode keysym output"), and the same helper could
later be reached from clipboard-paste or framework-supplied data.
Mitigation: explicitly reject [0xD800, 0xDFFF] and > 0x10FFFF,
and return an empty string. Symmetric to the validation already done
in CommitUnicodeInput at text_input_plugin.cc:485.
M3 — std::stoul parses Unicode hex without locale defence
text_input_plugin.cc:478-482:
unsigned long codepoint = 0;
try {
codepoint = std::stoul(unicode_hex_, nullptr, 16);
} catch (...) {
// Malformed hex — nothing to insert.
}unicode_hex_ is built only from IsHexDigit ASCII characters
(text_input_plugin.cc:38-41, 220), and is bounded to 6 characters
(:220). So overflow is structurally impossible (max parsed value is
0xFFFFFF, well under ULONG_MAX). The post-parse codepoint
validation at :485-486 correctly excludes surrogates and values >
U+10FFFF. No exploitable issue. Documented here so a future
relaxation (e.g., allowing 8-char hex) doesn’t silently regress the
overflow argument.
M4 — Legacy raw-keyevent channel sends keysym as int to Dart
key_event_handler.cc:281-296. keysym is a uint32_t; RapidJSON
encodes it via AddMember(kKeyCodeKey, keysym, allocator) which
selects the int overload — values > INT32_MAX get encoded with the
sign bit set. xkb_keysym_t values can exceed INT32_MAX for some
Unicode-plane keysyms (e.g., 0x01000000 + codepoint), so the JSON
recipient sees a negative number. Dart’s framework treats keysyms as
int and likely tolerates this, but it’s a footgun if anyone matches
on positive values. RawKeyboard is deprecated post-3.19 anyway; the
fix is to coerce to uint64_t/int64_t explicitly so the JSON
encoding stays positive.
L1 — KeyEventCallbackData heap allocation per keystroke
Not a security issue per se; memory-safety reviewed under reliability
(see reliability M2). Listed here only so the audit covers the new
heap object: it’s zero-initialized via aggregate init, contains no
pointers we own beyond text_input (covered by reliability H1), and
is freed exactly once on every code path (sentinel-bail or
OnKeyEventResponse). No leak primitive on its own.
L2 — Modifier mask defaults assume an X-style keymap
key_event_handler.h:88-90 and text_input_plugin.h:106-110. Defaults
are shift=0x1, ctrl=0x4, alt=0x8, applied until KeymapChanged
overwrites them. If a Wayland compositor never sends a keymap event
(spec-non-compliant), every keystroke is interpreted with the X
default. This is not a vulnerability, but Ctrl shortcut detection
(including the new clipboard handler at key_event_handler.cc:248)
silently misfires under unusual keymaps. Document that
KeymapChanged must be received before the first keystroke is
trusted, or short-circuit clipboard logic when masks are at their
default.
L3 — text_input_plugin.cc:600 SetText is unbounded
Framework can call TextInput.setEditingState with arbitrarily large
text. RapidJSON parses this into a string we copy into the model.
A multi-megabyte payload causes a sluggish but functional UI; not a
security concern unless the device is memory-constrained. Bound it
at the channel handler if memory pressure matters in this deployment.
L4 — keyboard_handle_keymap mmap’d fd path unchanged
display.cc:600-614. The fd from the compositor is mmap’d and parsed
by libxkbcommon. No new surface area; pre-existing behaviour. Just
noting it sits next to the new keymap-changed callback, and any
future change there should preserve the mmap/munmap discipline.
Verdict
No critical CVE-class issue introduced by this branch. The two highest
items (H1, H2) are channel-arg validation gaps that are reachable from
the Flutter framework side; they should be fixed before shipping if the
deployment doesn’t cryptographically validate bundles. The clipboard is
intentionally process-scoped per the TODO and doesn’t escape the
process, but lacks any zeroing discipline (M1).
Performance
M1 — Per-event heap allocations on the keystroke path
shell/platform/homescreen/key_event_handler.cc:206-208:
auto* cb_data = new KeyEventCallbackData{
text_input_, character_str, keysym, xkb_scancode, modifiers,
ctrl_mask_, alt_mask_, released, alive_};Every keystroke heap-allocates KeyEventCallbackData (≈80 bytes plus
std::string for character_str, plus std::weak_ptr control block touch).
On top of that:
character_stris itself a heap allocation when the codepoint is
non-ASCII (≥4 bytes triggers SBO miss on most libstdc++).- The lambda captured into
asio::postis large (10 captures); the asio
internals box this into a shared state — another small allocation per
post. rapidjson::Documentconstructed per legacy channel send (line 281)
carries its own allocator — at minimum one chunk allocation per event,
more if growth is needed.
At 30 evt/s steady state: ~3 small allocs × 30 = 90 mallocs/sec. Not a
crisis, but it’s entirely avoidable.
Cheap fix: pool KeyEventCallbackData (a free-list keyed by
KeyEventHandler) and reuse a rapidjson::Document with Clear() /
new allocator pool on a per-event basis.
Cheaper-still fix: pass small fixed-size POD by value in the lambda
capture, drop the heap object entirely, and only heap-allocate when the
embedder API needs the callback context (then keep the size to a single
unique_ptr<>-sized object).
M2 — Legacy flutter/keyevent JSON channel runs unconditionally
key_event_handler.cc:281-296. Every keystroke also serializes a JSON
document and sends it on the legacy raw-keyboard channel:
rapidjson::Document event(rapidjson::kObjectType);
auto& allocator = event.GetAllocator();
event.AddMember(kKeyCodeKey, keysym, allocator);
…
channelPtr->Send(event);Comment notes "deprecated from Flutter 3.19". For apps that only use
HardwareKeyboard (the default since 3.19), this work is wasted —
allocates JSON, runs through the codec, fans out to a Dart isolate that
ignores it.
Cheap fix: gate the channel send on a feature flag, default off; turn on
only for legacy bundles.
If the legacy path must stay on, a meaningful saving is to keep one
rapidjson::Document per KeyEventHandler, Clear() it before each
send, and reuse the allocator (most fields are integer keys/values so
this avoids ≥7 small allocations per event).
M3 — Utf8PosToUtf16Index runs over the entire text on every Up/Down arrow
text_input_plugin.cc:280-312. Each Up/Down arrow keystroke calls
active_model_->GetText() (UTF-16 → UTF-8 conversion of the full buffer)
twice in the worst case (once for PreviousLinePosition, then
Utf8PosToUtf16Index walks the full UTF-8 prefix from byte 0).
Complexity is O(N) per arrow tick where N is text size. With a 10K-char
buffer and held-down arrow at 30 Hz, that’s 600K bytes of UTF-8 walk
per second plus a UTF-16→UTF-8 round-trip. Visible on slow ARM IVI SoCs
under text-editor workloads.
Fix: maintain the cursor’s UTF-8 byte offset alongside the UTF-16 index
in TextInputModel, or do up/down navigation directly in UTF-16 space
(it’s where the model already lives). The current “convert to UTF-8,
seek, convert back” round-trip is purely incidental.
M4 — xkb_state_serialize_mods called on every event including repeat
shell/wayland/display.cc:639-640, 711-714. Two new
xkb_state_serialize_mods(...) calls — one per key event, one per repeat
tick.
xkb_state_serialize_mods is cheap (a few branches inside libxkbcommon),
but it is now also called from the repeat timer at the system tick rate.
Negligible in absolute terms (single-digit microseconds at 30 Hz);
mention this only because the previous code passed 0 as modifiers and
this is the new cost line.
If you want to avoid even this, cache the serialized mods on
keyboard_handle_modifiers and read the cached value on each key.
L1 — kKeysymToLogical unordered_map lookup
shell/platform/homescreen/key_mapping.cc:355-434, 469-471. The map
holds ~80 entries, hashed by xkb_keysym_t. One lookup per non-printable
key. unordered_map is pessimistic for a tiny static set — a switch
on the keysym (or a sorted-array binary search) is faster, smaller, and
avoids the cold-cache miss on the bucket array.
Not on the hottest path (printable chars hit the early return at
:464-466), so this is constant-time noise — fix only when nearby.
L2 — pressed_logical_keys_ unordered_map
key_event_handler.h:95. Holds <10 typical entries; touched on every
event for count/find/erase/insert. Same observation as L1 —
a small flat array (linear search of 10 elements) is faster on every
modern CPU. Optimisation worth doing only if profiling actually
fingerprints it.
L3 — dynamic_cast in FlutterView::Initialize
shell/view/flutter_view.cc:181-184. Once-per-view at startup. Free.
L4 — Repeat timer arms even when key does not repeat
display.cc:665-672. Pre-existing logic now lacks the explicit "key does
not repeat" debug log (commented out). The timer arming is gated by
xkb_keymap_key_repeats, so behaviour is correct; just noting that the
removed log made the path easier to profile. Not a regression.
L5 — flutter_event.timestamp divides by 1000.0 every event
key_event_handler.cc:214-215. LibFlutterEngine->GetCurrentTime() is
nanoseconds; /1000.0 converts to microseconds (the embedder API
expects microseconds). One double divide per event. Trivially cheap.
Macro picture
The hot path now does this work per keystroke:
XkbScancodeToPhysicalKey— O(1) array lookup. Cheap.KeysymToLogicalKey— printable chars: O(1). Non-printable:
unordered_maplookup. Cheap.- Heap-allocate
KeyEventCallbackData+std::string. Avoidable. asio::postto platform strand — small lambda alloc.- On the strand: build
FlutterKeyEvent, call
LibFlutterEngine->SendKeyEvent. Engine work is independent. - Build a
rapidjson::Document,Sendon the legacy channel.
Mostly wasted on modern bundles.
At 30 events/sec, the sum is comfortably below 1 ms per event on a
modern laptop, but on a Renesas R-Car / iMX8 IVI SoC (typical target),
items 3 and 6 are the visible cost line. Pooling cb_data and gating
the legacy channel are the two changes with measurable impact.
Recommended priority:
- M2 (gate legacy channel) — biggest single win.
- M3 (avoid full-text round-trip on Up/Down) — only if multi-line text
editing is a real workload here. - M1 (pool
cb_data/rapidjson::Document) — small win, large code
surface; do alongside M2.
|
The other item is the keyboard integration test should cover this API in it's entirety, I don't believe it does now: https://api.flutter.dev/flutter/services/HardwareKeyboard-class.html |
Fixes #181
Includes migration to HardwareKeyboard as per Flutter 3.19 advisory: https://docs.flutter.dev/release/breaking-changes/key-event-migration
For validation instructions against https://github.com/flutter/samples check the issue above