Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 72191899 authored by Ale Nijamkin's avatar Ale Nijamkin
Browse files

Makes rememberViewModel's coroutine context configurable

There are two ways to control it:
- WithConfiguredRememberViewModels wraps a composable hierarchy and
  makes every rememberViewModel within it use the provided
CoroutineContext
- rememberViewModel takes a coroutineContext parameter. If this is
  passed in, in addition to the one from
WithConfiguredRememberViewModels, the passed-in one wins

Bug: 436547539
Test: unit tests added, also tested manually with the followup CL,
making sure that activated view-models are reporting background thread
names instead of "main" when they get activated
Flag: EXEMPT no production changes in this CL

Change-Id: I1edd2dd261bf1dd3713711f1be242716b6687efe
parent 15148029
Loading
Loading
Loading
Loading
+7 −1
Original line number Diff line number Diff line
@@ -22,7 +22,10 @@ import androidx.compose.runtime.NonRestartableComposable
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.compose.LocalLifecycleOwner
import androidx.lifecycle.repeatOnLifecycle
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch

// This deprecated-error function shadows the varargs overload so that the varargs version
// is not used without key parameters.
@@ -51,11 +54,14 @@ private const val LaunchedEffectNoParamError =
fun LaunchedEffectWithLifecycle(
    key1: Any?,
    lifecycle: Lifecycle = LocalLifecycleOwner.current.lifecycle,
    coroutineContext: CoroutineContext = EmptyCoroutineContext,
    minActiveState: Lifecycle.State = Lifecycle.State.STARTED,
    block: suspend CoroutineScope.() -> Unit,
) {
    LaunchedEffect(key1, lifecycle, minActiveState) {
        lifecycle.repeatOnLifecycle(minActiveState, block)
        lifecycle.repeatOnLifecycle(minActiveState) {
            launch(context = this.coroutineContext + coroutineContext) { block() }
        }
    }
}

+74 −4
Original line number Diff line number Diff line
@@ -14,6 +14,9 @@
 * limitations under the License.
 */

@file:Suppress("UNUSED_VARIABLE")
@file:OptIn(ExperimentalCoroutinesApi::class)

package com.android.systemui.lifecycle

