Fix FragmentNavigator crash on pop up to start destination
FragmentNavigator crashes when navigating with pop up to initial entry (first entry in stack) because we normally don't add initial entries to pendingOps when it is getting popped, so FragmentNavigator does not expect a pop callback for it from FragmentManager.
But in the case of popping up to initial entry (inclusive), we do get a pop callback from FragmentManager. We can differentiate an isolated pop intialEntry (which should not be added to pendingOps) from a popUpTo initialEntry based on the fact that if it is popUpTo, initialEntry would have been added to pendingOps already as an incomingEntry when the entry on top of it was getting popped. So if initialEntry was already in pendingOps, we will re-add it to pendingOps with isPop = true so that FragmentNavigator knows it is getting popped.
Test: ./gradlew navigation:navigation-fragment:cC
Bug: 314652481
Change-Id: I241a93d79cf7cbc7a41e0ff92e016dd7c786cf2e
diff --git a/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/NavControllerWithFragmentTest.kt b/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/NavControllerWithFragmentTest.kt
index 60cfb2c..bb70163 100644
--- a/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/NavControllerWithFragmentTest.kt
+++ b/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/NavControllerWithFragmentTest.kt
@@ -338,6 +338,51 @@
)
}
+ @Test
+ fun testPopToInitialInFragmentStarted() = withNavigationActivity {
+ navController.graph = navController.createGraph("first") {
+ fragment<EmptyFragment>("first")
+ fragment<PopToInitialInOnStartedFragment>("second")
+ fragment<EmptyFragment>("third")
+ }
+ navController.navigate("second")
+
+ val fm = supportFragmentManager.findFragmentById(R.id.nav_host)?.childFragmentManager
+ fm?.executePendingTransactions()
+
+ assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("third")
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
+ }
+
+ @Test
+ fun testPopInitialAndNavigateInitial() = withNavigationActivity {
+ navController.graph = navController.createGraph("first") {
+ fragment<EmptyFragment>("first")
+ fragment<EmptyFragment>("second")
+ }
+ val fm = supportFragmentManager.findFragmentById(R.id.nav_host)?.childFragmentManager
+ fm?.executePendingTransactions()
+
+ // pop first as initial entry, then navigate to second which is the new initial entry
+ navController.navigate(
+ "second",
+ navOptions { popUpTo("first") { inclusive = true } }
+ )
+
+ fm?.executePendingTransactions()
+
+ assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("second")
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
+ val navigator = navController.navigatorProvider.getNavigator(FragmentNavigator::class.java)
+ // the popUpTo and following navigation to second are both isolated
+ // fragment operations (initial entry) so neither of them would trigger a callback from FM
+ assertThat(navigator.pendingOps).isEmpty()
+ }
+
@LargeTest
@Test
fun testSystemBackPressAfterPopUpToStartDestinationOffBackStack() = withNavigationActivity {
@@ -453,6 +498,17 @@
}
}
+class PopToInitialInOnStartedFragment : Fragment(R.layout.strict_view_fragment) {
+ override fun onStart() {
+ super.onStart()
+ findNavController().navigate("third") {
+ popUpTo("first") {
+ inclusive = true
+ }
+ }
+ }
+}
+
class TestDialogFragment : DialogFragment() {
val dialogs = mutableListOf<Dialog>()
diff --git a/navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt b/navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt
index cbc54c65..f58d911 100644
--- a/navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt
+++ b/navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt
@@ -338,7 +338,14 @@
addPendingOps(incomingEntry.id)
}
// add pending ops here before any animation (if present) starts
- poppedList.filter { it.id != initialEntry.id }.forEach { entry ->
+ poppedList.filter { entry ->
+ // normally we don't add initialEntry to pending ops because the adding/popping
+ // of an isolated fragment does not trigger onBackStackCommitted. But if initial
+ // entry was already added to pendingOps, it was likely an incomingEntry that now
+ // needs to be popped, so we need to overwrite isPop to true here.
+ pendingOps.asSequence().map { it.first }.contains(entry.id) ||
+ entry.id != initialEntry.id
+ }.forEach { entry ->
addPendingOps(entry.id, isPop = true)
}
