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

Release the AccessibilityBridge when destroying a legacy FlutterView #20610

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

jason-simmons
Copy link
Member

The legacy io.flutter.view.FlutterView class was releasing the
AccessibilityBridge in its override of View.onDetachedFromWindow.
However, onDetachedFromWindow is called after the activity onDestroy
lifecycle event which typically calls FlutterView.destroy. If the
AccessibilityBridge listener is invoked after FlutterView.destroy, then
its calls to Flutter engine JNI APIs will fail.

This change will release the AccessibilityBridge and remove the listener
before FlutterView.destroy detaches from the engine.

Fixes flutter/flutter#63555

The legacy io.flutter.view.FlutterView class was releasing the
AccessibilityBridge in its override of View.onDetachedFromWindow.
However, onDetachedFromWindow is called after the activity onDestroy
lifecycle event which typically calls FlutterView.destroy.  If the
AccessibilityBridge listener is invoked after FlutterView.destroy, then
its calls to Flutter engine JNI APIs will fail.

This change will release the AccessibilityBridge and remove the listener
before FlutterView.destroy detaches from the engine.

Fixes flutter/flutter#63555
@flutter-dashboard
Copy link

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.

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

@jason-simmons
Copy link
Member Author

Also see discussions of this in flutter/flutter#33511 and #17292 (comment)

@auto-assign auto-assign bot requested a review from liyuqian August 18, 2020 21:48
@xster
Copy link
Member

xster commented Aug 19, 2020

I was gonna ask you for tests but then thought it'd be ok to exempt files we're about to delete soon™️

flutter/flutter#64181

LGTM

@jason-simmons jason-simmons 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 Aug 19, 2020
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 19, 2020
@xster xster 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 Aug 19, 2020
@fluttergithubbot fluttergithubbot merged commit d41e10b into flutter:master Aug 19, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 19, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 20, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes platform-android 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
Development

Successfully merging this pull request may close these issues.

Cannot execute operation because FlutterJNI is not attached to native
4 participants