IssueTracker

since it is already marked as deprecated, we can probably do it by now.

Do you have a sample app with your specific refactors to reproduce that error?

I followed the steps to repro and I do get a long list in backStackStates where About page is stored multiple times. But that is because restoreState = false by default when navigating to About. If you set something like this in Title.kt

view.findViewById<Button>(R.id.about_btn).setOnClickListener {
            findNavController().navigate(R.id.action_title_to_about, null, navOptions { restoreState = true })
        }

Then backStackStates gets cleared when you navigate back to About. I'm not sure if what you're seeing is different. If so, can you please upload your specific sample?

Yes, if you change the navOptions for the button it will be fine. But under the default case, I suppose the error scenario would be doing the repetition above to fill up the backStackStates, then navigating to About, then to Leaderboard, and then press Home on the bottom nav bar. This would correctly restore the About page by taking one item from backStackStates. However, the other About states will still stay in backStackStates, and can in fact never be restored.

In the example with restoreState = true, this problem is only avoided because only one About item is able to be added to backStackStates (while popping up when navigating to Leaderboard) before it's restored (either by going Leaderboard > back Home > About, or Leaderboard > nav item Home).

The code below in executePopOperations is where the problem is:

if (saveState) {
    ...
    if (savedState.isNotEmpty()) {
        ...
        val firstStateDestination = findDestination(firstState.destinationId)
        generateSequence(firstStateDestination) { destination ->
            if (destination.parent?.startDestinationId == destination.id) {
                destination.parent
            } else {
                null
            }
        }.takeWhile { destination ->
            // Only add the state if it doesn't already exist
            !backStackMap.containsKey(destination.id)
        }.forEach { destination ->
            backStackMap[destination.id] = firstState.id
        }
        // And finally, store the actual state itself
        backStackStates[firstState.id] = savedState
    }
}

The firstState.id is added to backStackMap only if the state is not already saved, as per the comment. In the repetition case above, the About state will already be saved the second time navigating from About > Leaderboard, hence firstState.id will not be saved in this case. However savedState will still be added to backStackStates regardless (the last line).

Both of the restoreStateInternal functions contain:

val backStackId = backStackMap[id]
// Clear out the state we're going to restore so that it isn't restored a second time
backStackMap.values.removeAll { it == backStackId }
val backStackState = backStackStates.remove(backStackId)
...
val entries = instantiateBackStack(backStackState)
return executeRestoreState(entries, args, navOptions, navigatorExtras)

The first line gets the (single) saved id that is the key to the saved state array in backStackStates, then the fourth line removes this (single) saved state from backStackStates. This is the only place in NavController in which anything is removed from backStackStates (besides activity recreation). Since there is never going to be any backStackId that is the key for any of the other saved backStackStates for About, they will remain unreachable/unrestorable. The keys for these never got saved in backStackMap thanks to the takeWhile in executePopOperations above.

Obviously this is problematic because all of these states are just kept in memory for the life of the NavController object even though it is impossible that they will ever be used again. And in fact they are even all saved and all restored in saveState and restoreState if the activity is recreated.

I assume the solution would be something similar to:

val firstStateDestination = findDestination(firstState.destinationId)
generateSequence(firstStateDestination) { destination ->
    if (destination.parent?.startDestinationId == destination.id) {
        destination.parent
    } else {
        null
    }
}.takeWhile { destination ->
    // Only add the state if it doesn't already exist
    !backStackMap.containsKey(destination.id)
}.forEach { destination ->
    backStackMap[destination.id] = firstState.id
}
if (backStackMap.values.contains(firstState.id)) {
    backStackStates[firstState.id] = savedState
}

Fixed internally and will be available in navigation 2.8.0-alpha01.

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.navigation:navigation-runtime:2.8.0-alpha01