-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Avoid exception when calling Activity.reportFullyDrawn() #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid exception when calling Activity.reportFullyDrawn() #103
Conversation
Just a heads up that we are in the middle of transitioning to |
(But if this gets in in the next few days you won't run into any issues) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this code to androidx.activity.ComponentActivity
and write a test for this (at the very least, our internal CI runs on API 19 devices and will help serve as a regression test that this method doesn't crash).
!= PackageManager.PERMISSION_GRANTED) { | ||
return; | ||
} | ||
super.reportFullyDrawn(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this should be guarded with a Build.VERSION.SDK_INT >= 19
so that reportFullyDrawn()
can be called on all API levels.
That means it may be more appropriate to re-arrange this method to be something more like:
if (Build.VERSION.SDK_INT >= 20) {
super.reportFullyDrawn();
} else if (Build.VERSION.SDK_INT == 19 && ContextCompat...) {
// Your current code and comment re: the specific behavior of API 19
}
// On KitKat (API 19) the Activity.reportFullyDrawn() method requires the | ||
// UPDATE_DEVICE_STATS permission, otherwise it throws an exception. Instead of throwing, | ||
// we fall back to a no-op call here. | ||
if (Build.VERSION.SDK_INT == Build.VERSION_CODES.KITKAT && ContextCompat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general convention in AndroidX code is to use the numbers themselves, rather than Build.VERSION_CODES
values. Please use 19 here.
Sorry for the churn - do you mind actually basing your PR on top of |
Sure, no problem ✔️ |
jcenter 502'ing :| |
Proposed Changes
Activity.reportFullyDrawn()
I'm not quite sure if this class is the right place for this workaround, the code could also live in other places, e.g.:
androidx.core.app.ComponentActivity
androidx.activity.ComponentActivity
androidx.appcompat.app.AppCompatActivity
Testing
Test: Manually tested. If your test harness is also executed on devices with API 19, I could write an automated test for this change if you want.
Issues Fixed
Fixes: 163239764