Refactor permissions requests to use ActivityResultContract (#4391)

The app currently uses a custom callback system for requesting
permissions, located in `BaseActivity`.
This code doesn't work when the activity gets re-created before the
permission is granted: the callback will be lost with the Activity and
no action will be performed after the permission is granted.

To avoid these issues while still simplifying the code, Google
recommends to use the ActivityResultContract APIs to request
permissions, which allow to register persistent callbacks. This pull
request removes the legacy API and replaces the calls with
`registerForActivityResult(ActivityResultContracts.RequestPermission())`.

- `ActivityResultContracts.RequestPermission` will check if the
permission is already granted and call the callback synchronously when
possible. So this check doesn't need to be performed anymore.
- In `SearchStatusesFragment` and `SFragment`, the download action to
launch after granting the permission requires an argument (a list of
media URLs). This argument is temporarily stored in a Fragment field and
saved and restored as part of the instance state, so the action can
properly resume after an Activity re-creation.
This commit is contained in:
Christophe Beyls 2024-05-03 21:42:35 +02:00 committed by GitHub
commit ad1afdd241
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 133 additions and 150 deletions

View file

@ -19,7 +19,6 @@ import android.app.ActivityManager;
import android.content.Context;
import android.content.Intent;
import android.content.SharedPreferences;
import android.content.pm.PackageManager;
import android.content.res.Configuration;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
@ -34,8 +33,6 @@ import androidx.annotation.Nullable;
import androidx.annotation.StringRes;
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.app.AppCompatActivity;
import androidx.core.app.ActivityCompat;
import androidx.core.content.ContextCompat;
import androidx.preference.PreferenceManager;
import com.google.android.material.color.MaterialColors;
@ -46,14 +43,11 @@ import com.keylesspalace.tusky.db.entity.AccountEntity;
import com.keylesspalace.tusky.db.AccountManager;
import com.keylesspalace.tusky.di.Injectable;
import com.keylesspalace.tusky.interfaces.AccountSelectionListener;
import com.keylesspalace.tusky.interfaces.PermissionRequester;
import com.keylesspalace.tusky.settings.AppTheme;
import com.keylesspalace.tusky.settings.PrefKeys;
import com.keylesspalace.tusky.util.ActivityExtensions;
import com.keylesspalace.tusky.util.ThemeUtils;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import javax.inject.Inject;
@ -71,9 +65,6 @@ public abstract class BaseActivity extends AppCompatActivity implements Injectab
@NonNull
public AccountManager accountManager;
private static final int REQUESTER_NONE = Integer.MAX_VALUE;
private HashMap<Integer, PermissionRequester> requesters;
@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
@ -107,8 +98,6 @@ public abstract class BaseActivity extends AppCompatActivity implements Injectab
if(requiresLogin()) {
redirectIfNotLoggedIn();
}
requesters = new HashMap<>();
}
private boolean activityTransitionWasRequested() {
@ -273,36 +262,4 @@ public abstract class BaseActivity extends AppCompatActivity implements Injectab
startActivity(intent);
finish();
}
@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
super.onRequestPermissionsResult(requestCode, permissions, grantResults);
if (requesters.containsKey(requestCode)) {
PermissionRequester requester = requesters.remove(requestCode);
requester.onRequestPermissionsResult(permissions, grantResults);
}
}
public void requestPermissions(@NonNull String[] permissions, @NonNull PermissionRequester requester) {
ArrayList<String> permissionsToRequest = new ArrayList<>();
for(String permission: permissions) {
if (ContextCompat.checkSelfPermission(this, permission) != PackageManager.PERMISSION_GRANTED) {
permissionsToRequest.add(permission);
}
}
if (permissionsToRequest.isEmpty()) {
int[] permissionsAlreadyGranted = new int[permissions.length];
requester.onRequestPermissionsResult(permissions, permissionsAlreadyGranted);
return;
}
int newKey = requester == null ? REQUESTER_NONE : requesters.size();
if (newKey != REQUESTER_NONE) {
requesters.put(newKey, requester);
}
String[] permissionsCopy = new String[permissionsToRequest.size()];
permissionsToRequest.toArray(permissionsCopy);
ActivityCompat.requestPermissions(this, permissionsCopy, newKey);
}
}

View file

