A quick Back no longer crashes, and the crash report screen survives a crash

app/src/androidTest/java/com/ninelivesaudio/app/ui/navigation/BackGestureRaceTest.kt

@@ -0,0 +1,214 @@
+package com.ninelivesaudio.app.ui.navigation
+
+import androidx.activity.BackEventCompat
+import androidx.activity.ComponentActivity
+import androidx.activity.OnBackPressedDispatcher
+import androidx.activity.compose.LocalOnBackPressedDispatcherOwner
+import androidx.activity.compose.setContent
+import androidx.compose.material3.Text
+import androidx.lifecycle.Lifecycle
+import androidx.navigation.NavBackStackEntry
+import androidx.navigation.NavHostController
+import androidx.navigation.compose.NavHost
+import androidx.navigation.compose.composable
+import androidx.navigation.compose.rememberNavController
+import androidx.test.core.app.ActivityScenario
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.util.concurrent.atomic.AtomicReference
+
+/**
+ * Issue #104. A back gesture or Back key lands, and before the navigation
+ * host gets to act on it the app itself changes the back stack (Play
+ * finishing its load and opening the Player, a toolbar back arrow, a tab).
+ * The host then tried to animate the page that was already gone and threw
+ * "Cannot transition entry that is not in the back stack", which killed
+ * the app.
+ *
+ * These run on a real activity with the real main-thread dispatcher, because
+ * the crash lives in the gap between two main-thread messages.
+ */
+@RunWith(AndroidJUnit4::class)
+class BackGestureRaceTest {
+
+    private val failure = AtomicReference<Throwable?>(null)
+    private var previousHandler: Thread.UncaughtExceptionHandler? = null
+    private lateinit var scenario: ActivityScenario<ComponentActivity>
+    private lateinit var nav: NavHostController
+    private lateinit var dispatcher: OnBackPressedDispatcher
+
+    @Before fun setUp() {
+        previousHandler = Thread.getDefaultUncaughtExceptionHandler()
+        Thread.setDefaultUncaughtExceptionHandler { _, e -> failure.compareAndSet(null, e) }
+        scenario = ActivityScenario.launch(ComponentActivity::class.java)
+        scenario.onActivity { activity ->
+            activity.setContent {
+                nav = rememberNavController()
+                dispatcher = LocalOnBackPressedDispatcherOwner.current!!.onBackPressedDispatcher
+                // The same pairing NineLivesNavHost uses.
+                CancelPendingBackOnNavigation(nav)
+                NavHost(nav, startDestination = Routes.HOME) {
+                    composable(Routes.HOME) { Text("home") }
+                    composable(Routes.LIBRARY) { Text("library") }
+                    composable(Routes.PLAYER) { Text("player") }
+                    composable(Routes.BOOK_DETAIL) { Text("detail") }
+                }
+            }
+        }
+        settle()
+        // Library, then a book, the way a person gets to a book page.
+        onMain { nav.navigateToTab(Routes.LIBRARY) }
+        settle()
+        onMain { nav.navigate(Routes.bookDetail("book")) }
+        settle()
+        onMain { assertEquals(Routes.BOOK_DETAIL, nav.currentDestination?.route) }
+    }
+
+    @After fun tearDown() {
+        scenario.close()
+        Thread.setDefaultUncaughtExceptionHandler(previousHandler)
+    }
+
+    @Test fun backKeyThenTheAppOpensThePlayer() {
+        onMain {
+            dispatcher.onBackPressed()
+            // Play finished loading and opened the Player before the host
+            // got to the Back.
+            nav.navigateToTab(Routes.PLAYER)
+        }
+        settle()
+        assertNoCrash()
+        onMain { assertEquals(Routes.PLAYER, nav.currentDestination?.route) }
+    }
+
+    @Test fun backKeyThenTheToolbarBackArrow() {
+        onMain {
+            dispatcher.onBackPressed()
+            nav.popBackStack()
+        }
+        settle()
+        assertNoCrash()
+    }
+
+    @Test fun backSwipeStartsThenTheAppLeavesThePage() {
+        onMain {
+            dispatcher.dispatchOnBackStarted(BackEventCompat(5f, 900f, 0f, BackEventCompat.EDGE_LEFT))
+            nav.popBackStack()
+        }
+        settle()
+        onMain {
+            dispatcher.dispatchOnBackProgressed(BackEventCompat(300f, 900f, 0.5f, BackEventCompat.EDGE_LEFT))
+            dispatcher.onBackPressed()
+        }
+        settle()
+        assertNoCrash()
+        // The app's own pop already left the book page and cancelled the
+        // swipe. Lifting the finger still counts as a Back from the Library,
+        // so the user lands on Home.
+        assertSettledOn(Routes.HOME)
+    }
+
+    @Test fun backSwipeInProgressThenTheAppLeavesThePage() {
+        onMain { dispatcher.dispatchOnBackStarted(BackEventCompat(5f, 900f, 0f, BackEventCompat.EDGE_LEFT)) }
+        settle()
+        onMain { dispatcher.dispatchOnBackProgressed(BackEventCompat(200f, 900f, 0.3f, BackEventCompat.EDGE_LEFT)) }
+        settle()
+        // The host is mid-swipe on the book page when the app pops it.
+        onMain { nav.popBackStack() }
+        settle()
+        onMain {
+            dispatcher.dispatchOnBackProgressed(BackEventCompat(300f, 900f, 0.5f, BackEventCompat.EDGE_LEFT))
+            dispatcher.onBackPressed()
+        }
+        settle()
+        assertNoCrash()
+        assertSettledOn(Routes.HOME)
+    }
+
+    @Test fun aPlainBackStillGoesToTheLibrary() {
+        val detail = currentEntry()
+        onMain { dispatcher.onBackPressed() }
+        settle()
+        assertNoCrash()
+        assertSettledOn(Routes.LIBRARY)
+        assertGone(detail)
+    }
+
+    @Test fun aPlainBackSwipeStillGoesToTheLibrary() {
+        onMain { dispatcher.dispatchOnBackStarted(BackEventCompat(5f, 900f, 0f, BackEventCompat.EDGE_LEFT)) }
+        settle()
+        onMain { dispatcher.dispatchOnBackProgressed(BackEventCompat(300f, 900f, 0.5f, BackEventCompat.EDGE_LEFT)) }
+        settle()
+        val detail = currentEntry()
+        onMain { dispatcher.onBackPressed() }
+        settle()
+        assertNoCrash()
+        assertSettledOn(Routes.LIBRARY)
+        assertGone(detail)
+    }
+
+    @Test fun twoQuickBacksGoToTheLibraryThenHome() {
+        onMain { dispatcher.onBackPressed() }
+        onMain { dispatcher.onBackPressed() }
+        settle()
+        assertNoCrash()
+        onMain {
+            val route = nav.currentDestination?.route
+            check(route == Routes.LIBRARY || route == Routes.HOME) { "ended on $route" }
+        }
+    }
+
+    private fun assertNoCrash() {
+        failure.get()?.let { throw AssertionError("app crashed", it) }
+    }
+
+    private fun currentEntry(): NavBackStackEntry {
+        var entry: NavBackStackEntry? = null
+        onMain { entry = nav.currentBackStackEntry }
+        return entry!!
+    }
+
+    /**
+     * On [route], with the page animation finished: the page is RESUMED and
+     * it is the only page the host still draws.
+     */
+    private fun assertSettledOn(route: String) {
+        waitUntil("$route never settled") {
+            nav.currentDestination?.route == route &&
+                nav.currentBackStackEntry!!.lifecycle.currentState == Lifecycle.State.RESUMED &&
+                nav.visibleEntries.value.count { it.destination.navigatorName == "composable" } == 1
+        }
+        onMain { assertEquals(route, nav.currentDestination?.route) }
+    }
+
+    /** The popped page finished its exit and was torn down. */
+    private fun assertGone(entry: NavBackStackEntry) {
+        waitUntil("${entry.destination.route} was never destroyed") {
+            entry.lifecycle.currentState == Lifecycle.State.DESTROYED
+        }
+    }
+
+    private fun waitUntil(message: String, condition: () -> Boolean) {
+        val deadline = System.currentTimeMillis() + 5_000
+        while (true) {
+            var met = false
+            onMain { met = condition() }
+            if (met) return
+            if (System.currentTimeMillis() > deadline) throw AssertionError(message)
+            Thread.sleep(50)
+        }
+    }
+
+    private fun onMain(block: () -> Unit) = scenario.onActivity { block() }
+
+    private fun settle() {
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync()
+        Thread.sleep(600)
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync()
+    }
+}

