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

Commit 79998988 authored by Anton Potapov's avatar Anton Potapov
Browse files

Inject repeatWhenAttached CoroutineScope to prevent leaking

Before this CL @VolumeDialog CoroutineScope had slightly different
lifecycle rather than the scopes used in the ViewBinders. This along
side View#viewModel (which recreates the ViewModel when the view is
reattached) could lead to the CoroutinesScope used inside of the
ViewModel for stateIn to leak when the ViewModel is recreated insider the
ViewBinder.

Now @VolumeDialog is the same CoroutineScope and it's safe to inject it
whereever in the @VolumeDialogScope

Flag: com.android.systemui.volume_redesign
Bug: 369994101
Test: manual on the tablet. VolumeDialog appears after physical button
press

Change-Id: I683f4690eca40b76e37be83f4384872562299ee4
parent ac2a1f25
Loading
Loading
Loading
Loading
+14 −3
Original line number Diff line number Diff line
@@ -21,20 +21,24 @@ import android.content.Context
import android.graphics.PixelFormat
import android.os.Bundle
import android.view.MotionEvent
import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
import com.android.app.tracing.coroutines.coroutineScopeTraced
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.lifecycle.repeatWhenAttached
import com.android.systemui.res.R
import com.android.systemui.volume.Events
import com.android.systemui.volume.dialog.dagger.VolumeDialogComponent
import com.android.systemui.volume.dialog.domain.interactor.VolumeDialogVisibilityInteractor
import com.android.systemui.volume.dialog.ui.binder.VolumeDialogViewBinder
import javax.inject.Inject
import kotlinx.coroutines.awaitCancellation

class VolumeDialog
@Inject
constructor(
    @Application context: Context,
    private val viewBinder: VolumeDialogViewBinder,
    private val componentFactory: VolumeDialogComponent.Factory,
    private val visibilityInteractor: VolumeDialogVisibilityInteractor,
) : Dialog(context, R.style.Theme_SystemUI_Dialog_Volume) {

@@ -64,7 +68,14 @@ constructor(
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.volume_dialog)
        viewBinder.bind(this)
        requireViewById<View>(R.id.volume_dialog_root).repeatWhenAttached {
            coroutineScopeTraced("[Volume]dialog") {
                val component = componentFactory.create(this)
                with(component.volumeDialogViewBinder()) { bind(this@VolumeDialog) }

                awaitCancellation()
            }
        }
    }

    /**
+2 −2
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package com.android.systemui.volume.dialog

import com.android.app.tracing.coroutines.coroutineScopeTraced
import com.android.app.tracing.coroutines.launchTraced as launch
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.plugins.VolumeDialog
@@ -23,7 +24,6 @@ import com.android.systemui.volume.dialog.dagger.VolumeDialogPluginComponent
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.coroutineScope

class VolumeDialogPlugin
@Inject
@@ -38,7 +38,7 @@ constructor(
    override fun init(windowType: Int, callback: VolumeDialog.Callback?) {
        job =
            applicationCoroutineScope.launch {
                coroutineScope {
                coroutineScopeTraced("[Volume]plugin") {
                    pluginComponent =
                        volumeDialogPluginComponentFactory.create(this).also {
                            it.viewModel().launchVolumeDialog()
+11 −10
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import com.android.systemui.volume.dialog.dagger.module.VolumeDialogModule
import com.android.systemui.volume.dialog.dagger.scope.VolumeDialog
import com.android.systemui.volume.dialog.dagger.scope.VolumeDialogScope
import com.android.systemui.volume.dialog.sliders.dagger.VolumeDialogSliderComponent
import com.android.systemui.volume.dialog.ui.binder.VolumeDialogViewBinder
import dagger.BindsInstance
import dagger.Subcomponent
import kotlinx.coroutines.CoroutineScope
@@ -32,21 +33,21 @@ import kotlinx.coroutines.CoroutineScope
@Subcomponent(modules = [VolumeDialogModule::class])
interface VolumeDialogComponent {

    /**
     * Provides a coroutine scope to use inside [VolumeDialogScope].
     * [com.android.systemui.volume.dialog.VolumeDialogPlugin] manages the lifecycle of this scope.
     * It's cancelled when the dialog is disposed. This helps to free occupied resources when volume
     * dialog is not shown.
     */
    @VolumeDialog fun coroutineScope(): CoroutineScope

    @VolumeDialogScope fun volumeDialog(): com.android.systemui.volume.dialog.VolumeDialog
    fun volumeDialogViewBinder(): VolumeDialogViewBinder

    fun sliderComponentFactory(): VolumeDialogSliderComponent.Factory

    @Subcomponent.Factory
    interface Factory {

        fun create(@BindsInstance @VolumeDialog scope: CoroutineScope): VolumeDialogComponent
        fun create(
            /**
             * Provides a coroutine scope to use inside [VolumeDialogScope].
             * [com.android.systemui.volume.dialog.VolumeDialogPlugin] manages the lifecycle of this
             * scope. It's cancelled when the dialog is disposed. This helps to free occupied
             * resources when volume dialog is not shown.
             */
            @BindsInstance @VolumeDialog scope: CoroutineScope
        ): VolumeDialogComponent
    }
}
+4 −15
Original line number Diff line number Diff line
@@ -18,9 +18,6 @@ package com.android.systemui.volume.dialog.sliders.ui

