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

Don't use rFD in pre-Q versions#28593

Merged
blasten merged 2 commits into
flutter-team-archive:masterfrom
blasten:fix_exception_rFD
Sep 13, 2021
Merged

Don't use rFD in pre-Q versions#28593
blasten merged 2 commits into
flutter-team-archive:masterfrom
blasten:fix_exception_rFD

Conversation

@blasten

@blasten blasten commented Sep 13, 2021

Copy link
Copy Markdown

Fixes flutter/flutter#88767

We cannot safely rely on this API since there's no test in Android prior to Q.

Comment thread shell/platform/android/io/flutter/embedding/android/FlutterActivity.java Outdated
// permissions in some Android versions.
// * rFD behavior isn't tested on pre-Q versions.
// See https://github.com/flutter/flutter/issues/46172, and
// https://github.com/flutter/flutter/issues/88767.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: consider adding that there are vendor-specific bugs.

How confident are we that this API works on all vendors after Q?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I only know that a CTS test was added. Vendors should be running these tests, but we don't really know.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SGTM

@dnfield dnfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nit.

I am wondering if we know for sure that the Android team's coverage of reportFullyDrawn is good enough to trust it on all vendors after Q.

@blasten

blasten commented Sep 13, 2021

Copy link
Copy Markdown
Author

I am wondering if we know for sure that the Android team's coverage of reportFullyDrawn is good enough to trust it on all vendors after Q.

Android uses CTS for this.

@blasten blasten merged commit 3dce983 into flutter-team-archive:master Sep 13, 2021
@blasten blasten deleted the fix_exception_rFD branch September 13, 2021 22:31
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 15, 2021
christopherfujino added a commit that referenced this pull request Sep 16, 2021
* 'Update Dart SDK to 3300f32'
* Don't use rFD in pre-Q versions (#28593)
* updated 3rd party license golden
* hard-code Build.VERSION_CODE.S
* renew cirrus key (#28584)

Co-authored-by: Emmanuel Garcia <egarciad@google.com>
Co-authored-by: Christopher Fujino <christopherfujino@gmail.com>
Co-authored-by: keyonghan <54558023+keyonghan@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

java.lang.SecurityException: Permission Denial crash at launch

2 participants