@ -23,7 +23,6 @@ import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager
import android.graphics.Bitmap
import android.graphics.Color
import android.net.Uri
@ -38,6 +37,7 @@ import android.view.View
import android.view.WindowManager
import android.webkit.MimeTypeMap
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts
import androidx.core.app.ShareCompat
import androidx.core.content.FileProvider
import androidx.core.content.IntentCompat
@ -92,6 +92,19 @@ class ViewMediaActivity :
private val toolbarVisibilityListeners = mutableListOf<ToolbarVisibilityListener>()
private var imageUrl: String? = null
private val requestDownloadMediaPermissionLauncher =
registerForActivityResult(ActivityResultContracts.RequestPermission()) { isGranted ->
if (isGranted) {
downloadMedia()
} else {
showErrorDialog(
binding.toolbar,
R.string.error_media_download_permission,
R.string.action_retry
) { requestDownloadMedia() }
}
}
fun addToolbarVisibilityListener(listener: ToolbarVisibilityListener): Function0<Boolean> {
this.toolbarVisibilityListeners.add(listener)
listener(isToolbarVisible)
@ -235,22 +248,7 @@ class ViewMediaActivity :
private fun requestDownloadMedia() {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
requestPermissions(
arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE)
) { _, grantResults ->
if (
grantResults.isNotEmpty() &&
grantResults[0] == PackageManager.PERMISSION_GRANTED
) {
downloadMedia()
} else {
showErrorDialog(
binding.toolbar,
R.string.error_media_download_permission,
R.string.action_retry
) { requestDownloadMedia() }
}
}
requestDownloadMediaPermissionLauncher.launch(Manifest.permission.WRITE_EXTERNAL_STORAGE)
} else {
downloadMedia()
}

View file

