Restoring Fragment inside compose [312895363]

Fixed

Feature Request

Status Update

No update yet.

Description

ju...@veepee.com

created issue #1

Nov 23, 2023 02:58PM

What is the correct strategy to restoring a fragment commited inside a compose AndroidView? I would expect that something like the example below to be enough to handle the fragment state restoration in an orientation change, but when rotating the screen the fragment is no longer drawn.

                   val containerId = rememberSaveable {
                        ViewCompat.generateViewId()
                    }
                    val fragmentTag = rememberSaveable {
                        "FragmentTag$containerId"
                    }
                    AndroidView(factory = { context ->
                        FragmentContainerView(context).apply {
                            id = containerId
                        }
                    }, update = { containerView ->
                        if (containerView.getFragment<Fragment?>() == null) {
                            supportFragmentManager.commit {
                                replace(containerId, SomeFragment(), fragmentTag)
                            }
                        }
                    })

StackOverflow proposes a solution using reflection, but as that is not a public API I would like to avoid using it and instead having a proper support from androidx team.

It would be nice to have a composable version of FragmentContainerView that could handle Fragment state restoration properly.

FragmentStateRestorationInsideCompose.zip

115 KB

Download

Comments

jb...@google.com <jb...@google.com> #2Nov 27, 2023 04:12PM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

ju...@gmail.com <ju...@gmail.com> #3Nov 28, 2023 02:02PM

yea i'll take it.

ju...@gmail.com <ju...@gmail.com> #4Nov 28, 2023 02:03PM

Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.

ap...@google.com <ap...@google.com> #5Mar 27, 2024 11:07PM

just emitting same live data reproduces the issue.

@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}

ju...@veepee.com <ju...@veepee.com> #6Mar 28, 2024 10:02AM

With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)

il...@google.com <il...@google.com> #7Mar 28, 2024 04:59PM

yea sorry immediate does not fix it.
I actually have a WIP fix for it:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186

if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.

val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}

il...@google.com <il...@google.com> #8Mar 28, 2024 05:01PM

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

commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019

Fix coroutine livedata race condition

This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.

We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)

Bug: 140249349
Test: BuildLiveDataTest#raceTest_*

Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40

M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt

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

https://goto.google.com/android-sha1/af12e75e6b4110f48e44ca121466943909de8f06

ju...@veepee.com <ju...@veepee.com> #9Mar 29, 2024 07:18AM

We were always allowed to create a new Fragment and to commit a Fragment instance in FragmentManager, could you align with that as well? My abstraction returns a Fragment instance because this allows us to avoid directly depending on the gradle module that provides the Fragment and this strategy improves compilation time by making the gradle dependency graph flatter.

Message last modified on Mar 29, 2024 07:43AM

il...@google.com <il...@google.com> #10Mar 29, 2024 05:01PM

You'll want your abstraction to be implemented in terms of a FragmentFactory.

ju...@veepee.com <ju...@veepee.com> #11Apr 2, 2024 08:20AM

My abstraction does not use fragmentFactory, and don't need to. How is that related to this issue? Devs have always been able to instantiate a fragment using default fragment constructor, I am not exposing which Fragment subclass is being instantiated in the calling site or using the className as key to find which fragment to instantiate, to add a fragment to FragmentManager we just need the base class Fragment.

ba...@gmail.com <ba...@gmail.com> #12Apr 4, 2024 09:10AM

Hi. Would it be possible to update an existing fragment when the args given to it change? I've a PageFragment in my app that load an url received as argument. When I navigate using my bottom bar, a PageFragment is loaded with different url for each. Right now it won't work with AndroidFragment as the clazz is the same and it won't recompose based on arguments alone.

il...@google.com <il...@google.com> #13Apr 8, 2024 05:55PM

Re comment#12 - you can surround the AndroidFragment with key(url) to reset that subtree whenever url changes.

ba...@gmail.com <ba...@gmail.com> #14Apr 8, 2024 07:52PM

Thanks, using key(url) works like a charm. I thought this function was only available for LazyList|Column...

ba...@gmail.com <ba...@gmail.com> #15Apr 25, 2024 12:23PM

For your information we have the following crash using AndroidView:

Exception java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState at androidx.fragment.app.FragmentManager.checkStateLoss (FragmentManager.java:1) at androidx.fragment.app.FragmentManager.ensureExecReady (FragmentManager.java:1) at androidx.fragment.app.FragmentManager.execSingleAction (FragmentManager.java:1) at androidx.fragment.app.BackStackRecord.commitNow (BackStackRecord.java:1) at androidx.fragment.compose.AndroidFragmentKt$AndroidFragment$4.invoke (AndroidFragment.kt:110) at androidx.fragment.compose.AndroidFragmentKt$AndroidFragment$4.invoke (AndroidFragment.kt:3) at androidx.compose.runtime.DisposableEffectImpl.onRemembered (Effects.kt:7) at androidx.compose.runtime.CompositionImpl$RememberEventDispatcher.dispatchRememberObservers (Composition.kt:144) at androidx.compose.runtime.CompositionImpl.applyChangesInLocked (Composition.kt:1) at androidx.compose.runtime.CompositionImpl.applyChanges (Composition.kt:6) at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$1.invoke (Recomposer.kt:1) at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$1.invoke (Recomposer.kt:7) at androidx.compose.ui.platform.AndroidUiFrameClock$withFrameNanos$2$callback$1.doFrame (AndroidUiFrameClock.android.kt:11) at androidx.compose.ui.platform.AndroidUiDispatcher.performFrameDispatch (AndroidUiDispatcher.android.kt:34) at androidx.compose.ui.platform.AndroidUiDispatcher.access$performFrameDispatch (AndroidUiDispatcher.android.kt:1) at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.doFrame (AndroidUiDispatcher.android.kt:17) at android.view.Choreographer$CallbackRecord.run (Choreographer.java:1229) at android.view.Choreographer$CallbackRecord.run (Choreographer.java:1239) at android.view.Choreographer.doCallbacks (Choreographer.java:899) at android.view.Choreographer.doFrame (Choreographer.java:827) at android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:1214) at android.os.Handler.handleCallback (Handler.java:942) at android.os.Handler.dispatchMessage (Handler.java:99) at android.os.Looper.loopOnce (Looper.java:201) at android.os.Looper.loop (Looper.java:288) at android.app.ActivityThread.main (ActivityThread.java:7918) at java.lang.reflect.Method.invoke at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:936)

lo...@gmail.com <lo...@gmail.com> #16Apr 25, 2024 03:18PM

Will there also be an overload in which we can directly pass in an already created fragment instance? Like:

fun <T : Fragment> AndroidFragment(
    fragment: T,
    modifier: Modifier = Modifier,
    fragmentState: FragmentState = rememberFragmentState(),
    arguments: Bundle = Bundle.EMPTY,
    onUpdate: (T) -> Unit = { },
) { ... }

il...@google.com <il...@google.com> #17Apr 25, 2024 07:59PM

Re comment#15 - please file a separate issue so we can track that crash.

il...@google.com <il...@google.com> #18Apr 25, 2024 08:00PM

Marked as fixed, reassigned to jb...@google.com.

Re comment#16 - no, as explained in comment#7.