Add a channel to query the engine keyboard state #122885

bleroux · 2023-03-17T13:14:56Z

Description

This PR adds a new channel to query the engine keyboard state.
See #87391 (comment) for motivation.

Related Issue

Framework side implementation for #87391.

Once approved the framework will try to query the initial keyboard state from the engine. PRs will be needed on the engine side to answer the framework query.

Tests

Adds 1 test.

flutter-dashboard · 2023-03-17T13:14:59Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

dkwingsmt · 2023-03-21T05:45:49Z

Yes! As far as I understand, this is exactly I was describing.

Another test I'd suggest is that the engine sends several KeyData before keyboard gets the chance to initialize itself, and keyboard ignores these earlier KeyDatas, pulls the half-way state, and tracks all KeyDatas after that. Not sure if it's simple to implement though.

Thank you so much for working on this!

bleroux · 2023-03-21T10:29:13Z

@dkwingsmt
Thanks for your feedback!

Unfortunately, it seems that the current implementation results in many web tests failing on CI (I can't repro those failures locally). The issue is probably related to the asynchronous call that I added. With this call, it is possible that an app (especially tests) tries to send key events before the handlers are set up.

 void _initKeyboard() {
    _keyboard = HardwareKeyboard();
    _keyEventManager = KeyEventManager(_keyboard, RawKeyboard.instance);
    _keyboard.syncKeyboardState().then((_) {
      platformDispatcher.onKeyData = _keyEventManager.handleKeyData;
      SystemChannels.keyEvent.setMessageHandler(_keyEventManager.handleRawKeyMessage);
    });
   // No keys handlers before syncKeyboardState completed.
  }

Bringing asynchronicity in ServiceBinding.initInstances might be a dead end? I wonder if we should try something similar to readInitialLifecycleStateFromNativeWindow (having the engine directly updating a PlatformDispatcher field, in our case it would do so for every key pressed state changes while the framework has not read the field).
I am very interested to get your insight on this.

dkwingsmt

Looks good!

packages/flutter/lib/src/services/binding.dart

packages/flutter/lib/src/services/system_channels.dart

Manual roll Flutter from 66fa4c5 to 828a040 (79 revisions) Manual roll requested by [email protected] flutter/flutter@66fa4c5...828a040 2023-05-01 [email protected] Roll Flutter Engine from 666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818) 2023-05-01 [email protected] Revert "Add migrator to upgrade gradle version when conflict with And… (flutter/flutter#125813) 2023-05-01 [email protected] Roll pub packages (flutter/flutter#125801) 2023-05-01 [email protected] [tools] fix `expect` calls in `FakeCommand` (flutter/flutter#125783) 2023-05-01 [email protected] Roll Packages from 7e3f5da to de6131d (41 revisions) (flutter/flutter#125811) 2023-05-01 [email protected] Introduce `TabBar.tabAlignment` (flutter/flutter#125036) 2023-05-01 [email protected] Roll Flutter Engine from b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805) 2023-05-01 [email protected] add support to customize Slider interacivity (flutter/flutter#121483) 2023-05-01 [email protected] Roll Flutter Engine from b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800) 2023-05-01 [email protected] Roll Flutter Engine from 605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795) 2023-05-01 [email protected] Roll Flutter Engine from bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793) 2023-05-01 [email protected] Roll Flutter Engine from 2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791) 2023-05-01 [email protected] Roll Flutter Engine from 30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789) 2023-05-01 [email protected] Roll Flutter Engine from d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787) 2023-05-01 [email protected] [tools] Apply Android Studio version detection logic to explicitly configured installation directory (`flutter config --android-studio-dir`) (flutter/flutter#125596) 2023-04-30 [email protected] Roll Flutter Engine from f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776) 2023-04-30 [email protected] Roll Flutter Engine from c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773) 2023-04-30 [email protected] Roll Flutter Engine from e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762) 2023-04-30 [email protected] Roll Flutter Engine from 1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758) 2023-04-30 [email protected] Roll Flutter Engine from 7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757) 2023-04-29 [email protected] Roll Flutter Engine from 8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750) 2023-04-29 [email protected] Roll Flutter Engine from 900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748) 2023-04-29 [email protected] Roll Flutter Engine from c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747) 2023-04-29 [email protected] Roll Flutter Engine from 0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746) 2023-04-29 [email protected] Roll Flutter Engine from 68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736) 2023-04-29 [email protected] Roll Flutter Engine from 0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735) 2023-04-29 [email protected] Fix crasher in DragableScrollableSheet when controller is animating and switching widgets (flutter/flutter#125721) 2023-04-29 [email protected] Roll Flutter Engine from 8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734) 2023-04-29 [email protected] Roll Flutter Engine from 788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731) 2023-04-29 [email protected] Roll Flutter Engine from 89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729) 2023-04-29 [email protected] Roll Flutter Engine from 3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725) 2023-04-29 [email protected] Roll Flutter Engine from 1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722) 2023-04-29 [email protected] fix package template create platform folders (flutter/flutter#125292) 2023-04-28 [email protected] Sliver Cross Axis Group (flutter/flutter#123862) 2023-04-28 [email protected] Roll Flutter Engine from 2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718) 2023-04-28 [email protected] Remove bringup from new_gallery_skia_ios__transition_perf (flutter/flutter#125715) 2023-04-28 [email protected] Roll Flutter Engine from 98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714) 2023-04-28 [email protected] Opt into CMake policy CMP0135 (flutter/flutter#125502) 2023-04-28 [email protected] Add a channel to query the engine keyboard state (flutter/flutter#122885) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125698) 2023-04-28 [email protected] `Checkbox.fillColor` should be applied to checkbox's background color when it is unchecked. (flutter/flutter#125643) 2023-04-28 [email protected] Add back one Skia test on iOS (flutter/flutter#125663) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125447) 2023-04-28 [email protected] Nit: grammar in documentation (flutter/flutter#125462) ...

