Make it easy to register for activity results from a Composable [172690553]

Fixed

Feature Request

Status Update

No update yet.

Description

il...@google.com

created issue #1

Nov 6, 2020 11:36PM

Jetpack Compose release version: 1.0.0-alpha06

The Activity Result API and specifically the ActivityResultRegistry APIs are currently much easier to work with from activities and fragments as they implement ActivityResultCaller and provide an easy to use registerForActivityResult() API.

Something similar should be directly available for Composables. I don't think it is important to specifically implement ActivityResultCaller, but perhaps just providing a Compose specific version of registerForActivityResult():

@Composable
fun <I, O> registerForActivityResult(
    contract: ActivityResultContract<I, O>,
    onResult: (O) -> Unit
) : ActivityResultLauncher<I>

As demonstrated in this StackOverflow answer. This would handle the appropriate register and unregister calls given the lifetime of the composable.

Comments

lo...@gmail.com <lo...@gmail.com> #2Jan 31, 2021 06:42AM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

il...@google.com <il...@google.com> #3Jan 31, 2021 06:57AM

yea i'll take it.

ez...@gmail.com <ez...@gmail.com> #4Jan 31, 2021 07:36AM

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.

lo...@gmail.com <lo...@gmail.com> #5Jan 31, 2021 08:53AM

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()
}
}

il...@google.com <il...@google.com> #6Feb 1, 2021 01:10AM

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)

ap...@google.com <ap...@google.com> #7Feb 4, 2021 07:36PM

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> #8Feb 11, 2021 12:14AM

Marked as fixed.

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

no...@gmail.com <no...@gmail.com> #9Mar 18, 2021 10:11AM

After migrating from the old activity.registerForActivityResult to this mechanism, I received this error from down the call stack:

java.lang.IllegalArgumentException: Can only use lower 16 bits for requestCode

If someone else has this problem, it can be fixed by adding the following dependency, as suggested in this SO answer:

implementation 'androidx.fragment:fragment:1.3.0'

il...@google.com <il...@google.com> #10Mar 18, 2021 03:57PM

Re #9 - this is also specifically called out in the Activity release notes:

Note: when upgrading to Activity 1.2.0, you must upgrade to Fragment 1.3.0 to fix issues with invalid request codes and ensure that permission requests are delivered correctly. The InvalidFragmentVersionForActivityResult Lint check will warn you if you do not upgrade to an appropriate Fragment version.