Nested fragment's ViewModel never cleared [296173018]

Fixed

Bug

Status Update

No update yet.

Description

te...@gmail.com

created issue #1

Aug 16, 2023 07:46AM

Component used: Navigation
Version used: 2.6.0
Devices/Android versions reproduced on: every device & tested in above SDK 23

If this is a bug in the library, we would appreciate if you could attach:
- Sample project to trigger the issue.
- A screenrecord or screenshots showing the issue (if UI related).

Issue:

When using navigation component in fragment, the destination fragment's viewModel (which configured savedState & restoreState) is not cleared even though the destination popped from backstack.

For clearing destination's VM, I should call `navController.clearBackStack(destinationId)`.
I know It is intended feature for preserving VM's state for later restore.

However, the problem happens in the situation that the destination fragment has nested fragment and nested fragment has its VM (call nested VM).

Nested VM never cleared when call `navController.clearBackStack(destinationId)` of parent fragment.
Sadly, VM never cleared even though activity is destroyed. (if application not terminated)

I attached sample project and screen record for reproduce.

navigationcomponentsample.zip

446 KB

Download

navigation_component_bug1.mov

33 MB

Download

navigation_component_bug2.mov

22 MB

Download

Comments

jb...@google.com <jb...@google.com> Aug 16, 2023 01:37PM

Assigned to cl...@google.com.

te...@gmail.com <te...@gmail.com> #2Oct 23, 2023 07:15AM

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

cl...@google.com <cl...@google.com> Oct 24, 2023 01:43AM

Accepted by cl...@google.com.

cl...@google.com <cl...@google.com> #3Oct 24, 2023 09:10PM

Marked as fixed.

Fixed internally and will be available in fragment 1.6.2.

ap...@google.com <ap...@google.com> #4Oct 24, 2023 09:12PM

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

commit 6899e1031ecb6d9421c0e986a9290388c84e8a27
Author: Clara Fok <clarafok@google.com>
Date: Mon Oct 23 18:33:17 2023

Fix nested ViewModel not cleared

Ensure that child Fragment's ViewModel is cleared when the Parent ViewModel is destroyed during clearBackStack.

Test: ./gradlew fragment:fragment:cC
Bug: 296173018
Relnote: "Nested Fragment's ViewModel will now be cleared when parent ViewModel is cleared"
Change-Id: I6d83cd93421837b213d25d2fdbe5405348cd2637

M fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentManagerViewModelTest.kt
M fragment/fragment/src/androidTest/java/androidx/fragment/app/SaveRestoreBackStackTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentManagerViewModel.java
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentStateManager.java

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

te...@gmail.com <te...@gmail.com> #5Oct 25, 2023 02:29AM

Great!

na...@google.com <na...@google.com> #6Nov 1, 2023 05:20PM

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

androidx.fragment:fragment:1.6.2

pr...@google.com <pr...@google.com> #7Nov 29, 2023 09:39PM

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

androidx.fragment:fragment:1.7.0-alpha07

Issue 296173018

Description

Issue summary

Comments

jb...@google.com <jb...@google.com> Aug 16, 2023 01:37PM

te...@gmail.com <te...@gmail.com> #2Oct 23, 2023 07:15AM

cl...@google.com <cl...@google.com> Oct 24, 2023 01:43AM

cl...@google.com <cl...@google.com> #3Oct 24, 2023 09:10PM

ap...@google.com <ap...@google.com> #4Oct 24, 2023 09:12PM

te...@gmail.com <te...@gmail.com> #5Oct 25, 2023 02:29AM

na...@google.com <na...@google.com> #6Nov 1, 2023 05:20PM

pr...@google.com <pr...@google.com> #7Nov 29, 2023 09:39PM

Add comment

Issue metadata