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

Request a reattach when creating the text input plugin on Android #13474

Merged
merged 3 commits into from
Nov 1, 2019
Merged

Request a reattach when creating the text input plugin on Android #13474

merged 3 commits into from
Nov 1, 2019

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Nov 1, 2019

Engine side of a fix for flutter/flutter#35054

If the view heirarchy was torn down, for example because the app was backgrounded and detached from the FlutterEngine instance, the embedder loses all knowledge of text input clients from the framework. However, the framework can maintain these client(s) if the engine is preserved. When the view is reattached, it needs to be re-informed of such clients. Another PR will be opened in the framework to respond to this message.

We can't use AppLifecycleState.resumed, because it comes too early (Dart code receives it before the FlutterView has re-attached to the engine, and has no way of knowing when it's safe to re-send the message). @gaaclarke - I thought maybe the Channel Buffers stuff would handle that but apparently it's not. We can discuss more if that's unexpected.

/cc @dannyvalentesonos

dnfield added a commit to dnfield/flutter that referenced this pull request Nov 1, 2019
@dnfield
Companion to flutter/engine#13474
193d3e1
@dnfield dnfield mentioned this pull request Nov 1, 2019
Merged
9 tasks
@dnfield dnfield added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Nov 1, 2019
mklim
mklim approved these changes Nov 1, 2019
View reviewed changes
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

matthew-carroll
matthew-carroll approved these changes Nov 1, 2019
View reviewed changes
Copy link
Contributor

@matthew-carroll matthew-carroll left a comment

Choose a reason for hiding this comment

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

LGTM. Left a comment about a name change.

shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java Outdated
* to a {@link FlutterEngine}, as the engine may have kept alive a text
* editing client on the Dart side.
*/
public void reattach() {
Copy link
Contributor

@matthew-carroll matthew-carroll Nov 1, 2019

Choose a reason for hiding this comment

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

Would it make sense to call this something like requestExistingInputState() instead, so that we remove the implicit temporal relationship and clarify why we call it?

Copy link
Contributor Author

@dnfield dnfield Nov 1, 2019

Choose a reason for hiding this comment

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

SGTM.

@gaaclarke
Copy link
Member

gaaclarke commented Nov 1, 2019

@gaaclarke - I thought maybe the Channel Buffers stuff would handle that but apparently it's not. We can discuss more if that's unexpected.

There may be an overflow happening, another lifecycle event before receiving the resumed event? It should print to the terminal when messages are dropped. The queues are all size 1 right now.

@dnfield dnfield merged commit ced6c63 into flutter:master Nov 1, 2019
@dnfield dnfield deleted the android_text_reattach branch November 1, 2019 22:30
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 5, 2019
@engine-flutter-autoroll
ced6c63 Request a reattach when creating the text input plugin on And…
b9bd8f7 
…roid (flutter/engine#13474)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 5, 2019
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 5, 2019
@engine-flutter-autoroll
ced6c63 Request a reattach when creating the text input plugin on And…
5df2f56 
…roid (flutter/engine#13474)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 5, 2019
Merged
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Nov 5, 2019
@engine-flutter-autoroll
Roll engine 0a8bd9d..e73c9c8 (13 commits) (#44156)
220c6fb 
git@github.com:flutter/engine.git/compare/0a8bd9dd6fbb...e73c9c8

git log 0a8bd9d..e73c9c8 --no-merges --oneline
2019-11-04 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8e083eee8ece..4cc2dc64ff13 (1 commits) (flutter/engine#13543)
2019-11-04 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ggsQ5... to XcaoM... (flutter/engine#13535)
2019-11-04 yjbanov@google.com fix getBoxesForRange for zero-length ranges (flutter/engine#13483)
2019-11-04 bkonyi@google.com Roll src/third_party/dart bbe2ac28c9..ab5cf0f854 (73 commits) (flutter/engine#13614)
2019-11-02 skia-flutter-autoroll@skia.org Roll src/third_party/skia 283ec65f632a..8e083eee8ece (32 commits) (flutter/engine#13489)
2019-11-02 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from FKqQ_... to ggsQ5... (flutter/engine#13490)
2019-11-02 xster@google.com Release shim bindings when detaching (flutter/engine#13432)
2019-11-01 gw280@google.com Add fuchsia.intl.PropertyProvider to our services on Fuchsia (flutter/engine#13486)
2019-11-01 fmil@google.com [dart] Makes the intl services available (flutter/engine#13460)
2019-11-01 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from 866GG... to dhwMR... (flutter/engine#13475)
2019-11-01 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from JngMB... to FKqQ_... (flutter/engine#13477)
2019-11-01 skia-flutter-autoroll@skia.org Roll src/third_party/skia 809ec77893be..283ec65f632a (14 commits) (flutter/engine#13472)
2019-11-01 dnfield@google.com Request a reattach when creating the text input plugin on Android (flutter/engine#13474)


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 cbracken@google.com on the revert to ensure that a human
is aware of the problem.

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/+/master/autoroll/README.md
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@engine-flutter-autoroll @Inconnu08
Roll engine 0a8bd9d..e73c9c8 (13 commits) (flutter#44156)
5dab5c8 
git@github.com:flutter/engine.git/compare/0a8bd9dd6fbb...e73c9c8

git log 0a8bd9d..e73c9c8 --no-merges --oneline
2019-11-04 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8e083eee8ece..4cc2dc64ff13 (1 commits) (flutter/engine#13543)
2019-11-04 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ggsQ5... to XcaoM... (flutter/engine#13535)
2019-11-04 yjbanov@google.com fix getBoxesForRange for zero-length ranges (flutter/engine#13483)
2019-11-04 bkonyi@google.com Roll src/third_party/dart bbe2ac28c9..ab5cf0f854 (73 commits) (flutter/engine#13614)
2019-11-02 skia-flutter-autoroll@skia.org Roll src/third_party/skia 283ec65f632a..8e083eee8ece (32 commits) (flutter/engine#13489)
2019-11-02 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from FKqQ_... to ggsQ5... (flutter/engine#13490)
2019-11-02 xster@google.com Release shim bindings when detaching (flutter/engine#13432)
2019-11-01 gw280@google.com Add fuchsia.intl.PropertyProvider to our services on Fuchsia (flutter/engine#13486)
2019-11-01 fmil@google.com [dart] Makes the intl services available (flutter/engine#13460)
2019-11-01 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from 866GG... to dhwMR... (flutter/engine#13475)
2019-11-01 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from JngMB... to FKqQ_... (flutter/engine#13477)
2019-11-01 skia-flutter-autoroll@skia.org Roll src/third_party/skia 809ec77893be..283ec65f632a (14 commits) (flutter/engine#13472)
2019-11-01 dnfield@google.com Request a reattach when creating the text input plugin on Android (flutter/engine#13474)


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 cbracken@google.com on the revert to ensure that a human
is aware of the problem.

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/+/master/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@xster xster xster approved these changes

@mklim mklim mklim approved these changes

@matthew-carroll matthew-carroll matthew-carroll approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dnfield @gaaclarke @xster @mklim @matthew-carroll @googlebot