Fix entry not marked complete after view is destroyed
As part of a fix for b/279644470 in aosp/2566210, when the fragment view is destroyed, we would mark the transitioning (outgoing) entry as complete only if the entry has also been removed from backstack. However, in the simple navigate() case where the outgoing entry is not popped and remains in the backstack, this entry is not marked complete and ends up in the visibleEntries even though the view has been destroyed.
Now the backstack check in ON_DESTROY is no longer necessary to fix the original b/279644470 bug -- the view being destroyed should be the right signal for completing entries regardless of whether transitioning effects are used. So this check is removed.
Test: ./gradlew navigation:navigation-fragment:cC
Test: ./gradlew navigation:navigation-runtime:cC
Bug: 288520638
(cherry picked from https://android-review.googlesource.com/q/commit:f61da31809827bfac89af629c88ad9b8423e88b6)
Merged-In: I5caa9af1b5bd7084e76d7daf9515f7430bf2489d
Change-Id: I5caa9af1b5bd7084e76d7daf9515f7430bf2489d
diff --git a/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt b/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt
index dc60ee0..5ae8346 100644
--- a/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt
+++ b/navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/FragmentNavigatorTest.kt
@@ -171,6 +171,7 @@
assertWithMessage("Replacement Fragment should be the primary navigation Fragment")
.that(fragmentManager.primaryNavigationFragment)
.isSameInstanceAs(replacementFragment)
+ assertThat(navigatorState.transitionsInProgress.value).isEmpty()
}
@UiThreadTest
@@ -359,6 +360,7 @@
assertWithMessage("Fragment should be the primary navigation Fragment after pop")
.that(fragmentManager.primaryNavigationFragment)
.isSameInstanceAs(fragment)
+ assertThat(navigatorState.transitionsInProgress.value).isEmpty()
}
@UiThreadTest
@@ -528,6 +530,26 @@
assertWithMessage("PrimaryFragment should be the correct type")
.that(fragmentManager.primaryNavigationFragment)
.isNotInstanceOf(EmptyFragment::class.java)
+ assertThat(navigatorState.transitionsInProgress.value).isEmpty()
+ }
+
+ @UiThreadTest
+ @Test
+ fun testMultipleNavigateFragmentTransactionsThenPopMultiple() {
+ val entry = createBackStackEntry()
+ val secondEntry = createBackStackEntry(SECOND_FRAGMENT, clazz = Fragment::class)
+ val thirdEntry = createBackStackEntry(THIRD_FRAGMENT)
+
+ // Push 3 fragments
+ fragmentNavigator.navigate(listOf(entry, secondEntry, thirdEntry), null, null)
+ fragmentManager.executePendingTransactions()
+
+ // Now pop multiple fragments with savedState so that the secondEntry does not get
+ // marked complete by clear viewModel
+ fragmentNavigator.popBackStack(secondEntry, true)
+ fragmentManager.executePendingTransactions()
+ assertThat(navigatorState.backStack.value).containsExactly(entry)
+ assertThat(navigatorState.transitionsInProgress.value).isEmpty()
}
@UiThreadTest
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 d000958..21e0c3c 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
@@ -74,6 +74,9 @@
assertWithMessage("New Entry should be RESUMED")
.that(navController.currentBackStackEntry!!.lifecycle.currentState)
.isEqualTo(Lifecycle.State.RESUMED)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry!!
+ )
}
@Ignore("b/276806142")
@Test
@@ -205,6 +208,7 @@
// ensure original Fragment is dismissed and backStacks are in sync
assertThat(navigator.backStack.value.size).isEqualTo(1)
assertThat(fm.fragments.size).isEqualTo(2) // start + dialog fragment
+ assertThat(navController.visibleEntries.value.size).isEqualTo(2)
}
@Test
@@ -298,6 +302,9 @@
fm?.executePendingTransactions()
assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("first")
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@LargeTest
@@ -323,6 +330,9 @@
onBackPressedDispatcher.onBackPressed()
assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("third")
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry!!
+ )
}
@LargeTest
@@ -352,6 +362,9 @@
onBackPressedDispatcher.onBackPressed()
assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("fourth")
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry!!
+ )
}
private fun withNavigationActivity(
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 3fb3248..0c8b113 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
@@ -85,16 +85,14 @@
entry.id == fragment.tag
}
if (entry != null) {
- if (!state.backStack.value.contains(entry)) {
- if (FragmentManager.isLoggingEnabled(Log.VERBOSE)) {
- Log.v(
- TAG,
- "Marking transition complete for entry $entry " +
- "due to fragment $source lifecycle reaching DESTROYED"
- )
- }
- state.markTransitionComplete(entry)
+ if (FragmentManager.isLoggingEnabled(Log.VERBOSE)) {
+ Log.v(
+ TAG,
+ "Marking transition complete for entry $entry " +
+ "due to fragment $source lifecycle reaching DESTROYED"
+ )
}
+ state.markTransitionComplete(entry)
}
}
}
@@ -113,19 +111,16 @@
}
state.markTransitionComplete(entry)
}
- // Once the lifecycle reaches DESTROYED, if the entry is not in the back stack, we can
- // mark the transition complete
+ // Once the lifecycle reaches DESTROYED, we can mark the transition complete
if (event == Lifecycle.Event.ON_DESTROY) {
- if (!state.backStack.value.contains(entry)) {
- if (FragmentManager.isLoggingEnabled(Log.VERBOSE)) {
- Log.v(
- TAG,
- "Marking transition complete for entry $entry due " +
- "to fragment $owner view lifecycle reaching DESTROYED"
- )
- }
- state.markTransitionComplete(entry)
+ if (FragmentManager.isLoggingEnabled(Log.VERBOSE)) {
+ Log.v(
+ TAG,
+ "Marking transition complete for entry $entry due " +
+ "to fragment $owner view lifecycle reaching DESTROYED"
+ )
}
+ state.markTransitionComplete(entry)
}
}
}
diff --git a/navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt b/navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
index 3b662ee..2ddc413 100644
--- a/navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
+++ b/navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
@@ -186,6 +186,9 @@
assertThat(navigator.backStack.size)
.isEqualTo(1)
assertThat(originalViewModel.isCleared).isTrue()
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -215,6 +218,7 @@
val newViewModel = ViewModelProvider(newBackStackEntry).get<TestAndroidViewModel>()
assertThat(newBackStackEntry.id).isSameInstanceAs(originalBackStackEntry.id)
assertThat(newViewModel).isSameInstanceAs(originalViewModel)
+ assertThat(navController.visibleEntries.value).containsExactly(newBackStackEntry)
}
@UiThreadTest
@@ -606,6 +610,9 @@
navController.navigate(R.id.second_test)
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.second_test)
assertThat(navigator.backStack.size).isEqualTo(2)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -2028,6 +2035,7 @@
.isFalse()
assertThat(navController.currentDestination).isNull()
assertThat(navigator.backStack.size).isEqualTo(0)
+ assertThat(navController.visibleEntries.value).isEmpty()
}
@UiThreadTest
@@ -2074,6 +2082,9 @@
.isTrue()
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.start_test)
assertThat(navigator.backStack.size).isEqualTo(1)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -2092,6 +2103,9 @@
navigator.popCurrent()
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.start_test)
assertThat(navigator.backStack.size).isEqualTo(1)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -2132,6 +2146,9 @@
)
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.second_test)
assertThat(navigator.backStack.size).isEqualTo(1)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -2172,6 +2189,9 @@
.isTrue()
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.start_test)
assertThat(navigator.backStack.size).isEqualTo(1)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
@@ -2229,6 +2249,9 @@
navController.navigate(R.id.self)
assertThat(navController.currentDestination?.id ?: 0).isEqualTo(R.id.second_test)
assertThat(navigator.backStack.size).isEqualTo(2)
+ assertThat(navController.visibleEntries.value).containsExactly(
+ navController.currentBackStackEntry
+ )
}
@UiThreadTest
diff --git a/navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt b/navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
index 338a3d8..db02a04 100644
--- a/navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
+++ b/navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
@@ -127,8 +127,9 @@
* whenever they change. If there is no visible [NavBackStackEntry], this will be set to an
* empty list.
*
- * - `CREATED` entries are listed first and include all entries that have been popped from
- * the back stack and are in the process of completing their exit transition
+ * - `CREATED` entries are listed first and include all entries that are in the process of
+ * completing their exit transition. Note that this can include entries that have been
+ * popped off the Navigation back stack.
* - `STARTED` entries on the back stack are next and include all entries that are running
* their enter transition and entries whose destination is partially covered by a
* `FloatingWindow` destination
