Merge "Ensure FragmentNavigator does not destroy entries early" into androidx-main

Author: Treehugger Robot

navigation/navigation-fragment/src/androidTest/java/androidx/navigation/fragment/NavControllerWithFragmentTest.kt

@@ -24,6 +24,8 @@ import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
 import androidx.navigation.NavOptions
+import androidx.navigation.createGraph
+import androidx.navigation.fragment.test.EmptyFragment
 import androidx.navigation.fragment.test.NavigationActivity
 import androidx.navigation.fragment.test.R
 import androidx.navigation.navOptions
@@ -284,6 +286,20 @@ class NavControllerWithFragmentTest {
         assertThat(fm.fragments.size).isEqualTo(2) // start + last dialog fragment
     }
 
+    @Test
+    fun testPopEntryInFragmentResumed() = withNavigationActivity {
+        navController.graph = navController.createGraph("first") {
+            fragment<EmptyFragment>("first")
+            fragment<PopInOnResumeFragment>("second")
+        }
+        navController.navigate("second")
+
+        val fm = supportFragmentManager.findFragmentById(R.id.nav_host)?.childFragmentManager
+        fm?.executePendingTransactions()
+
+        assertThat(navController.currentBackStackEntry?.destination?.route).isEqualTo("first")
+    }
+
     private fun withNavigationActivity(
         block: NavigationActivity.() -> Unit
     ) {
@@ -295,6 +311,15 @@ class NavControllerWithFragmentTest {
     }
 }
 
+class PopInOnResumeFragment : Fragment(R.layout.strict_view_fragment) {
+    override fun onResume() {
+        super.onResume()
+        findNavController().navigate("first") {
+            popUpTo("first")
+        }
+    }
+}
+
 class TestDialogFragment : DialogFragment() {
     val dialogs = mutableListOf<Dialog>()
 

navigation/navigation-fragment/src/main/java/androidx/navigation/fragment/FragmentNavigator.kt

@@ -29,7 +29,6 @@ import androidx.fragment.app.FragmentManager.OnBackStackChangedListener
 import androidx.fragment.app.FragmentTransaction
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleEventObserver
-import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.viewmodel.CreationExtras
@@ -75,38 +74,34 @@ public open class FragmentNavigator(
      */
     internal val backStack get() = state.backStack
 
-    private val fragmentObserver = object : LifecycleEventObserver {
-        override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) {
-            if (event == Lifecycle.Event.ON_DESTROY) {
-                val fragment = source as Fragment
-                val entry = state.transitionsInProgress.value.lastOrNull { entry ->
-                    entry.id == fragment.tag
-                }
-                entry?.let {
-                    entriesToPop.remove(entry.id)
-                    state.markTransitionComplete(it)
+    private val fragmentObserver = LifecycleEventObserver { source, event ->
+        if (event == Lifecycle.Event.ON_DESTROY) {
+            val fragment = source as Fragment
+            val entry = state.transitionsInProgress.value.lastOrNull { entry ->
+                entry.id == fragment.tag
+            }
+            if (entry != null) {
+                entriesToPop.remove(entry.id)
+                if (!state.backStack.value.contains(entry)) {
+                    state.markTransitionComplete(entry)
                 }
-                fragment.lifecycle.removeObserver(this)
             }
         }
     }
 
     private val fragmentViewObserver = { entry: NavBackStackEntry ->
-        object : LifecycleEventObserver {
-            override fun onStateChanged(
-                source: LifecycleOwner,
-                event: Lifecycle.Event
-            ) {
-                // Once the lifecycle reaches RESUMED, we can mark the transition complete
-                if (event == Lifecycle.Event.ON_RESUME) {
-                    state.markTransitionComplete(entry)
-                }
-                // Once the lifecycle reaches DESTROYED, we can mark the transition complete and
-                // remove the observer.
-                if (event == Lifecycle.Event.ON_DESTROY) {
-                    entriesToPop.remove(entry.id)
+        LifecycleEventObserver { _, event ->
+            // Once the lifecycle reaches RESUMED, if the entry is in the back stack we can mark
+            // the transition complete
+            if (event == Lifecycle.Event.ON_RESUME && state.backStack.value.contains(entry)) {
+                state.markTransitionComplete(entry)
+            }
+            // Once the lifecycle reaches DESTROYED, if the entry is not in the back stack, we can
+            // mark the transition complete
+            if (event == Lifecycle.Event.ON_DESTROY) {
+                entriesToPop.remove(entry.id)
+                if (!state.backStack.value.contains(entry)) {
                     state.markTransitionComplete(entry)
-                    source.lifecycle.removeObserver(this)
                 }
             }
         }