Merge "Mark destination navigating away from as transitioning" into a…

…ndroidx-main
ggfan · Jul 29, 2021 · 2ecff9d · 2ecff9d
2 parents 2e9d375 + e3b742c
commit 2ecff9d
Show file tree

Hide file tree

Showing 11 changed files with 224 additions and 177 deletions.
diff --git a/navigation/navigation-common/api/current.txt b/navigation/navigation-common/api/current.txt
@@ -487,17 +487,14 @@ package androidx.navigation {
     ctor public NavigatorState();
     method public abstract androidx.navigation.NavBackStackEntry createBackStackEntry(androidx.navigation.NavDestination destination, android.os.Bundle? arguments);
     method public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> getBackStack();
-    method public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> getTransitionsInProgress();
+    method public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> getTransitionsInProgress();
+    method public void markTransitionComplete(androidx.navigation.NavBackStackEntry entry);
     method public void pop(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
+    method public void popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
     method public void push(androidx.navigation.NavBackStackEntry backStackEntry);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
+    method public void pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
     property public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> backStack;
-    property public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> transitionsInProgress;
-  }
-
-  public static fun interface NavigatorState.OnTransitionCompleteListener {
-    method public void onTransitionComplete();
+    property public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> transitionsInProgress;
   }
 
   @androidx.navigation.NavOptionsDsl public final class PopUpToBuilder {

diff --git a/navigation/navigation-common/api/public_plus_experimental_current.txt b/navigation/navigation-common/api/public_plus_experimental_current.txt
@@ -487,17 +487,14 @@ package androidx.navigation {
     ctor public NavigatorState();
     method public abstract androidx.navigation.NavBackStackEntry createBackStackEntry(androidx.navigation.NavDestination destination, android.os.Bundle? arguments);
     method public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> getBackStack();
-    method public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> getTransitionsInProgress();
+    method public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> getTransitionsInProgress();
+    method public void markTransitionComplete(androidx.navigation.NavBackStackEntry entry);
     method public void pop(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
+    method public void popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
     method public void push(androidx.navigation.NavBackStackEntry backStackEntry);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
+    method public void pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
     property public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> backStack;
-    property public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> transitionsInProgress;
-  }
-
-  public static fun interface NavigatorState.OnTransitionCompleteListener {
-    method public void onTransitionComplete();
+    property public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> transitionsInProgress;
   }
 
   @androidx.navigation.NavOptionsDsl public final class PopUpToBuilder {

diff --git a/navigation/navigation-common/api/restricted_current.txt b/navigation/navigation-common/api/restricted_current.txt
@@ -487,17 +487,14 @@ package androidx.navigation {
     ctor public NavigatorState();
     method public abstract androidx.navigation.NavBackStackEntry createBackStackEntry(androidx.navigation.NavDestination destination, android.os.Bundle? arguments);
     method public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> getBackStack();
-    method public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> getTransitionsInProgress();
+    method public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> getTransitionsInProgress();
+    method public void markTransitionComplete(androidx.navigation.NavBackStackEntry entry);
     method public void pop(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
+    method public void popWithTransition(androidx.navigation.NavBackStackEntry popUpTo, boolean saveState);
     method public void push(androidx.navigation.NavBackStackEntry backStackEntry);
-    method public androidx.navigation.NavigatorState.OnTransitionCompleteListener pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
+    method public void pushWithTransition(androidx.navigation.NavBackStackEntry backStackEntry);
     property public final kotlinx.coroutines.flow.StateFlow<java.util.List<androidx.navigation.NavBackStackEntry>> backStack;
-    property public final kotlinx.coroutines.flow.StateFlow<java.util.Map<androidx.navigation.NavBackStackEntry,androidx.navigation.NavigatorState.OnTransitionCompleteListener>> transitionsInProgress;
-  }
-
-  public static fun interface NavigatorState.OnTransitionCompleteListener {
-    method public void onTransitionComplete();
+    property public final kotlinx.coroutines.flow.StateFlow<java.util.Set<androidx.navigation.NavBackStackEntry>> transitionsInProgress;
   }
 
   @androidx.navigation.NavOptionsDsl public final class PopUpToBuilder {

diff --git a/navigation/navigation-common/src/main/java/androidx/navigation/NavigatorState.kt b/navigation/navigation-common/src/main/java/androidx/navigation/NavigatorState.kt
@@ -18,7 +18,6 @@ package androidx.navigation
 
 import android.os.Bundle
 import androidx.annotation.RestrictTo
-import androidx.navigation.NavigatorState.OnTransitionCompleteListener
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
@@ -32,9 +31,8 @@ import kotlin.concurrent.withLock
 public abstract class NavigatorState {
     private val backStackLock = ReentrantLock(true)
     private val _backStack: MutableStateFlow<List<NavBackStackEntry>> = MutableStateFlow(listOf())
-    private val _transitionsInProgress:
-        MutableStateFlow<Map<NavBackStackEntry, OnTransitionCompleteListener>> =
-            MutableStateFlow(mapOf())
+    private val _transitionsInProgress: MutableStateFlow<Set<NavBackStackEntry>> =
+        MutableStateFlow(setOf())
 
     /**
      * @hide
@@ -52,13 +50,11 @@ public abstract class NavigatorState {
     public val backStack: StateFlow<List<NavBackStackEntry>> = _backStack.asStateFlow()
 
     /**
-     * This is the map of currently running transitions to their individual
-     * [OnTransitionCompleteListener]s. Use this map to retrieve the listener and execute the
-     * callback once the transition is complete.
+     * This is the set of currently running transitions. Use this set to retrieve the entry and call
+     * [markTransitionComplete] once the transition is complete.
      */
-    public val transitionsInProgress:
-        StateFlow<Map<NavBackStackEntry, OnTransitionCompleteListener>> =
-            _transitionsInProgress.asStateFlow()
+    public val transitionsInProgress: StateFlow<Set<NavBackStackEntry>> =
+        _transitionsInProgress.asStateFlow()
 
     /**
      * Adds the given [backStackEntry] to the [backStack].
@@ -70,15 +66,25 @@ public abstract class NavigatorState {
     }
 
     /**
-     * Provides listener that once activated, adds the given [backStackEntry] to the [backStack].
+     * Adds the given [backStackEntry] to the [backStack]. This also adds the given and
+     * previous entry to the [set of in progress transitions][transitionsInProgress].
+     * Added entries have their [Lifecycle] capped at [Lifecycle.State.STARTED] until an entry is
+     * passed into the [markTransitionComplete] callback, when they are allowed to go to
+     * [Lifecycle.State.RESUMED].
+     *
+     * @see transitionsInProgress
+     * @see markTransitionComplete
+     * @see popWithTransition
      */
-    public open fun pushWithTransition(
-        backStackEntry: NavBackStackEntry
-    ): OnTransitionCompleteListener {
-        push(backStackEntry)
-        return OnTransitionCompleteListener {
-            removeInProgressTransition(backStackEntry)
+    public open fun pushWithTransition(backStackEntry: NavBackStackEntry) {
+        val previousEntry = backStack.value.lastOrNull()
+        // When navigating, we need to mark the outgoing entry as transitioning until it
+        // finishes its outgoing animation.
+        if (previousEntry != null) {
+            _transitionsInProgress.value = _transitionsInProgress.value + previousEntry
         }
+        _transitionsInProgress.value = _transitionsInProgress.value + backStackEntry
+        push(backStackEntry)
     }
 
     /**
@@ -100,50 +106,46 @@ public abstract class NavigatorState {
     }
 
     /**
-     * Provides listener that once activated, Pops all destinations up to and including [popUpTo].
+     * Pops all destinations up to and including [popUpTo]. This also adds the given and
+     * incoming entry to the [set of in progress transitions][transitionsInProgress]. Added
+     * entries have their [Lifecycle] held at [Lifecycle.State.CREATED] until an entry is
+     * passed into the [markTransitionComplete] callback, when they are allowed to go to
+     * [Lifecycle.State.DESTROYED] and have their state cleared.
      *
      * This will remove those destinations from the [backStack], saving their state if
      * [saveState] is `true`.
+     *
+     * @see transitionsInProgress
+     * @see markTransitionComplete
+     * @see pushWithTransition
      */
-    public open fun popWithTransition(
-        popUpTo: NavBackStackEntry,
-        saveState: Boolean
-    ): OnTransitionCompleteListener {
-        val listener = OnTransitionCompleteListener {
-            removeInProgressTransition(popUpTo)
+    public open fun popWithTransition(popUpTo: NavBackStackEntry, saveState: Boolean) {
+        _transitionsInProgress.value = _transitionsInProgress.value + popUpTo
+        val incomingEntry = backStack.value.lastOrNull { entry ->
+            entry != popUpTo &&
+                backStack.value.lastIndexOf(entry) < backStack.value.lastIndexOf(popUpTo)
+        }
+        // When popping, we need to mark the incoming entry as transitioning so we keep it
+        // STARTED until the transition completes at which point we can move it to RESUMED
+        if (incomingEntry != null) {
+            _transitionsInProgress.value = _transitionsInProgress.value + incomingEntry
         }
         pop(popUpTo, saveState)
-        return listener
     }
 
     /**
-     * Adds a transition listener to the group of in progress transitions.
+     * This removes the given [NavBackStackEntry] from the [set of the transitions in
+     * progress][transitionsInProgress]. This should be called in conjunction with
+     * [pushWithTransition] and [popWithTransition] as those call are responsible for adding
+     * entries to [transitionsInProgress].
      *
-     * @hide
-     */
-    @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
-    public fun addInProgressTransition(
-        entry: NavBackStackEntry,
-        listener: OnTransitionCompleteListener
-    ) {
-        _transitionsInProgress.value = _transitionsInProgress.value + (entry to listener)
-    }
-
-    /**
-     * @hide
+     * Failing to call this method could result in entries being prevented from reaching their
+     * final [Lifecycle.State]}.
+     *
+     * @see pushWithTransition
+     * @see popWithTransition
      */
-    @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
-    public fun removeInProgressTransition(entry: NavBackStackEntry) {
+    public open fun markTransitionComplete(entry: NavBackStackEntry) {
         _transitionsInProgress.value = _transitionsInProgress.value - entry
     }
-
-    /**
-     * OnTransitionCompleteListener receives a callback when a destination transition is complete.
-     */
-    public fun interface OnTransitionCompleteListener {
-        /**
-         * Callback for when the transition has completed.
-         */
-        public fun onTransitionComplete()
-    }
 }
diff --git a/navigation/navigation-compose/src/main/java/androidx/navigation/compose/ComposeNavigator.kt b/navigation/navigation-compose/src/main/java/androidx/navigation/compose/ComposeNavigator.kt
@@ -59,6 +59,18 @@ public class ComposeNavigator : Navigator<Destination>() {
         state.popWithTransition(popUpTo, savedState)
     }
 
+    /**
+     * Callback that removes the given [NavBackStackEntry] from the [map of the transitions in
+     * progress][transitionsInProgress]. This should be called in conjunction with [navigate] and
+     * [popBackStack] as those call are responsible for adding entries to [transitionsInProgress].
+     *
+     * Failing to call this method could result in entries being prevented from reaching their
+     * final [Lifecycle.State]}.
+     */
+    internal fun onTransitionComplete(entry: NavBackStackEntry) {
+        state.markTransitionComplete(entry)
+    }
+
     /**
      * NavDestination specific to [ComposeNavigator]
      */

diff --git a/navigation/navigation-compose/src/main/java/androidx/navigation/compose/NavHost.kt b/navigation/navigation-compose/src/main/java/androidx/navigation/compose/NavHost.kt
@@ -118,7 +118,7 @@ public fun NavHost(
     val backStack by composeNavigator.backStack.collectAsState()
     val transitionsInProgress by composeNavigator.transitionsInProgress.collectAsState()
 
-    val backStackEntry = transitionsInProgress.keys.lastOrNull { entry ->
+    val backStackEntry = transitionsInProgress.lastOrNull { entry ->
         entry.lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)
     } ?: backStack.lastOrNull { entry ->
         entry.lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)
@@ -137,13 +137,13 @@ public fun NavHost(
                     // There's no animation for the initial crossfade,
                     // so we can instantly mark the transition as complete
                     transitionsInProgress.forEach { entry ->
-                        entry.value.onTransitionComplete()
+                        composeNavigator.onTransitionComplete(entry)
                     }
                     initialCrossfade = false
                 }
                 onDispose {
                     transitionsInProgress.forEach { entry ->
-                        entry.value.onTransitionComplete()
+                        composeNavigator.onTransitionComplete(entry)
                     }
                 }
             }

diff --git a/...tion/navigation-runtime/src/androidTest/java/androidx/navigation/NavBackStackEntryTest.kt b/...tion/navigation-runtime/src/androidTest/java/androidx/navigation/NavBackStackEntryTest.kt
@@ -388,10 +388,55 @@ class NavBackStackEntryTest {
             .isTrue()
     }
 
-    private fun createNavController(): NavController {
+    @Suppress("DEPRECATION")
+    @UiThreadTest
+    @Test
+    fun testOnClearedWhenHostClearedAfterSaveStateWithTransitions() {
+        val hostStore = ViewModelStore()
+        val navController = createNavController(true)
+        navController.setViewModelStore(hostStore)
+        val navGraph = navController.navigatorProvider.navigation(
+            id = 1,
+            startDestination = R.id.start_test
+        ) {
+            test(R.id.start_test)
+        }
+        navController.setGraph(navGraph, null)
+
+        val owner = navController.getBackStackEntry(R.id.start_test)
+        assertThat(owner).isNotNull()
+        val viewModel: TestAndroidViewModel = ViewModelProvider(owner).get()
+        assertThat(viewModel.isCleared).isFalse()
+
+        // Navigate to a new instance of start_test, popping the previous one and saving state
+        navController.navigate(
+            R.id.start_test,
+            null,
+            navOptions {
+                popUpTo(R.id.start_test) {
+                    inclusive = true
+                    saveState = true
+                }
+            }
+        )
+        val newEntry = navController.getBackStackEntry(R.id.start_test)
+        navController.navigatorProvider[TestNavigator::class].onTransitionComplete(newEntry)
+
+        assertWithMessage("ViewModel should be saved when the destination is saved")
+            .that(viewModel.isCleared)
+            .isFalse()
+
+        hostStore.clear()
+
+        assertWithMessage("ViewModel should be cleared when the host is cleared")
+            .that(viewModel.isCleared)
+            .isTrue()
+    }
+
+    private fun createNavController(withTransitions: Boolean = false): NavController {
         val navController = NavHostController(ApplicationProvider.getApplicationContext())
         navController.setLifecycleOwner(TestLifecycleOwner())
-        val navigator = TestNavigator()
+        val navigator = TestNavigator(withTransitions)
         navController.navigatorProvider.addNavigator(navigator)
         return navController
     }