app/src/main/java/com/ninelivesaudio/app/AppProcess.kt

@@ -0,0 +1,11 @@
+package com.ninelivesaudio.app
+
+/**
+ * True when [processName] is the app's main process, the one named after the
+ * package. Side processes such as the crash dialog's ":acra" get a suffix.
+ *
+ * An unknown name counts as main, because an app that starts nothing is worse
+ * than one that starts everything.
+ */
+internal fun isMainAppProcess(processName: String?, packageName: String): Boolean =
+    processName == null || processName == packageName

app/src/main/java/com/ninelivesaudio/app/NineLivesApp.kt

@@ -34,45 +34,57 @@ import org.acra.config.mailSender
 import org.acra.data.StringFormat
 import org.acra.ktx.initAcra
 import com.ninelivesaudio.app.widget.WidgetRefresher
+import dagger.Lazy
 import javax.inject.Inject
 
 @HiltAndroidApp
 class NineLivesApp : Application(), ImageLoaderFactory {
 
     @Inject
-    lateinit var connectivityMonitor: ConnectivityMonitor
+    lateinit var connectivityMonitorLazy: Lazy<ConnectivityMonitor>
+    val connectivityMonitor: ConnectivityMonitor get() = connectivityMonitorLazy.get()
 
     @Inject
-    lateinit var syncManager: SyncManager
+    lateinit var syncManagerLazy: Lazy<SyncManager>
+    val syncManager: SyncManager get() = syncManagerLazy.get()
 
     @Inject
-    lateinit var settingsManager: SettingsManager
+    lateinit var settingsManagerLazy: Lazy<SettingsManager>
+    val settingsManager: SettingsManager get() = settingsManagerLazy.get()
 
     @Inject
-    lateinit var libraryRepository: LibraryRepository
+    lateinit var libraryRepositoryLazy: Lazy<LibraryRepository>
+    val libraryRepository: LibraryRepository get() = libraryRepositoryLazy.get()
 
     @Inject
-    lateinit var apiService: ApiService
+    lateinit var apiServiceLazy: Lazy<ApiService>
+    val apiService: ApiService get() = apiServiceLazy.get()
 
     @Inject
-    lateinit var billingManager: BillingManager
+    lateinit var billingManagerLazy: Lazy<BillingManager>
+    val billingManager: BillingManager get() = billingManagerLazy.get()
 
     @Inject
-    lateinit var downloadManager: DownloadManager
+    lateinit var downloadManagerLazy: Lazy<DownloadManager>
+    val downloadManager: DownloadManager get() = downloadManagerLazy.get()
 
     @Inject
-    lateinit var playbackManager: PlaybackManager
+    lateinit var playbackManagerLazy: Lazy<PlaybackManager>
+    val playbackManager: PlaybackManager get() = playbackManagerLazy.get()
 
     @Inject
-    lateinit var entitlementRepository: EntitlementRepository
+    lateinit var entitlementRepositoryLazy: Lazy<EntitlementRepository>
+    val entitlementRepository: EntitlementRepository get() = entitlementRepositoryLazy.get()
 
     // The app's OkHttpClient carries the auth token, self-signed cert config, and
     // dynamic base URL, so cover requests authenticate like every other call.
     @Inject
-    lateinit var okHttpClient: OkHttpClient
+    lateinit var okHttpClientLazy: Lazy<OkHttpClient>
+    val okHttpClient: OkHttpClient get() = okHttpClientLazy.get()
 
     @Inject
-    lateinit var widgetRefresher: WidgetRefresher
+    lateinit var widgetRefresherLazy: Lazy<WidgetRefresher>
+    val widgetRefresher: WidgetRefresher get() = widgetRefresherLazy.get()
 
     private val appScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)
 
