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