Crash in 2.8.0 when quickly popping multiple pages with navigateUp() [365027664]

Fixed

Bug

Status Update

No update yet.

Description

va...@gmail.com

created issue #1

Sep 6, 2024 09:48AM

Component used: Navigation Version used: 2.8.0 Devices/Android versions reproduced on: A13 Samsung Galaxy A51, A10 Huawei Mate 20 pro.

          Fatal Exception: java.util.NoSuchElementException: List is empty.
       at kotlin.collections.CollectionsKt__MutableCollectionsKt.removeLast(MutableCollections.kt:257)
       at androidx.navigation.NavController.tryRelaunchUpToExplicitStack(NavController.kt:961)
       at androidx.navigation.NavController.navigateUp(NavController.kt:933)
// our app code trace

When quickly popping multiple destinations with nested graphs, the nav controller will throw in navigateUp()

Our graph structure:

 * **Legend:**                                           
 * * 🗺️: Navigation graph                              
 * * 📍: Destination                                   
 * * 🏁: Marks 🗺️/📍as the start of the parent graph   
 * * 🧩: Means 🗺️/📍is generated on external module          
 * -------------------------------------------------------
 *
 * * 🗺️[RootGraph]
 * * ∙∙∙∙∙∙∙∙↳📍[OtherDestinations]
 * * ∙∙∙∙∙∙∙∙↳📍[DownloadGame]
 * * ∙∙∙∙∙∙∙∙↳🗺️🏁[GFYGraph]
 * * ∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙↳📍🏁[GFYGames]
 * * ∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙↳📍[Settings]
 * * ∙∙∙∙∙∙∙∙↳🗺️[ProfileGraph]
 * * ∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙↳📍🏁[OwnProfile] <- reproduces from here
 * * ∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙∙↳📍[UserProfile]

Expected behavior: Please add safeguards to check for graph emptiness.

Comments

jb...@google.com <jb...@google.com> #2Sep 9, 2024 03:53AM

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

da...@gmail.com <da...@gmail.com> #3Sep 11, 2024 11:57AM

Hi, I created a sample code to reproduce the problem described in this issue:
https://github.com/DanielNogueiraAndroid/PredictiveBackHandler

Steps to reproduce issue:

1 - Open the app
2 - Navigate by clicking the navigate button until you get to item 3
3 - Press the hardware back button 3 times very quickly

Expected:

The app should show the text "Compose 1!"

Actual:

The app will crash with an IndexOutOfBoundsException

Message last modified on Sep 11, 2024 12:05PM

va...@gmail.com <va...@gmail.com> #4Sep 11, 2024 12:04PM

Yes we do see this crash with the latest activity version you mentioned. Thanks for the repro from #3

jb...@google.com <jb...@google.com> #5Sep 11, 2024 07:49PM

Trying this with the provided test app and I still can't get a crash? Are there any other instructions that may help here?

spamming_back_no_crash.webm

4.4 MB

Download

da...@gmail.com <da...@gmail.com> #6Sep 12, 2024 10:55AM

Hey I have update the

https://github.com/DanielNogueiraAndroid/PredictiveBackHandler so now the last button emulates clicking very fast on hardware back button.
You can see in the screen record back_all_crash_20240912_114158.webm

Is also is possible to do it clicking very fast on hardware back button you can see in the screen record:
human_click_crash_20240912_115148.webm

Message last modified on Sep 12, 2024 12:33PM

back_all_crash_20240912_114158.webm

2.7 MB

Download

human_click_crash_20240912_115148.webm

3.8 MB

Download

jb...@google.com <jb...@google.com> Sep 26, 2024 04:08PM

Assigned to jb...@google.com.

ap...@google.com <ap...@google.com> #7Oct 4, 2024 10:41PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Jeremy Woods <jbwoods@google.com>
Link: https://android-review.googlesource.com/3285000

Fix failing PredictiveBackHandler enabled changes

Expand for full commit details

Fix failing PredictiveBackHandler enabled changes

As part of asop/3164566 we attempted to fix an error with
PredictiveBackHandler by closing the internal onBackInstance when the
enabled status changed. This would have worked, except by the time we
made it to the LaunchedEffect the onBackInstance was always null, so we
weren't closing anything.

I believe the cause of this is that `onBackInstance` was a field of a
function and we couldn't reference it across different calls. Because we
needed to change it as part of the backCallback, it also could not be
remembered.

So instead of using the onBackInstance, we can just use the callback
`isEnabled` state directly by passing the callback to the
`OnBackInstance`. This ensures that we only fire the `onBack` callback
when the function is actually enabled.

Test: PredictiveBackHandlerTest, tested and confirmed in sample app
Bug: 365027664
Bug: 340202286
RelNote: "`PredictiveBackHandler` will no longer fire it's callback
after the handler has been disabled. This will fix an issue where
the `NavHost` from Navigation Compose would throw an
`IndexOutOfBoundsException`."

Change-Id: I3f75eb2415f39b914f18cf4b87bf4ed57bb5a483

Files:

M activity/activity-compose/src/androidTest/java/androidx/activity/compose/PredictiveBackHandlerTest.kt
M activity/activity-compose/src/main/java/androidx/activity/compose/PredictiveBackHandler.kt

Hash: 1658eca1da25d8092f3cd48806946edcdec73688
Date: Thu Sep 26 18:15:59 2024

jb...@google.com <jb...@google.com> #8Oct 8, 2024 04:24PM

Marked as fixed.

This has been fixed internally and is available in the Activity 1.9.3 release.

na...@google.com <na...@google.com> #9Oct 16, 2024 05:48PM

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

androidx.activity:activity-compose:1.9.3

pr...@google.com <pr...@google.com> #10Oct 30, 2024 05:38PM

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

androidx.activity:activity-compose:1.10.0-alpha03

Issue 365027664

Description

Issue summary

Comments

jb...@google.com <jb...@google.com> #2Sep 9, 2024 03:53AM

da...@gmail.com <da...@gmail.com> #3Sep 11, 2024 11:57AM

Steps to reproduce issue:

Expected:

Actual:

va...@gmail.com <va...@gmail.com> #4Sep 11, 2024 12:04PM

jb...@google.com <jb...@google.com> #5Sep 11, 2024 07:49PM

da...@gmail.com <da...@gmail.com> #6Sep 12, 2024 10:55AM

jb...@google.com <jb...@google.com> Sep 26, 2024 04:08PM

ap...@google.com <ap...@google.com> #7Oct 4, 2024 10:41PM

jb...@google.com <jb...@google.com> #8Oct 8, 2024 04:24PM

na...@google.com <na...@google.com> #9Oct 16, 2024 05:48PM

pr...@google.com <pr...@google.com> #10Oct 30, 2024 05:38PM

Add comment

Issue metadata