import android.annotation.SuppressLint
import android.view.View
import com.android.systemui.lifecycle.WindowLifecycleState
import com.android.systemui.lifecycle.repeatWhenAttached
import com.android.systemui.lifecycle.viewModel
import com.android.systemui.res.R
import com.android.systemui.volume.dialog.sliders.dagger.VolumeDialogSliderScope
import com.android.systemui.volume.dialog.sliders.ui.viewmodel.VolumeDialogSliderTouchesViewModel
@@ -30,17 +27,11 @@ import javax.inject.Inject
@VolumeDialogSliderScope
class VolumeDialogSliderTouchesViewBinder
@Inject
constructor(private val viewModelFactory: VolumeDialogSliderTouchesViewModel.Factory) {
constructor(private val viewModel: VolumeDialogSliderTouchesViewModel) {

    @SuppressLint("ClickableViewAccessibility")
    fun bind(view: View) {
        with(view.requireViewById<Slider>(R.id.volume_dialog_slider)) {
            repeatWhenAttached {
                viewModel(
                    traceName = "VolumeDialogSliderTouchesViewBinder",
                    minWindowLifecycleState = WindowLifecycleState.ATTACHED,
                    factory = { viewModelFactory.create() },
                ) { viewModel ->
            setOnTouchListener { _, event ->
                viewModel.onTouchEvent(event)
                false
@@ -48,5 +39,3 @@ constructor(private val viewModelFactory: VolumeDialogSliderTouchesViewModel.Fac
        }
    }
}
    }
}
+10 −25
Original line number Diff line number Diff line
@@ -20,9 +20,6 @@ import android.animation.Animator
import android.animation.ObjectAnimator
import android.view.View
import android.view.animation.DecelerateInterpolator
import com.android.systemui.lifecycle.WindowLifecycleState
import com.android.systemui.lifecycle.repeatWhenAttached
import com.android.systemui.lifecycle.viewModel
import com.android.systemui.res.R
import com.android.systemui.volume.dialog.shared.model.VolumeDialogStreamModel
import com.android.systemui.volume.dialog.sliders.dagger.VolumeDialogSliderScope
@@ -33,7 +30,7 @@ import com.google.android.material.slider.LabelFormatter
import com.google.android.material.slider.Slider
import javax.inject.Inject
import kotlin.math.roundToInt
import kotlinx.coroutines.awaitCancellation
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

@@ -43,32 +40,20 @@ private const val PROGRESS_CHANGE_ANIMATION_DURATION_MS = 80L
class VolumeDialogSliderViewBinder
@Inject
constructor(
    private val viewModelFactory: VolumeDialogSliderViewModel.Factory,
    private val viewModel: VolumeDialogSliderViewModel,
    private val jankListenerFactory: JankListenerFactory,
) {

    fun bind(view: View) {
        with(view) {
    fun CoroutineScope.bind(view: View) {
        val sliderView: Slider =
                requireViewById<Slider>(R.id.volume_dialog_slider).apply {
            view.requireViewById<Slider>(R.id.volume_dialog_slider).apply {
                labelBehavior = LabelFormatter.LABEL_GONE
            }
            repeatWhenAttached {
                viewModel(
                    traceName = "VolumeDialogSliderViewBinder",
                    minWindowLifecycleState = WindowLifecycleState.ATTACHED,
                    factory = { viewModelFactory.create() },
                ) { viewModel ->
        sliderView.addOnChangeListener { _, value, fromUser ->
            viewModel.setStreamVolume(value.roundToInt(), fromUser)
        }

        viewModel.model.onEach { it.bindToSlider(sliderView) }.launchIn(this)

                    awaitCancellation()
                }
            }
        }
    }

    private suspend fun VolumeDialogStreamModel.bindToSlider(slider: Slider) {
Loading