From 24cda260584ab48f6a125a9a70c92d935057b222 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 27 Jul 2026 15:21:17 -0400 Subject: [PATCH] Fix preventBackNavigation crash by clearing the pending activity result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Activity.finishAffinity() throws IllegalStateException("Can not be called to deliver a result") when the activity's result code isn't RESULT_CANCELED or it has result data. openPlayStoreLink() is called from fragments hosted by a wide range of activities, several of which set a result, so the host frequently has one pending. The earlier guard added in #22838 checked isFinishing/isDestroyed, which is orthogonal to that precondition, so it never suppressed the crash. Clear the result before finishing instead — we're tearing down the whole task to hand the user over to Jetpack, so the activity that would have received it is being finished too. Falls back to finish() for the remaining embedded-activity path. Sentry: WORDPRESS-ANDROID-3F23, WORDPRESS-ANDROID-3F1V, WORDPRESS-ANDROID-3F3X --- .../android/ui/ActivityLauncherWrapper.kt | 16 +++++++++- .../android/ui/ActivityLauncherWrapperTest.kt | 31 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncherWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncherWrapper.kt index 3f5e063501f1..136ad795e475 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncherWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncherWrapper.kt @@ -11,6 +11,7 @@ import org.wordpress.android.R import org.wordpress.android.fluxc.model.PostImmutableModel import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.posts.RemotePreviewLogicHelper.RemotePreviewType +import org.wordpress.android.util.AppLog import org.wordpress.android.util.UrlUtils import javax.inject.Inject @@ -75,10 +76,23 @@ class ActivityLauncherWrapper @Inject constructor() { * user can't press Back to return to it — completing the "switch over" flow in one direction. Only invoked * when the target app was already installed; if it wasn't, we leave the source app intact so the user can * return after visiting the Play Store. + * + * [Activity.finishAffinity] throws IllegalStateException when the Activity still has a result to deliver + * (its result code isn't RESULT_CANCELED, or it has result data). This is reachable here because + * openPlayStoreLink() is called from fragments hosted by a wide range of activities, several of which + * set a result — so we can't assume the host has none. */ private fun preventBackNavigation(activity: Activity, shouldPrevent: Boolean) { - if (shouldPrevent && !activity.isFinishing && !activity.isDestroyed) { + if (!shouldPrevent || activity.isFinishing || activity.isDestroyed) return + // We're finishing the whole task to hand the user over to the target app, so the Activity that would + // have received this result is being torn down too — clearing it is safe here. + activity.setResult(Activity.RESULT_CANCELED, null) + try { activity.finishAffinity() + } catch (e: IllegalStateException) { + // The only remaining throw path is an embedded (ActivityGroup) child activity. + AppLog.e(AppLog.T.UTILS, "finishAffinity failed, finishing the activity only", e) + activity.finish() } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/ActivityLauncherWrapperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/ActivityLauncherWrapperTest.kt index 2ad196bcc19b..7f23fbb0cb4d 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/ActivityLauncherWrapperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/ActivityLauncherWrapperTest.kt @@ -11,6 +11,9 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -36,6 +39,7 @@ class ActivityLauncherWrapperTest { classToTest.openPlayStoreLink(activity, "packageName") verify(activity, never()).finishAffinity() + verify(activity, never()).setResult(any(), anyOrNull()) } @Test @@ -47,6 +51,31 @@ class ActivityLauncherWrapperTest { verify(activity).finishAffinity() } + @Test + fun `openPlayStoreLink, when target app is preinstalled, should clear the pending result before finishing`() { + val activity = mockAppPreinstalled(true) + + classToTest.openPlayStoreLink(activity, "packageName") + + // finishAffinity() throws if the Activity still has a result to deliver, so the result + // has to be cleared first. + inOrder(activity) { + verify(activity).setResult(Activity.RESULT_CANCELED, null) + verify(activity).finishAffinity() + } + } + + @Test + fun `openPlayStoreLink, when finishAffinity throws, should finish the activity instead of crashing`() { + val activity = mockAppPreinstalled(true) + doThrow(IllegalStateException("Can not be called to deliver a result")) + .whenever(activity).finishAffinity() + + classToTest.openPlayStoreLink(activity, "packageName") + + verify(activity).finish() + } + @Test fun `openPlayStoreLink, when activity is finishing, should not call finishAffinity`() { val activity = mockAppPreinstalled(true) @@ -55,6 +84,7 @@ class ActivityLauncherWrapperTest { classToTest.openPlayStoreLink(activity, "packageName") verify(activity, never()).finishAffinity() + verify(activity, never()).setResult(any(), anyOrNull()) } @Test @@ -65,6 +95,7 @@ class ActivityLauncherWrapperTest { classToTest.openPlayStoreLink(activity, "packageName") verify(activity, never()).finishAffinity() + verify(activity, never()).setResult(any(), anyOrNull()) } @Test