import android.view.View
@@ -32,6 +35,9 @@ import com.android.systemui.ui.viewmodel.FakeSysUiViewModel
import com.android.systemui.util.Assert
import com.google.common.truth.Truth.assertThat
import com.google.common.truth.Truth.assertWithMessage
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.coroutineContext
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.awaitCancellation
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.runCurrent
@@ -59,9 +65,8 @@ class SysUiViewModelTest : SysuiTestCase() {
            val keepAlive by keepAliveMutable
            if (keepAlive) {
                // Need to explicitly state the type to avoid a weird issue where the factory seems
                // to
                // return Unit instead of FakeSysUiViewModel. It might be an issue with the compose
                // compiler.
                // to return Unit instead of FakeSysUiViewModel. It might be an issue with the
                // compose compiler.
                val unused: FakeSysUiViewModel =
                    rememberViewModel("test") {
                        FakeSysUiViewModel(
@@ -174,7 +179,6 @@ class SysUiViewModelTest : SysuiTestCase() {
            )
            .forEachIndexed { index, lifecycleState ->
                composeRule.runOnUiThread { lifecycleOwner.lifecycle.currentState = lifecycleState }
                composeRule.waitForIdle()
                val expectedIsActive = lifecycleState.isAtLeast(minActiveState)
                assertWithMessage(
                        "isActive=$isActive but expected to be $expectedIsActive when" +
@@ -210,6 +214,59 @@ class SysUiViewModelTest : SysuiTestCase() {
        assertThat(isActive).isFalse()
    }

    @Test
    fun rememberActivated_withCoroutineContext() {
        val flag = FlagElement("flag")
        var viewModel: FakeViewModel? = null

        composeRule.setContent {
            viewModel =
                rememberViewModel(
                    traceName = "test",
                    coroutineContext = flag,
                    factory = { FakeViewModel() },
                )
        }

        assertThat(viewModel?.lastActivationCoroutineContext?.get(flag.key)).isSameInstanceAs(flag)
    }

    @Test
    fun rememberActivated_withConfiguredCompositionLocal() {
        val flag = FlagElement("flag")
        var viewModel: FakeViewModel? = null

        composeRule.setContent {
            WithConfiguredRememberViewModels(coroutineContext = flag) {
                viewModel = rememberViewModel(traceName = "test", factory = { FakeViewModel() })
            }
        }

        assertThat(viewModel?.lastActivationCoroutineContext?.get(flag.key)).isSameInstanceAs(flag)
    }

    @Test
    fun rememberActivated_withConfiguredCompositionLocal_andCoroutineContextOverride() {
        val configuredFlag = FlagElement("configured")
        val innerOverrideFlag = FlagElement("innerOverride")
        var viewModel: FakeViewModel? = null

        composeRule.setContent {
            WithConfiguredRememberViewModels(coroutineContext = configuredFlag) {
                viewModel =
                    rememberViewModel(
                        traceName = "test",
                        coroutineContext = innerOverrideFlag,
                        factory = { FakeViewModel() },
                    )
            }
        }

        assertThat(viewModel?.lastActivationCoroutineContext?.get(configuredFlag.key)).isNull()
        assertThat(viewModel?.lastActivationCoroutineContext?.get(innerOverrideFlag.key))
            .isSameInstanceAs(innerOverrideFlag)
    }

    @Test
    fun viewModel_viewBinder() = runTest {
        Assert.setTestThread(Thread.currentThread())
@@ -242,9 +299,16 @@ class SysUiViewModelTest : SysuiTestCase() {

private class FakeViewModel : ExclusiveActivatable() {
    var isActivated = false
    /**
     * The [CoroutineContext] used for the most recent activation of this [FakeViewModel]; will be
     * `null` before the first activation and will be updated for every subsequent activation.
     */
    var lastActivationCoroutineContext: CoroutineContext? = null
        private set

    override suspend fun onActivated(): Nothing {
        isActivated = true
        lastActivationCoroutineContext = coroutineContext
        try {
            awaitCancellation()
        } finally {
@@ -252,3 +316,9 @@ private class FakeViewModel : ExclusiveActivatable() {
        }
    }
}

private class FlagElement(private val name: String) : CoroutineContext.Element {
    override val key = object : CoroutineContext.Key<FlagElement> {}

    override fun toString(): String = name
}
+36 −2
Original line number Diff line number Diff line
@@ -18,11 +18,16 @@ package com.android.systemui.lifecycle

import android.view.View
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.remember
import androidx.compose.runtime.staticCompositionLocalOf
import androidx.lifecycle.Lifecycle
import com.android.app.tracing.TraceUtils
import com.android.app.tracing.coroutines.launchTraced as launch
import com.android.app.tracing.coroutines.traceCoroutine
import com.android.compose.lifecycle.LaunchedEffectWithLifecycle
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineScope

/**
@@ -39,23 +44,49 @@ import kotlinx.coroutines.CoroutineScope
 * The remembered view-model is activated every time the [minActiveState] is reached and deactivated
 * each time the lifecycle state falls "below" the [minActiveState]. This can be used to have more
 * granular control over when exactly a view-model becomes active.
 *
 * Note that, by default, `rememberViewModel` will activate its view-model in the [CoroutineContext]
 * from which it was called. To configure this, either pass a [coroutineContext] to this method or
 * use [WithConfiguredRememberViewModels] to bulk-configure all usages of `rememberViewModel`s
 * within the composable hierarchy. If you do both, the provided [coroutineContext] takes precedence
 * over the [WithConfiguredRememberViewModels] one.
 */
@Composable
fun <T> rememberViewModel(
    traceName: String,
    minActiveState: Lifecycle.State = Lifecycle.State.STARTED,
    coroutineContext: CoroutineContext = LocalCoroutineContext.current,
    key: Any = Unit,
    factory: () -> T,
): T {
    val instance = remember(key) { factory() }
    if (instance is Activatable) {
        LaunchedEffectWithLifecycle(key1 = instance, minActiveState = minActiveState) {
        LaunchedEffectWithLifecycle(
            key1 = instance,
            coroutineContext = coroutineContext,
            minActiveState = minActiveState,
        ) {
            TraceUtils.traceAsync("SystemUI.rememberViewModel", traceName) {
                traceCoroutine(traceName) { instance.activate() }
            }
        }
    }
    return instance
}

/**
 * Configures all usages of [rememberViewModel] in this composition to use the provided
 * [coroutineContext] to run their activations. Individual calls to [rememberViewModel] can still
 * override this behavior by passing a different [CoroutineContext].
 */
@Composable
fun WithConfiguredRememberViewModels(
    coroutineContext: CoroutineContext = EmptyCoroutineContext,
    block: @Composable () -> Unit,
) {
    CompositionLocalProvider(LocalCoroutineContext provides coroutineContext, block)
}

/**
 * Invokes [block] in a new coroutine with a new view-model that is automatically activated whenever
 * `this` [View]'s Window's [WindowLifecycleState] is at least at [minWindowLifecycleState], and is
@@ -79,3 +110,6 @@ suspend fun <T> View.viewModel(
        }
        block(instance)
    }

private val LocalCoroutineContext =
    staticCompositionLocalOf<CoroutineContext> { EmptyCoroutineContext }