Avoid exception when calling Activity.reportFullyDrawn()#103
Avoid exception when calling Activity.reportFullyDrawn()#103simonschiller wants to merge 3 commits intoandroidx:androidx-master-devfrom
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) |
ianhanniballake
left a comment
There was a problem hiding this comment.
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.
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.
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.ComponentActivityandroidx.activity.ComponentActivityandroidx.appcompat.app.AppCompatActivityTesting
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