Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Jun 12, 2023

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.

@bleroux bleroux force-pushed the android_return_keyboard_pressed_state branch from 64e7681 to e6d1b27 Compare June 12, 2023 08:33
@bleroux bleroux requested review from dkwingsmt and jiahaog June 12, 2023 12:30
@chinmaygarde
Copy link
Member

cc @dkwingsmt who may have context. In triage, we were wondering how this can be implemented without being inherently racy.

@jiahaog jiahaog removed their request for review June 20, 2023 02:59
@jiahaog
Copy link
Member

jiahaog commented Jun 20, 2023

I cherrypicked this change into the internal app that reported the issue which led to the revert #42616, and can no longer reproduce it. I don't think I'm the right person to review this PR though, so removing myself.

@dkwingsmt
Copy link
Contributor

@jiahaog Thank you so much for verifying this!

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 21, 2023
@auto-submit auto-submit bot merged commit 62e6d19 into flutter:main Jun 21, 2023
@bleroux bleroux deleted the android_return_keyboard_pressed_state branch June 21, 2023 05:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 21, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Jun 21, 2023
…129234)

flutter/engine@946f523...5313ca3

2023-06-21 [email protected] Roll Fuchsia Mac SDK from fdtK_FqpscIVvbb_j... to ct6r5YjdG2xpZPhkT... (flutter/engine#43035)
2023-06-21 [email protected] [Android] Return the keyboard pressed state (flutter/engine#42758)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from fdtK_FqpscIV to ct6r5YjdG2xp

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Jul 26, 2023
## 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.
auto-submit bot pushed a commit that referenced this pull request Aug 9, 2023
## 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.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
## 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.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
## 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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A KeyUpEvent is dispatched, but the state shows that the physical key is not pressed when hitting Backspace the first time
4 participants