@ -21,7 +21,6 @@ import android.content.ClipData
import android.content.Context
import android.content.Intent
import android.content.SharedPreferences
import android.content.pm.PackageManager
import android.graphics.Bitmap
import android.graphics.PorterDuff
import android.graphics.PorterDuffColorFilter
@ -50,8 +49,6 @@ import androidx.annotation.ColorInt
import androidx.annotation.StringRes
import androidx.annotation.VisibleForTesting
import androidx.appcompat.app.AlertDialog
import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat
import androidx.core.content.FileProvider
import androidx.core.content.IntentCompat
import androidx.core.content.res.use
@ -164,13 +161,30 @@ class ComposeActivity :
private var maxUploadMediaNumber = InstanceInfoRepository.DEFAULT_MAX_MEDIA_ATTACHMENTS
private val takePicture =
private val takePictureLauncher =
registerForActivityResult(ActivityResultContracts.TakePicture()) { success ->
if (success) {
pickMedia(photoUploadUri!!)
}
}
private val pickMediaFile = registerForActivityResult(PickMediaFiles()) { uris ->
private val pickMediaFilePermissionLauncher =
registerForActivityResult(ActivityResultContracts.RequestPermission()) { isGranted ->
if (isGranted) {
pickMediaFileLauncher.launch(true)
} else {
Snackbar.make(
binding.activityCompose,
R.string.error_media_upload_permission,
Snackbar.LENGTH_SHORT
).apply {
setAction(R.string.action_retry) { onMediaPick() }
// necessary so snackbar is shown over everything
view.elevation = resources.getDimension(R.dimen.compose_activity_snackbar_elevation)
show()
}
}
}
private val pickMediaFileLauncher = registerForActivityResult(PickMediaFiles()) { uris ->
if (viewModel.media.value.size + uris.size > maxUploadMediaNumber) {
Toast.makeText(
this,
@ -971,14 +985,10 @@ class ComposeActivity :
// Wait until bottom sheet is not collapsed and show next screen after
if (newState == BottomSheetBehavior.STATE_COLLAPSED) {
addMediaBehavior.removeBottomSheetCallback(this)
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && ContextCompat.checkSelfPermission(this@ComposeActivity, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
ActivityCompat.requestPermissions(
this@ComposeActivity,
arrayOf(Manifest.permission.READ_EXTERNAL_STORAGE),
PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE
)
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
pickMediaFilePermissionLauncher.launch(Manifest.permission.READ_EXTERNAL_STORAGE)
} else {
pickMediaFile.launch(true)
pickMediaFileLauncher.launch(true)
}
}
}
@ -1130,31 +1140,6 @@ class ComposeActivity :
}
}
override fun onRequestPermissionsResult(
requestCode: Int,
permissions: Array<String>,
grantResults: IntArray
) {
super.onRequestPermissionsResult(requestCode, permissions, grantResults)
if (requestCode == PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE) {
if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
pickMediaFile.launch(true)
} else {
Snackbar.make(
binding.activityCompose,
R.string.error_media_upload_permission,
Snackbar.LENGTH_SHORT
).apply {
setAction(R.string.action_retry) { onMediaPick() }
// necessary so snackbar is shown over everything
view.elevation = resources.getDimension(R.dimen.compose_activity_snackbar_elevation)
show()
}
}
}
}
private fun initiateCameraApp() {
addMediaBehavior.state = BottomSheetBehavior.STATE_COLLAPSED
@ -1170,7 +1155,7 @@ class ComposeActivity :
this,
BuildConfig.APPLICATION_ID + ".fileprovider",
photoFile
).also { uri -> takePicture.launch(uri) }
).also { uri -> takePictureLauncher.launch(uri) }
}
private fun enableButton(button: ImageButton, clickable: Boolean, colorActive: Boolean) {
@ -1549,7 +1534,6 @@ class ComposeActivity :
companion object {
private const val TAG = "ComposeActivity" // logging tag
private const val PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE = 1
internal const val COMPOSE_OPTIONS_EXTRA = "COMPOSE_OPTIONS"
private const val PHOTO_UPLOAD_URI_KEY = "PHOTO_UPLOAD_URI"

View file

@ -21,16 +21,18 @@ import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager
import android.net.Uri
import android.os.Build
import android.os.Bundle
import android.os.Environment
import android.util.Log
import android.view.View
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.widget.PopupMenu
import androidx.core.app.ActivityOptionsCompat
import androidx.core.content.getSystemService
import androidx.core.view.ViewCompat
import androidx.lifecycle.lifecycleScope
import androidx.paging.PagingData
@ -41,7 +43,6 @@ import androidx.recyclerview.widget.LinearLayoutManager
import at.connyduck.calladapter.networkresult.fold
import at.connyduck.calladapter.networkresult.onFailure
import com.google.android.material.snackbar.Snackbar
import com.keylesspalace.tusky.BaseActivity
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.ViewMediaActivity
import com.keylesspalace.tusky.components.compose.ComposeActivity
@ -79,6 +80,34 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
private val searchAdapter
get() = super.adapter as SearchStatusesAdapter
private var pendingMediaDownloads: List<String>? = null
private val downloadAllMediaPermissionLauncher =
registerForActivityResult(ActivityResultContracts.RequestPermission()) { isGranted ->
if (isGranted) {
pendingMediaDownloads?.let { downloadAllMedia(it) }
} else {
Toast.makeText(
context,
R.string.error_media_download_permission,
Toast.LENGTH_SHORT
).show()
}
pendingMediaDownloads = null
}
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
pendingMediaDownloads = savedInstanceState?.getStringArrayList(PENDING_MEDIA_DOWNLOADS_STATE_KEY)
}
override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
pendingMediaDownloads?.let {
outState.putStringArrayList(PENDING_MEDIA_DOWNLOADS_STATE_KEY, ArrayList(it))
}
}
override fun createAdapter(): PagingDataAdapter<StatusViewData.Concrete, *> {
val preferences = PreferenceManager.getDefaultSharedPreferences(
binding.searchRecyclerView.context
@ -236,10 +265,6 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
}
}
companion object {
fun newInstance() = SearchStatusesFragment()
}
private fun reply(status: StatusViewData.Concrete) {
val actionableStatus = status.actionable
val mentionedUsernames = actionableStatus.mentions.map { it.username }
@ -499,37 +524,31 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
)
}
private fun downloadAllMedia(status: Status) {
private fun downloadAllMedia(mediaUrls: List<String>) {
Toast.makeText(context, R.string.downloading_media, Toast.LENGTH_SHORT).show()
for ((_, url) in status.attachments) {
val uri = Uri.parse(url)
val filename = uri.lastPathSegment
val downloadManager: DownloadManager = requireContext().getSystemService()!!
val downloadManager = requireActivity().getSystemService(
Context.DOWNLOAD_SERVICE
) as DownloadManager
for (url in mediaUrls) {
val uri = Uri.parse(url)
val request = DownloadManager.Request(uri)
request.setDestinationInExternalPublicDir(Environment.DIRECTORY_DOWNLOADS, filename)
request.setDestinationInExternalPublicDir(
Environment.DIRECTORY_DOWNLOADS,
uri.lastPathSegment
)
downloadManager.enqueue(request)
}
}
private fun requestDownloadAllMedia(status: Status) {
if (status.attachments.isEmpty()) {
return
}
val mediaUrls = status.attachments.map { it.url }
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
val permissions = arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE)
(activity as BaseActivity).requestPermissions(permissions) { _, grantResults ->
if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
downloadAllMedia(status)
} else {
Toast.makeText(
context,
R.string.error_media_download_permission,
Toast.LENGTH_SHORT
).show()
}
}
pendingMediaDownloads = mediaUrls
downloadAllMediaPermissionLauncher.launch(Manifest.permission.WRITE_EXTERNAL_STORAGE)
} else {
downloadAllMedia(status)
downloadAllMedia(mediaUrls)
}
}
@ -628,4 +647,10 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
)
}
}
companion object {
private const val PENDING_MEDIA_DOWNLOADS_STATE_KEY = "pending_media_downloads"
fun newInstance() = SearchStatusesFragment()
}
}

