Restore state with ID exception due to failure to find destination [339908057]

Fixed

Bug

Status Update

No update yet.

Description

cl...@google.com

created issue #1

May 10, 2024 10:34PM

Repro test. Cause likely due to findDestination within restore state not searching through entire graph. (Route pattern restore delegates to id-based implementation)

    @UiThreadTest
    @Test
    fun testSaveState3() {
        val navController = createNavController()
        val graph = createNavController().createGraph(route = "root", startDestination = "graphA") {
            navigation(route = "graphA", startDestination = "A1") {
                test("A1")
                test("A2")
            }
            navigation(route = "graphB", startDestination = "B1") {
                test("B1")
            }
        }
        navController.graph = graph
        navController.navigate("A2")

        assertThat(navController.currentDestination?.route).isEqualTo("A2")

        navController.navigate("graphB") {
            popUpTo("A2") {
                inclusive = true
                saveState = true
            }
        }

        navController.navigate("A2") {
            restoreState = true
        }
    }

Note: possible duplication b/334120900, need investigation

Comments

cl...@google.com <cl...@google.com> May 15, 2024 09:33PM

Accepted by cl...@google.com.

ap...@google.com <ap...@google.com> #2May 22, 2024 10:07PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3May 22, 2024 10:07PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3b2381082be7e3c3ab1c11131deb7fef1a82845e
Author: Clara Fok <clarafok@google.com>
Date: Tue May 14 12:31:40 2024

Add findDestination and findNode that recurses child nodes

Also consolidated the helpers that recurses through children.

Test: existing tests
Bug: 339908057

Change-Id: Idbf28e7ceffec27a591b81a999f142228bd222bd

M navigation/navigation-common/src/main/java/androidx/navigation/NavGraph.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/3089426

ap...@google.com <ap...@google.com> #4May 22, 2024 10:07PM

Project: platform/frameworks/support
Branch: androidx-main

commit e2375fafab10ce0747e7809b7be2d73c29c51c13
Author: Clara Fok <clarafok@google.com>
Date: Thu May 02 14:38:11 2024

Clean up findNode with id

Clean up in preparation for upcoming refactors.

Removed searchParents boolean by making `searchParents = true` a default behavior. Existing call sites of findeNode with `searchParents = false` essentially only searches through direct children, so they can just delegate to NavGraph.nodes.

Test: existing tests
Bug: 339908057
Change-Id: I7992a40d67c5244822d6ddc9443aab1c61667432

M navigation/navigation-common/src/main/java/androidx/navigation/NavGraph.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavGraphNavigator.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/3089425

ap...@google.com <ap...@google.com> #5May 22, 2024 11:06PM

Project: platform/frameworks/support
Branch: androidx-main

commit 8bb1f1cb1606dc128dfc2b3995016af3d9eb5e50
Author: Clara Fok <clarafok@google.com>
Date: Wed May 15 11:22:18 2024

Fix restoreState that is based on id

Id based savedState restoration searches for target destination by recursing up the parents. This mean you cannot directly restore a nested destination in a sibling graph.
This isn’t an issue in a strictly id-based world since you cannot navigate directly to a sibling graph to begin with.

However, these route based restoration delegates to id-based restoration:
Route pattern
Route from KClass

Given that route APIs allow navigation to sibling graphs directly, it should also allow restore state from sibling graphs. We add support for this by searching for the target dest by recursing through both the parents and its children.

Since clearBackStack eventually delegates to restore state, this change also fixes these route based clearBackStack that delegates to id implementation:
Route pattern
Route from KClass

Test: ./gradlew navigation:navigation-runtime:cC
Bug: 339908057
Relnote: "Fixed NavController state restoration crash during operations such as clearBackStackEntry."
Change-Id: I423c3f6d8ee27be46450dcb53c28272fe0a1e085

M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/3089428

cl...@google.com <cl...@google.com> #6May 22, 2024 11:28PM

Marked as fixed.

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

na...@google.com <na...@google.com> #7May 29, 2024 06:05PM

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

androidx.navigation:navigation-common:2.8.0-beta02
androidx.navigation:navigation-runtime:2.8.0-beta02

wa...@gmail.com <wa...@gmail.com> #8Jun 8, 2024 02:59PM

I still have this issue on 2.8.0-beta02. My app systematically crashes on the initial destination, when restored from the background, with this exception:

java.lang.IllegalStateException: Restoring the Navigation back stack failed: destination 129493933 cannot be found from the current destination ComposeNavGraph(0x0) startDestination={Destination(0x7ec3ee2) route=com.brewthings.app.ui.screens.navigation.Screen.Scanning}

cl...@google.com <cl...@google.com> #9Jun 10, 2024 04:37PM

Re comment#8 can you please file a separate bug and attach a sample app?

ab...@dexcom.com <ab...@dexcom.com> #10Jun 12, 2024 01:18PM

I have the same issue on 2.8.0-beta02 when revoking app permission and the process gets Killed

il...@google.com <il...@google.com> #11Jun 12, 2024 04:47PM

Re comment#10 - please also file a new issue with a sample project that reproduces your issue.

ab...@dexcom.com <ab...@dexcom.com> #12Jul 7, 2024 04:22AM

https://issuetracker.google.com/issues/339908057#comment11
thanks fixed in
Version 2.8.0-beta04

vi...@gmail.com <vi...@gmail.com> #13Aug 1, 2024 09:11PM

Will this fix land in 2.7.x?

cl...@google.com <cl...@google.com> #14Aug 1, 2024 10:01PM

As mentioned in comment#6, it is released in navigation 2.8.0-beta02. This fix is applied to that release onwards. So it will not be fixed in 2.7.x.

Message last modified on Aug 1, 2024 10:01PM