## Description This PR updates the Android engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue Fixes flutter/flutter#122441 Android engine implementation for flutter/flutter#87391 ## Tests Adds 2 tests.

## Description This PR updates the Linux engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue Linux engine implementation for flutter/flutter#87391 ## Tests Adds 2 tests.

## Description This PR updates the Android engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). This is a rework of #41695 which was reverted in #42346. This issue with #41695 was that the framework side did not get an answer when the channel was setup in the engine without registering a handler (on the engine side) to handle framework requests. The issue was reproducible when the engine initialization was managed by the app (see flutter/flutter#122441 (comment) for a repro). This PR fixes this issue by changing `flutter/keyboard` lifecycle: the engine now creates the channel and registers a handler just after the channel creation. In order to avoid regression, this PR also updates the channel implemenation (see `KeyboardChannel`) to return an empty `HashMap` when there is no handler registered. ## Related Issue Android engine implementation for flutter/flutter#87391 (see #42346 for Linux implementation) Fixes flutter/flutter#122441 ## Tests Adds 3 tests.

) Manual roll Flutter from 66fa4c5 to 828a040 (79 revisions) Manual roll requested by [email protected] flutter/flutter@66fa4c5...828a040 2023-05-01 [email protected] Roll Flutter Engine from 666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818) 2023-05-01 [email protected] Revert "Add migrator to upgrade gradle version when conflict with And… (flutter/flutter#125813) 2023-05-01 [email protected] Roll pub packages (flutter/flutter#125801) 2023-05-01 [email protected] [tools] fix `expect` calls in `FakeCommand` (flutter/flutter#125783) 2023-05-01 [email protected] Roll Packages from 7e3f5da to de6131d (41 revisions) (flutter/flutter#125811) 2023-05-01 [email protected] Introduce `TabBar.tabAlignment` (flutter/flutter#125036) 2023-05-01 [email protected] Roll Flutter Engine from b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805) 2023-05-01 [email protected] add support to customize Slider interacivity (flutter/flutter#121483) 2023-05-01 [email protected] Roll Flutter Engine from b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800) 2023-05-01 [email protected] Roll Flutter Engine from 605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795) 2023-05-01 [email protected] Roll Flutter Engine from bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793) 2023-05-01 [email protected] Roll Flutter Engine from 2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791) 2023-05-01 [email protected] Roll Flutter Engine from 30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789) 2023-05-01 [email protected] Roll Flutter Engine from d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787) 2023-05-01 [email protected] [tools] Apply Android Studio version detection logic to explicitly configured installation directory (`flutter config --android-studio-dir`) (flutter/flutter#125596) 2023-04-30 [email protected] Roll Flutter Engine from f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776) 2023-04-30 [email protected] Roll Flutter Engine from c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773) 2023-04-30 [email protected] Roll Flutter Engine from e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762) 2023-04-30 [email protected] Roll Flutter Engine from 1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758) 2023-04-30 [email protected] Roll Flutter Engine from 7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757) 2023-04-29 [email protected] Roll Flutter Engine from 8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750) 2023-04-29 [email protected] Roll Flutter Engine from 900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748) 2023-04-29 [email protected] Roll Flutter Engine from c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747) 2023-04-29 [email protected] Roll Flutter Engine from 0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746) 2023-04-29 [email protected] Roll Flutter Engine from 68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736) 2023-04-29 [email protected] Roll Flutter Engine from 0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735) 2023-04-29 [email protected] Fix crasher in DragableScrollableSheet when controller is animating and switching widgets (flutter/flutter#125721) 2023-04-29 [email protected] Roll Flutter Engine from 8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734) 2023-04-29 [email protected] Roll Flutter Engine from 788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731) 2023-04-29 [email protected] Roll Flutter Engine from 89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729) 2023-04-29 [email protected] Roll Flutter Engine from 3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725) 2023-04-29 [email protected] Roll Flutter Engine from 1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722) 2023-04-29 [email protected] fix package template create platform folders (flutter/flutter#125292) 2023-04-28 [email protected] Sliver Cross Axis Group (flutter/flutter#123862) 2023-04-28 [email protected] Roll Flutter Engine from 2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718) 2023-04-28 [email protected] Remove bringup from new_gallery_skia_ios__transition_perf (flutter/flutter#125715) 2023-04-28 [email protected] Roll Flutter Engine from 98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714) 2023-04-28 [email protected] Opt into CMake policy CMP0135 (flutter/flutter#125502) 2023-04-28 [email protected] Add a channel to query the engine keyboard state (flutter/flutter#122885) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125698) 2023-04-28 [email protected] `Checkbox.fillColor` should be applied to checkbox's background color when it is unchecked. (flutter/flutter#125643) 2023-04-28 [email protected] Add back one Skia test on iOS (flutter/flutter#125663) 2023-04-28 [email protected] Roll pub packages (flutter/flutter#125447) 2023-04-28 [email protected] Nit: grammar in documentation (flutter/flutter#125462) ...

