IssueTracker

Let's say you have two fragments, FragmentA and FragmentB. And let's say FragmentA launches FragmentB with the navigation library with some data, manipulates it, and then returns the manipulated result back to FragmentA.

Following the guide from https://developer.android.com/guide/navigation/use-graph/programmatic#returning_a_result, it's recommended to "listen" for navigation results via

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    val navController = findNavController();
    // We use a String here, but any type that can be put in a Bundle is supported
    navController.currentBackStackEntry?.savedStateHandle?.getLiveData<String>("key")?.observe(
        viewLifecycleOwner) { result ->
        // Do something with the result.
    }
}

And it also says:

If you’d only like to handle a result only once, you must call remove() on the SavedStateHandle to clear the result. If you do not remove the result, the LiveData will continue to return the last result to any new Observer instances.

There are two bugs going on here with this approach.

First Bug

This approach does not work with app restoration from process death. Or put it another way, doesn't work when testing with "Don't Keep Activities" in the developer options on the phone.

This is because navController.currentBackStackEntry is null at the time Fragment.onViewCreated is called on app restoration. In the normal app flow, it will be non-null.

My solution to this problem is to use the currentBackStackEntryFlow to essentially wait until there is a NavBackStackEntry and then observe on the LiveData. Here would be an example of the code:

launchOnLifecycle(state = Lifecycle.State.STARTED) {
    val navController = findNavController()
    // We use a String here, but any type that can be put in a Bundle is supported
    findNavController().currentBackStackEntryFlow.collectLatest {
        it.savedStateHandle.getLiveData<String>("key").observe(viewLifecycleOwner) { result ->
            // Do something with the result
        }
    }
}

However, I do not fully know / understand the ramifications of this code. If the flow emits multiple times, then it will add multiple duplicate observers. So this seems like a pretty big flaw in my solution. But I am not sure if there will be multiple emits to the collect. So Android community / Google, please find flaws in this!

Second Bug

If you want to consume the result once, the guide recommends this:

If you’d only like to handle a result only once, you must call remove() on the SavedStateHandle to clear the result. If you do not remove the result, the LiveData will continue to return the last result to any new Observer instances.

However there are flaws with this. While it does fix the problem where the result won't be emitted from the observer on device rotation (cause in this case, you only want to consume the result once and have it never emit again on rotation), but it does not allow for subsequent results.

Let's go back to my initial example with FragmentA and FragmentB but with one more additional step. Let's say FragmentA launches FragmentB with some data, manipulates it, and then returns the manipulated result back to FragmentA. FragmentA consumes the result and calls SavedStateHandle.remove(...).

Now let's say FragmentA does the same again. It launches FragmentB which returns the result back to FragmentA a second time. Since remove was called after consuming the first result, the second result will no longer be consumed because remove also removes the LiveData from the SavedStateHandle. This is the bug.

The code using the guide's example (and without my first bug fix) would look something like this.

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    val savedStateHandle = findNavController().currentBackStackEntry?.savedStateHandle

    // We use a String here, but any type that can be put in a Bundle is supported
    savedStateHandle?.getLiveData<String>("key")?.observe(
        viewLifecycleOwner) { result ->
        // Do something with the result.

        // Now remove key
        savedStateHandle.remove<String>("key")
    }
}

My solution would be to keep the LiveData alive but only null out the value of the LiveData to "consume" the result. My solution, which I don't like mind you, would be:

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    val savedStateHandle = findNavController().currentBackStackEntry?.savedStateHandle

    // We use a String here, but any type that can be put in a Bundle is supported
    val liveData = savedStateHandle?.getLiveData<String>("key")
    liveData?.observe(viewLifecycleOwner) { result ->
        // If result was already consumed, don't do anything
        if (liveData.value == null) return@observe

        // Do something with the result.

        // Now "consume" result
        liveData.value = null // Do this to "consume" result once rather than `savedStateHandle.remove<String>("key")`
    }
}

The obvious flaw here is that nullable results cannot be used as null here means that we consumed the result.

Combined Solutions

So putting my two solutions together, the code would look something like this:

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    launchOnLifecycle(state = Lifecycle.State.STARTED) {
        findNavController().currentBackStackEntryFlow.collectLatest {
            // We use a String here, but any type that can be put in a Bundle is supported
            val liveData = it.savedStateHandle.getLiveData<String>("key")
            liveData.observe(viewLifecycleOwner) { result ->
                if (liveData.value == null) return@observe

                // Do something with the result

                liveData.value = null
            }
        }
    }
}

My Ask

So my ask of you, Google, is to have a better approach to app process death & restoration when returning results as well as having better support for consuming results once while still allowing for subsequent results. Once a result is consumed, it should not be emitted again on device rotation for example.

Improvements

I did make some generic extension functions for anyone wanting to use my gross solutions that works setting & getting fragment results.

You can put these extension functions in some FragmentUtils or NavigationUtils class.

Using my example of FragmentA and FragmentB, FragmentA would use this extension function:

inline fun <reified T> Fragment.getNonNullNavigationResult(crossinline doOnResult: (T) -> Unit) {
    val key = T::class.java.name

    launchOnLifecycle {
        // Must wait until currentBackStackEntry is set until we can use it.
        // It will be null initially when the OS restores the fragment state restoration,
        // but non-null during normal app usage. In other words, it's a fix for DNKA.
        findNavController().currentBackStackEntryFlow.collectLatest {
            val liveData = it.savedStateHandle.getLiveData<T>(key)
            liveData.observe(viewLifecycleOwner) { result ->
                // Ignore any null results as this means that the result has already been consumed
                if (result == null) return@observe

                doOnResult(result)

                // Consume result by clearing out the value so that way
                // any subsequent calls do not trigger any actions
                liveData.value = null
            }
        }
    }
}

And FragmentB would use this extension function:

inline fun <reified T> Fragment.setNonNullNavigationResult(result: T) {
    val key = T::class.java.name

    findNavController().previousBackStackEntry?.savedStateHandle?.set(key, result)
}