@@ -118,6 +130,14 @@ class NineLivesApp : Application(), ImageLoaderFactory {
     override fun onCreate() {
         super.onCreate()
 
+        // Android runs onCreate in every process the app has, including the
+        // crash dialog's own ":acra" process. Everything below belongs to the
+        // main process only. WorkManager, for one, is never set up in the
+        // crash process, so the widget refresher scheduling work there threw
+        // and took the crash dialog down with it. The dependencies above are
+        // injected lazily, so the crash process does not even build them.
+        if (!isMainAppProcess(getProcessName(), packageName)) return
+
         // No grandfather writer here, deliberately. The unconditional latch that
         // shipped in 2.0.2 was deleted for the free build. With PAID_ERA_CUTOFF
         // gone, `legacy_paid` is the only grandfather signal, so any writer in
@@ -220,8 +240,10 @@ class NineLivesApp : Application(), ImageLoaderFactory {
     }
 
     override fun onTerminate() {
-        connectivityMonitor.stopMonitoring()
-        syncManager.stop()
+        if (isMainAppProcess(getProcessName(), packageName)) {
+            connectivityMonitor.stopMonitoring()
+            syncManager.stop()
+        }
         super.onTerminate()
     }
 

app/src/main/java/com/ninelivesaudio/app/ui/navigation/BackRaceGuard.kt

@@ -0,0 +1,51 @@
+package com.ninelivesaudio.app.ui.navigation
+
+import android.annotation.SuppressLint
+import androidx.activity.compose.LocalOnBackPressedDispatcherOwner
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.DisposableEffect
+import androidx.navigation.NavController
+
+/**
+ * Drops a Back that the navigation host has not acted on yet whenever the
+ * back stack changes under it (#104).
+ *
+ * The host in navigation-compose 2.8 does not handle Back on the spot. It
+ * queues the work for the next turn of the main thread and, when that runs,
+ * animates the page it last drew on top. If the app changes the stack in the
+ * gap (Play finishing its load and opening the Player, the toolbar back
+ * arrow, a tab tap) that page is already gone, and the host throws
+ * "Cannot transition entry that is not in the back stack", which kills the
+ * app. Navigation 2.10 reworked this, but it needs a newer build toolchain
+ * than this project uses.
+ *
+ * Every change to the stack, from any caller, reaches this listener before
+ * the queued work runs, so cancelling here means the stale Back never runs.
+ * The page change that cancelled it already moved the user, so nothing they
+ * asked for is lost. A Back still in progress when the user lifts their
+ * finger acts on whatever page is now on top.
+ *
+ * The listener also fires once when it is registered (Navigation reports the
+ * current page to every new listener), and again after a configuration change
+ * re-registers it. Nothing is pending at those moments, so the cancel does
+ * nothing. That is intended, not a stray Back being dropped.
+ *
+ * It leans on two behaviors of activity-compose 1.9 and navigation-compose
+ * 2.8: the dispatcher's test-only cancel reaching the host's back handler,
+ * and that cancel stopping the queued work. BackGestureRaceTest is the gate
+ * for any activity or navigation version bump.
+ *
+ * Call it next to the NavHost, with the same controller.
+ */
+@SuppressLint("VisibleForTests") // the dispatcher's only way to cancel a Back
+@Composable
+fun CancelPendingBackOnNavigation(navController: NavController) {
+    val dispatcher = LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher ?: return
+    DisposableEffect(navController, dispatcher) {
+        val listener = NavController.OnDestinationChangedListener { _, _, _ ->
+            dispatcher.dispatchOnBackCancelled()
+        }
+        navController.addOnDestinationChangedListener(listener)
+        onDispose { navController.removeOnDestinationChangedListener(listener) }
+    }
+}

app/src/main/java/com/ninelivesaudio/app/ui/navigation/NineLivesNavHost.kt

@@ -52,6 +52,7 @@ fun NineLivesNavHost(
     modifier: Modifier = Modifier,
     startDestination: String = Routes.HOME,
 ) {
+    CancelPendingBackOnNavigation(navController)
     NavHost(
         navController = navController,
         startDestination = startDestination,