Skip to content

Fix crash when requesting permissions after changing activities #4170

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 5, 2022
Merged

Fix crash when requesting permissions after changing activities #4170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions .../main/java/com/googletest/firebase/appdistribution/testapp/NotificationFeedbackTrigger.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.google.firebase.ktx.Firebase
import com.googletest.firebase.appdistribution.testapp.NotificationFeedbackTrigger.SCREENSHOT_FILE_NAME
import com.googletest.firebase.appdistribution.testapp.NotificationFeedbackTrigger.takeScreenshot
import java.io.IOException
import java.util.*

@SuppressLint("StaticFieldLeak") // Reference to Activity is set to null in onActivityDestroyed
object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
Expand All @@ -35,7 +36,9 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
private var isEnabled = false
private var hasRequestedPermission = false
private var currentActivity: Activity? = null
private var requestPermissionLauncher: ActivityResultLauncher<String>? = null
// TODO(lkellogg): this is getting too complex - simplify it by, for example, only enabling this
// trigger on a per-activity basis instead of app-wide
private var requestPermissionLaunchers: WeakHashMap<Activity, ActivityResultLauncher<String>?> = WeakHashMap()

fun initialize(application: Application) {
// Create the NotificationChannel, but only on API 26+ because
Expand Down Expand Up @@ -108,7 +111,7 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
}

private fun requestPermission(activity: Activity) {
var launcher = requestPermissionLauncher
var launcher = requestPermissionLaunchers[activity]
if (launcher == null) {
Log.i(TAG, "Not requesting permission, because of inability to register for result.")
} else {
Expand Down Expand Up @@ -155,14 +158,14 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
override fun onActivityDestroyed(activity: Activity) {
if (activity == currentActivity) {
Log.d(TAG, "clearing current activity")
requestPermissionLauncher = null
currentActivity = null
}
requestPermissionLaunchers[activity] = null
}

override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
if (activity is ActivityResultCaller && !hasRequestedPermission) {
requestPermissionLauncher =
requestPermissionLaunchers[activity] =
activity.registerForActivityResult(ActivityResultContracts.RequestPermission()) {
isGranted: Boolean ->
if (!isEnabled) {
Expand Down
56 changes: 36 additions & 20 deletions ...ava/com/googletest/firebase/appdistribution/testapp/ScreenshotDetectionFeedbackTrigger.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :
ContentObserver(handler), Application.ActivityLifecycleCallbacks {

private val seenImages = HashSet<Uri>()
private var requestPermissionLauncher: ActivityResultLauncher<String>? = null
// TODO(lkellogg): this is getting too complex - simplify it by, for example, only enabling this
// trigger on a per-activity basis instead of app-wide
private var requestPermissionLaunchers: WeakHashMap<Activity, ActivityResultLauncher<String>?> = WeakHashMap()
private var currentActivity: Activity? = null
private var currentUri: Uri? = null
private var isEnabled = false
Expand All @@ -46,7 +48,7 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :

override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
if (activity is ActivityResultCaller) {
requestPermissionLauncher =
requestPermissionLaunchers[activity] =
activity.registerForActivityResult(ActivityResultContracts.RequestPermission()) {
isGranted: Boolean ->
if (!isEnabled) {
Expand All @@ -62,10 +64,17 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :
}
}
} else {
Log.w(TAG, "Not listening for screenshots because this activity can't register for permission request results: $activity")
Log.w(
TAG,
"Not listening for screenshots because this activity can't register for permission request results: $activity"
)
}
}

override fun onActivityDestroyed(activity: Activity) {
requestPermissionLaunchers[activity] = null
}

override fun onActivityResumed(activity: Activity) {
currentActivity = activity
if (isEnabled) {
Expand All @@ -85,7 +94,10 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :
if (isPermissionGranted(activity)) {
maybeStartFeedbackForScreenshot(activity, uri)
} else if (hasRequestedPermission) {
Log.i(TAG, "We've already request permission. Not requesting again for the life of the activity.")
Log.i(
TAG,
"We've already request permission. Not requesting again for the life of the activity."
)
} else {
// Set an in memory flag so we don't ask them again right away
hasRequestedPermission = true
Expand All @@ -95,22 +107,27 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :
}

private fun requestReadPermission(activity: Activity, uri: Uri) {
if (activity.shouldShowRequestPermissionRationale(permissionToRequest)) {
Log.i(TAG, "Showing customer rationale for requesting permission.")
AlertDialog.Builder(activity)
.setMessage(
"Taking a screenshot of the app can initiate feedback to the developer. To enable this feature, allow the app access to device storage."
)
.setPositiveButton("OK") { _, _ ->
Log.i(TAG, "Launching request for permission.")
currentUri = uri
requestPermissionLauncher!!.launch(permissionToRequest)
}
.show()
var launcher = requestPermissionLaunchers[activity]
if (launcher == null) {
Log.i(TAG, "Not requesting permission, because of inability to register for result.")
} else {
Log.i(TAG, "Launching request for permission without rationale.")
currentUri = uri
requestPermissionLauncher!!.launch(permissionToRequest)
if (activity.shouldShowRequestPermissionRationale(permissionToRequest)) {
Log.i(TAG, "Showing customer rationale for requesting permission.")
AlertDialog.Builder(activity)
.setMessage(
"Taking a screenshot of the app can initiate feedback to the developer. To enable this feature, allow the app access to device storage."
)
.setPositiveButton("OK") { _, _ ->
Log.i(TAG, "Launching request for permission.")
currentUri = uri
launcher.launch(permissionToRequest)
}
.show()
} else {
Log.i(TAG, "Launching request for permission without rationale.")
currentUri = uri
launcher.launch(permissionToRequest)
}
}
}

Expand Down Expand Up @@ -161,7 +178,6 @@ private constructor(private val infoTextResourceId: Int, handler: Handler) :
}

// Other lifecycle methods
override fun onActivityDestroyed(activity: Activity) {}
override fun onActivityStarted(activity: Activity) {}
override fun onActivityStopped(activity: Activity) {}
override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {}
Expand Down