## Description This PR updates the macOS engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue macOS engine implementation for flutter/flutter#87391 Similar to: - Linux: #42346 - Android: #42758 ## Tests Adds 2 tests.

## Description This PR updates the Windows engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue Windows engine implementation for flutter/flutter#87391. Similar to: - Linux: #42346 - Android: #42758 - macOS: #42878 ## Tests Adds 2 tests.

## Description This PR updates the macOS engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue macOS engine implementation for flutter/flutter#87391 Similar to: - Linux: flutter#42346 - Android: flutter#42758 ## Tests Adds 2 tests.

## Description This PR updates the Windows engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885). ## Related Issue Windows engine implementation for flutter/flutter#87391. Similar to: - Linux: flutter#42346 - Android: flutter#42758 - macOS: flutter#42878 ## Tests Adds 2 tests.

…133663) ## Description This PR adds some documentation to SystemChannels.keyboard getKeyboardState. This method was added in #122885. ## Related Issue Fixes #132938. ## Tests Documentation only.

shinyxhh · 2024-12-28T07:47:19Z

@dkwingsmt Thanks for your feedback!

Unfortunately, it seems that the current implementation results in many web tests failing on CI (I can't repro those failures locally). The issue is probably related to the asynchronous call that I added. With this call, it is possible that an app (especially tests) tries to send key events before the handlers are set up.
 void _initKeyboard() {
    _keyboard = HardwareKeyboard();
    _keyEventManager = KeyEventManager(_keyboard, RawKeyboard.instance);
    _keyboard.syncKeyboardState().then((_) {
      platformDispatcher.onKeyData = _keyEventManager.handleKeyData;
      SystemChannels.keyEvent.setMessageHandler(_keyEventManager.handleRawKeyMessage);
    });
   // No keys handlers before syncKeyboardState completed.
  }
Bringing asynchronicity in ServiceBinding.initInstances might be a dead end? I wonder if we should try something similar to readInitialLifecycleStateFromNativeWindow (having the engine directly updating a PlatformDispatcher field, in our case it would do so for every key pressed state changes while the framework has not read the field). I am very interested to get your insight on this.

This asynchronous method delays the setMessageHandler of keyeventChannel in release mode. As a result, the setMessageHandler of keyeventChannel invoked by the application side is overwritten. But not happen in debug mode. What is the possible reason for the delay of asynchronous in release mode?

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 17, 2023

bleroux marked this pull request as draft March 17, 2023 13:15

bleroux changed the title ~~Add a channel to get keyboard state~~ Add a channel to query the engine keyboard state Mar 17, 2023

bleroux force-pushed the add_get_keyboard_state_channel branch 4 times, most recently from 312e7e0 to 3995ee6 Compare March 17, 2023 21:48

HansMuller requested a review from dkwingsmt March 17, 2023 21:55

bleroux force-pushed the add_get_keyboard_state_channel branch from 3995ee6 to 5a76977 Compare March 20, 2023 06:51

bleroux force-pushed the add_get_keyboard_state_channel branch 2 times, most recently from fab2838 to e25b9e6 Compare March 21, 2023 09:45

dkwingsmt approved these changes Apr 21, 2023

View reviewed changes

packages/flutter/lib/src/services/binding.dart Outdated Show resolved Hide resolved

dkwingsmt reviewed Apr 21, 2023

View reviewed changes

packages/flutter/lib/src/services/system_channels.dart Outdated Show resolved Hide resolved

bleroux force-pushed the add_get_keyboard_state_channel branch 12 times, most recently from b6d5a5b to e5ddb3a Compare April 27, 2023 15:09

bleroux mentioned this pull request Apr 27, 2023

Instantiate HardwareKeyboard with an initial pressed state #124588

Closed

bleroux force-pushed the add_get_keyboard_state_channel branch from 9297799 to a58ef8e Compare April 27, 2023 19:35

This was referenced May 3, 2023

[Android] Return keyboard pressed state flutter/engine#41695

Merged

Pressing and releasing a key before the framework has started throws #125975

Open

bleroux mentioned this pull request May 26, 2023

[Linux] Return keyboard pressed state flutter/engine#42346

Merged

bleroux deleted the add_get_keyboard_state_channel branch June 6, 2023 12:07

jiahaog mentioned this pull request Jun 7, 2023

Revert "[Android] Return keyboard pressed state" flutter/engine#42616

Merged

This was referenced Jun 12, 2023

[Android] Return the keyboard pressed state flutter/engine#42758

Merged

[macOS] Return keyboard pressed state flutter/engine#42878

Merged

bleroux mentioned this pull request Jul 11, 2023

Hot restart breaks keyboard input #87391

Closed

This was referenced Jul 25, 2023

Relax HarwareKeyboard assert on first received event #131258

Closed

[Windows] Return keyboard pressed state flutter/engine#43998

Merged

bleroux mentioned this pull request Aug 25, 2023

[missing Docs] in-complete docs, on why and when is services:SystemChannels.keyboard::getKeyboardState needed #132938

Closed

JSUYA mentioned this pull request Aug 28, 2023

[engine] Sync Flutter 3.13.1 source code flutter-tizen/embedder#44

Merged

bleroux mentioned this pull request Aug 30, 2023

Add more documentation for SystemChannels.keyboard getKeyboardState #133663

Merged

JSUYA mentioned this pull request Sep 8, 2023

Update to SystemChannel flutter-tizen/embedder#47

Closed

4 tasks

dkwingsmt mentioned this pull request Jun 6, 2024

Fix: Pressing and releasing a key before the framework has started should not throw #149790

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a channel to query the engine keyboard state #122885

Add a channel to query the engine keyboard state #122885

Uh oh!

bleroux commented Mar 17, 2023 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Mar 17, 2023

Uh oh!

dkwingsmt commented Mar 21, 2023

Uh oh!

bleroux commented Mar 21, 2023

Uh oh!

dkwingsmt left a comment

Uh oh!

Uh oh!

Uh oh!

shinyxhh commented Dec 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add a channel to query the engine keyboard state #122885

Add a channel to query the engine keyboard state #122885

Uh oh!

Conversation

bleroux commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Tests

Uh oh!

flutter-dashboard bot commented Mar 17, 2023

Uh oh!

dkwingsmt commented Mar 21, 2023

Uh oh!

bleroux commented Mar 21, 2023

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shinyxhh commented Dec 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bleroux commented Mar 17, 2023 •

edited

Loading