Fixed
Status Update
No update yet.
Comments
se...@google.com
dh...@gmail.com
Actually, the issue is not nested-fragment-specific, it affects any fragment in a backstack.
The following scenario will also leak viewmodel scoped to NestedFragmentWithViewModel:
@Test
fun fragmentInBackstackLeaksViewModel() {
val scenario = ActivityScenario.launch(TestActivity::class.java)
scenario
.moveToState(Lifecycle.State.RESUMED)
.onActivity {
it.supportFragmentManager
.beginTransaction()
.replace(R.id.activity_root, NestedFragmentWithViewModel())
.addToBackStack("nested 1")
.commit()
}
.onActivity {
it.supportFragmentManager
.beginTransaction()
.replace(R.id.activity_root, NestedFragment())
.addToBackStack("nested 2")
.commit()
}
.apply {
println("\nrecreating")
recreate()
println("recreated")
}
.apply {
println("\ndestroying")
moveToState(Lifecycle.State.DESTROYED)
println("destroyed")
}
}
The following scenario will also leak viewmodel scoped to NestedFragmentWithViewModel:
@Test
fun fragmentInBackstackLeaksViewModel() {
val scenario = ActivityScenario.launch(TestActivity::class.java)
scenario
.moveToState(Lifecycle.State.RESUMED)
.onActivity {
it.supportFragmentManager
.beginTransaction()
.replace(R.id.activity_root, NestedFragmentWithViewModel())
.addToBackStack("nested 1")
.commit()
}
.onActivity {
it.supportFragmentManager
.beginTransaction()
.replace(R.id.activity_root, NestedFragment())
.addToBackStack("nested 2")
.commit()
}
.apply {
println("\nrecreating")
recreate()
println("recreated")
}
.apply {
println("\ndestroying")
moveToState(Lifecycle.State.DESTROYED)
println("destroyed")
}
}
il...@google.com
Yeah, looks like Fragments has *never* cleared the non-config state for Fragments that don't reach CREATED - turns out that the one place that is true is Fragments on the back stack after a configuration changes (they stay in the default INITIALIZING internal state).
This applies to both ViewModels not receiving onCleared() and direct child Fragments that are retained not receiving onDestroy() as well as grandchild Fragments and beyond (which may also be retained Fragments).
I've been able to replicate the ViewModel case inhttps://android-review.googlesource.com/941796 , still need to create a test for the retained Fragment case and dispatching the destruction to the child Fragments' FragmentManager instances. As well as actually fix the issue.
This applies to both ViewModels not receiving onCleared() and direct child Fragments that are retained not receiving onDestroy() as well as grandchild Fragments and beyond (which may also be retained Fragments).
I've been able to replicate the ViewModel case in
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit c53eb8ad0d6763d3893f046cc58b91c838e845ff
Author: jbwoods <jbwoods@google.com>
Date: Tue Apr 16 15:35:39 2019
Save fragment mRemoving on save and restore
The mRemoving flag is not saved when a fragment goes through save
state. This means when a configuration change (or any other save and
restore cycle), the flag is lost. If a fragment is on the backstack,
this can cause that fragment to never reach the CREATED state again.
Adding the mRemoving flag to FragmentState ensures that fragments on
the backstack get to the CREATED state. A fragment can now also get to the
DESTROYED state if its FragmentManager is destroyed, meaning the
non-config state (viewModels) is properly cleared.
Test: new ViewModelTest, new SaveStateFragmentTest, rerun old test
Fixes: 129593351
Change-Id: Ia1d10e96f569dee606de2ac19c586901b240b3cd
M fragment/src/androidTest/java/androidx/fragment/app/SaveStateFragmentTest.kt
M fragment/src/androidTest/java/androidx/fragment/app/ViewModelTest.kt
M fragment/src/androidTest/java/androidx/fragment/app/test/ViewModelActivity.kt
M fragment/src/main/java/androidx/fragment/app/FragmentState.java
https://android-review.googlesource.com/941796
https://goto.google.com/android-sha1/c53eb8ad0d6763d3893f046cc58b91c838e845ff
Branch: androidx-master-dev
commit c53eb8ad0d6763d3893f046cc58b91c838e845ff
Author: jbwoods <jbwoods@google.com>
Date: Tue Apr 16 15:35:39 2019
Save fragment mRemoving on save and restore
The mRemoving flag is not saved when a fragment goes through save
state. This means when a configuration change (or any other save and
restore cycle), the flag is lost. If a fragment is on the backstack,
this can cause that fragment to never reach the CREATED state again.
Adding the mRemoving flag to FragmentState ensures that fragments on
the backstack get to the CREATED state. A fragment can now also get to the
DESTROYED state if its FragmentManager is destroyed, meaning the
non-config state (viewModels) is properly cleared.
Test: new ViewModelTest, new SaveStateFragmentTest, rerun old test
Fixes: 129593351
Change-Id: Ia1d10e96f569dee606de2ac19c586901b240b3cd
M fragment/src/androidTest/java/androidx/fragment/app/SaveStateFragmentTest.kt
M fragment/src/androidTest/java/androidx/fragment/app/ViewModelTest.kt
M fragment/src/androidTest/java/androidx/fragment/app/test/ViewModelActivity.kt
M fragment/src/main/java/androidx/fragment/app/FragmentState.java
dh...@gmail.com
Thanks for operative work guys! I guess this will be rolled out with a new androidx.fragment release?
il...@google.com
Yes, it'll be part of Fragments 1.1.0-alpha07
Description
Devices/Android versions reproduced on: robolectric, emulator, real devices
I have an activity, and a fragment in it (MasterFragment).
Than I add nested fragment (NestedFragmentWithViewModel) to a MasterFragment.
NestedFragmentWithViewModel obtains viewmodel by `ViewModelProviders.of(this)`
Than I replace it (with adding to a backstack) with another fragment (NestedFragment).
After that, activity gets recreated, which is causing:
- MasterFragment to recrete
- NestedFragment (as it is on top of backstack) to recreate
After that, activity gets destroyed, which is causing:
- MasterFragment to destroy and clear its ViewModelStore
- NestedFragment to destroy and clear its ViewModelStore
But no one cares neither about NestedFragmentWithViewModel in the backstack, nor about clearing its ViewModelStore.
It is not cleared when fragment is added to a backstack (cause backstack can be popped to show fragment again).
And it is not cleared when MasterFragment destroys, cause it calls onDestroy on all its 'created' nested fragments, but fragments in a backstack were not created after activity get recreated, so should not be destroyed.
Fragment's ViewModelStore is cleared in fragment's onDestroy() in case activity.isFinishing(). That never happens so, whole ViewModelStore gets leaked.
It is not a totally FragmentManager issue, cause its respects lifecycle of fragments - 'what is dead may never die'.
So it seems fragment's logic to clear its ViewModelStore should be fixed.
Project with described scenario is attached.