greysonp
Is a WeakReference really necessary, and does it handle the Activity going in to the background?
As the title asks, is a WeakReference really necessary? The reason I ask, is that there is nothing in this API that seems to handle the case where the Activity goes in to the background. As in, I would expect this:
@Override
protected void onPause()
{
super.onPause();
Permiso.getInstance().setActivity(null);
}
But setActivity is @NonNull, so this won't compile. Will WeakReference handle the case where the Activity goes in to the background? If not, then why not add a "Permiso.onPause" method and require the user to call it? (Internally the code would "Activity activity = mActivity;" and test activity for null and respond appropriately)
The main reason for the weak reference is that mActivity is held statically in memory (it's a member of a singleton).
Imagine you switch activities from a PermisoActivity to a regular activity. Permiso will still be holding on to the old Activity reference, causing a leak. The WeakReference lets this be garbage collected (see #6 for some history).
Seeing as all of this is held in static memory, the activity reference and such will persist until the application is destroyed. Meaning when the app is backgrounded, it'll still be there until Android sees fit to destroy it. The important thing is that because it is held by a weak reference, Permiso will not keep the activity in memory. As soon as nothing else is using it, it'll go away.
Understood, but if you add a requirement that the developer call "setActivity(null)" when the app backgrounds (onPause), then there is no leak. There is also no awkwardness of Permiso having even a WeakReference to an Activity that is no longer visible.
One different use case I have w/ my internal Permissions handling library is that my [main] app runs as a background service scanning for BLE devices. Scanning for BLE requires the "Dangerous" LOCATION permission.
At any point in time while my app is running, a user could turn off any of the required permissions. (FWIW, I hate this new Marshmallow permissions model). "Any point in time" especially includes when the app is running in the background (which it is by the very nature of them foregrounding the Settings->App screen and disabling the permission).
When any of my app's Activities come to the foreground they register w/ my permission handler. When the go in to the background they unregister w/ my permission handler.
Thus, whenever the app scans for BLE it has to test for permissions. If the permissions are granted then we're golden. If the permissions aren't granted and there is a registered activity then it displays any rationale and permissions request. If the permissions aren't granted and there is no registered activity then I display a notification that required permissions are denied; the notification action will foreground the app, which will then display any rationale and permissions request.
I could probably try to do some of this logic in IOnPermissionResult.onRationaleRequested, but some of it the logic would be redundant to Permiso tracking the current Activity. It would be nicer if Permiso could have a mode that shows a Notification if there is no set Activity, and a DialogFragment if there is a set Activity.
Hmm, checking for permissions in the background is an interesting case. Your activity can bind/unbind to the service in onResume/onPause (if you aren't doing that already), and so the service should know if an activity is in the foreground. If one is, then you can make the Permiso request (as long as the service is in the same process as your app, and therefore shares the same static memory).
I see what your saying about having Permiso make this easier. You don't necessarily want to have to keep track of whether or not activities are present. I like your notification idea, maybe that could be some sort of setting, like Permiso#shouldNotifyWhenBackgrounded(boolean) or something, and then a notification can be posted if the activity is null or backgrounded.
We just have to be careful though, because now all of the sudden Permiso has to keep track of a bundle of pending requests that have to be asked at the next available moment. That information would also have to be persisted in case the application was destroyed after the notification was posted. It's a fair bit of work for sure. I'd probably want to add unit tests before I dug into this to make sure I don't break anything 😉
Let me know if you have a more detailed proposal, thanks!
(Sorry for the delayed reply by the way. Life has been catching up with me 😋 )