Fixed
Status Update
No update yet.
Comments
il...@google.com
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 5d97c41e83cb40f9d61f4fbb117a4214c336a91e
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Apr 06 22:17:31 2023
Ensure postponed with timeout does not cause leak
If you use postponeEnterTransition with a timeout, we add a runnable to
do the handler to ensure we eventually call
startPostponedEnterTransition, but we don't clear that runnable from the
handler when we do startPostponedEnterTransition. This means we continue
to have a reference to the runnable and causes the fragment to leak.
We should ensure we remove the runnable from the handler when we
startPostponedEnterTransition.
RelNote: "Using `postponeEnterTransition` with a timeout and then
replace the postponed fragment no longer results in leaking the
postponed fragment."
Test: added postponedTest
Bug: 276375110
Change-Id: I2ec7d50c9a08be4902dcc3cce4615bc1e19ed47d
M fragment/fragment/src/androidTest/java/androidx/fragment/app/PostponedTransitionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java
https://android-review.googlesource.com/2528173
Branch: androidx-main
commit 5d97c41e83cb40f9d61f4fbb117a4214c336a91e
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Apr 06 22:17:31 2023
Ensure postponed with timeout does not cause leak
If you use postponeEnterTransition with a timeout, we add a runnable to
do the handler to ensure we eventually call
startPostponedEnterTransition, but we don't clear that runnable from the
handler when we do startPostponedEnterTransition. This means we continue
to have a reference to the runnable and causes the fragment to leak.
We should ensure we remove the runnable from the handler when we
startPostponedEnterTransition.
RelNote: "Using `postponeEnterTransition` with a timeout and then
replace the postponed fragment no longer results in leaking the
postponed fragment."
Test: added postponedTest
Bug: 276375110
Change-Id: I2ec7d50c9a08be4902dcc3cce4615bc1e19ed47d
M fragment/fragment/src/androidTest/java/androidx/fragment/app/PostponedTransitionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java
ni...@ansman.se
I left a comment on the CL, but I think the handler should be reused in case postponeEnterTransition is called multiple times.
jb...@google.com
Reusing it would mean we could end up not using the host's handler when we are actually attached. Probably should just clear it and start over with the right handler.
ni...@hinge.co
Well the problem now, at least, is that if you call that twice we attempt to remove the old callbacks from the wrong handler (in some cases).
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 880c9f06b39cf910003606a5ab5e2f88f9062ccb
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Apr 11 19:26:19 2023
Ensure postponed fragments don't leak with multiple calls
We need to make sure when postponedEnterTransition is called multiple
times, we still don't cause a leak whether we are attached to a
FragmentManager or not.
So if we call postponedEnterTransition again, we remove the callback
from the previous handler and proceed as if we never made the first
call.
Test: added PostponedTransitionTest
Bug: 276375110
Change-Id: Id6d0d3bf30a109bb1020de9f66bd4d972373334d
M fragment/fragment/src/androidTest/java/androidx/fragment/app/PostponedTransitionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java
https://android-review.googlesource.com/2531863
Branch: androidx-main
commit 880c9f06b39cf910003606a5ab5e2f88f9062ccb
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Apr 11 19:26:19 2023
Ensure postponed fragments don't leak with multiple calls
We need to make sure when postponedEnterTransition is called multiple
times, we still don't cause a leak whether we are attached to a
FragmentManager or not.
So if we call postponedEnterTransition again, we remove the callback
from the previous handler and proceed as if we never made the first
call.
Test: added PostponedTransitionTest
Bug: 276375110
Change-Id: Id6d0d3bf30a109bb1020de9f66bd4d972373334d
M fragment/fragment/src/androidTest/java/androidx/fragment/app/PostponedTransitionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/Fragment.java
jb...@google.com
This has been fixed internally and will be available in the Fragment 1.5.7 and 1.6.0-beta01 releases.
pr...@google.com
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.fragment:fragment:1.5.7androidx.fragment:fragment:1.6.0-beta01
Description
Component used: Fragment
Version used:
1.5.6If you call
Fragment.postponeEnterTransitionwith a delay,Fragmentwill post a runnable on a Handler with the specified delay as the delay. This runnable is not removed when callingstartPostponedEnterTransitionhowever which means that if the fragment is destroyed it will be retained until the runnable finally runs.You can easily verify this with LeakCanary by having two fragments where in the first fragment you call
postponeEnterTransition(1, TimeUnit.HOURS)and then you navigate to the other fragment. When leak canary runs you can see that the initial fragment is retained.The solution should be to remove the runnable when calling
startPostponedEnterTransition. This might not be feasible in the case where you callpostponeEnterTransitionbefore the fragment is attached but it feels OK to ignore that case for now.