View file

@ -21,17 +21,19 @@ import android.content.ClipboardManager
import android.content.Context
import android.content.DialogInterface
import android.content.Intent
import android.content.pm.PackageManager
import android.net.Uri
import android.os.Build
import android.os.Bundle
import android.os.Environment
import android.util.Log
import android.view.MenuItem
import android.view.View
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.widget.PopupMenu
import androidx.core.app.ActivityOptionsCompat
import androidx.core.content.getSystemService
import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope
import at.connyduck.calladapter.networkresult.fold
@ -93,6 +95,22 @@ abstract class SFragment : Fragment(), Injectable {
@Inject
lateinit var instanceInfoRepository: InstanceInfoRepository
private var pendingMediaDownloads: List<String>? = null
private val downloadAllMediaPermissionLauncher =
registerForActivityResult(ActivityResultContracts.RequestPermission()) { isGranted ->
if (isGranted) {
pendingMediaDownloads?.let { downloadAllMedia(it) }
} else {
Toast.makeText(
context,
R.string.error_media_download_permission,
Toast.LENGTH_SHORT
).show()
}
pendingMediaDownloads = null
}
override fun startActivity(intent: Intent) {
requireActivity().startActivityWithSlideInAnimation(intent)
}
@ -106,6 +124,18 @@ abstract class SFragment : Fragment(), Injectable {
}
}
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
pendingMediaDownloads = savedInstanceState?.getStringArrayList(PENDING_MEDIA_DOWNLOADS_STATE_KEY)
}
override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
pendingMediaDownloads?.let {
outState.putStringArrayList(PENDING_MEDIA_DOWNLOADS_STATE_KEY, ArrayList(it))
}
}
override fun onResume() {
super.onResume()
@ -522,13 +552,11 @@ abstract class SFragment : Fragment(), Injectable {
}
}
private fun downloadAllMedia(status: Status) {
private fun downloadAllMedia(mediaUrls: List<String>) {
Toast.makeText(context, R.string.downloading_media, Toast.LENGTH_SHORT).show()
val downloadManager = requireActivity().getSystemService(
Context.DOWNLOAD_SERVICE
) as DownloadManager
val downloadManager: DownloadManager = requireContext().getSystemService()!!
for ((_, url) in status.attachments) {
for (url in mediaUrls) {
val uri = Uri.parse(url)
downloadManager.enqueue(
DownloadManager.Request(uri).apply {
@ -542,26 +570,22 @@ abstract class SFragment : Fragment(), Injectable {
}
private fun requestDownloadAllMedia(status: Status) {
if (status.attachments.isEmpty()) {
return
}
val mediaUrls = status.attachments.map { it.url }
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
val permissions = arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE)
(activity as BaseActivity).requestPermissions(permissions) { _, grantResults ->
if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
downloadAllMedia(status)
} else {
Toast.makeText(
context,
R.string.error_media_download_permission,
Toast.LENGTH_SHORT
).show()
}
}
pendingMediaDownloads = mediaUrls
downloadAllMediaPermissionLauncher.launch(Manifest.permission.WRITE_EXTERNAL_STORAGE)
} else {
downloadAllMedia(status)
downloadAllMedia(mediaUrls)
}
}
companion object {
private const val TAG = "SFragment"
private const val PENDING_MEDIA_DOWNLOADS_STATE_KEY = "pending_media_downloads"
private fun accountIsInMentions(
account: AccountEntity?,
mentions: List<Status.Mention>

View file

@ -1,5 +0,0 @@
package com.keylesspalace.tusky.interfaces
fun interface PermissionRequester {
fun onRequestPermissionsResult(permissions: Array<String>, grantResults: IntArray)
}