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

Commit 6deed656 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

[Flexiglass] Fix QS AndroidView 

This CL fixes two things:

* After introducing collectAsStateWithLifecycle with a null default
  value for the view, this was causing the view (and a new QSImpl) to be
  created every time the shade was opened (because the lifecycle was
  started again). Instead, convert it into a StateFlow (as it should
  keep it's state beyond the composition) and use the collection without
  default.
* After moving unsquishing to be a lambda, this lambda does not change
  when the animation change (only the returned value), this was cauing
  the StateFlow<State> in QSSceneAdapterImpl to conflate those values so
  the animation was not working. Instead, make those types of State to
  not be data classes so they are not conflated.

Also, fix a leak where tiles were being re-added to a destroyed
QSPanelControllerBase because the view was momentarily attached. After
the controller is destroyed, no tiles should be added. We can't handle
that in detach (as we used to before scene container) because the scene
container may attach and detach the container AndroidView throughout its
lifetime.

Test: atest QSSceneAdapterImpl QSSceneAdapter
Test: atest QSPanelControllerBaseTest
Test: manual, expand shade and QS in regular and split shade
Flag: com.android.systemui.scene_container
Fixes: 341938306

Change-Id: Ia12f46546a7aa3dccb24e1be369895dfc9f1f67b
parent 16bde21c

packages/SystemUI/compose/features/src/com/android/systemui/qs/ui/composable/QuickSettings.kt

+20 −8
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.systemui.qs.ui.composable



import android.view.ViewGroup

import android.widget.FrameLayout

import androidx.compose.foundation.layout.Box

import androidx.compose.foundation.layout.fillMaxHeight

import androidx.compose.foundation.layout.fillMaxWidth
@@ -164,11 +166,8 @@ private fun QuickSettingsContent( 
    state: () -> QSSceneAdapter.State,

    modifier: Modifier = Modifier,

) {

    val qsView by qsSceneAdapter.qsView.collectAsStateWithLifecycle(null)

    val isCustomizing by

        qsSceneAdapter.isCustomizerShowing.collectAsStateWithLifecycle(

            qsSceneAdapter.isCustomizerShowing.value

        )

    val qsView by qsSceneAdapter.qsView.collectAsStateWithLifecycle()

    val isCustomizing by qsSceneAdapter.isCustomizerShowing.collectAsStateWithLifecycle()

    QuickSettingsTheme {

        val context = LocalContext.current
@@ -184,11 +183,24 @@ private fun QuickSettingsContent( 
            ) {

                AndroidView(

                    modifier = Modifier.fillMaxWidth(),

                    factory = { _ ->

                    factory = { context ->

                        qsSceneAdapter.setState(state())

                        view

                        FrameLayout(context).apply {

                            (view.parent as? ViewGroup)?.removeView(view)

                            addView(view)

                        }

                    },

                    // When the view changes (e.g. due to a theme change), this will be recomposed

                    // if needed and the new view will be attached to the FrameLayout here.

                    update = {

                        qsSceneAdapter.setState(state())

                        if (view.parent != it) {

                            it.removeAllViews()

                            (view.parent as? ViewGroup)?.removeView(view)

                            it.addView(view)

                        }

                    },

                    update = { qsSceneAdapter.setState(state()) }

                    onRelease = { it.removeAllViews() }

                )

            }

        }

packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java

+4 −1
Original line number Diff line number Diff line
@@ -93,6 +93,8 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 


    private final Provider<QSLongPressEffect> mLongPressEffectProvider;



    private boolean mDestroyed = false;



    @VisibleForTesting

    protected final QSPanel.OnConfigurationChangedListener mOnConfigurationChangedListener =

            new QSPanel.OnConfigurationChangedListener() {
@@ -192,7 +194,7 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 
        // will remove the attach listener. We don't need to do that, because once this object is

        // detached from the graph, it will be gc.

        mHost.removeCallback(mQSHostCallback);



        mDestroyed = true;

        for (TileRecord record : mRecords) {

            record.tile.removeCallback(record.callback);

            mView.removeTile(record);
@@ -242,6 +244,7 @@ public abstract class QSPanelControllerBase<T extends QSPanel> extends ViewContr 


    /** */

    public void setTiles(Collection<QSTile> tiles, boolean collapsedView) {

        if (mDestroyed) return;

        // TODO(b/168904199): move this logic into QSPanelController.

        if (!collapsedView && mQsTileRevealController != null) {

            mQsTileRevealController.updateRevealedTiles(tiles);

packages/SystemUI/src/com/android/systemui/qs/ui/adapter/QSSceneAdapter.kt

+21 −7
Original line number Diff line number Diff line
@@ -46,7 +46,6 @@ import kotlin.coroutines.suspendCoroutine 
import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.channels.BufferOverflow

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.MutableSharedFlow

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.SharingStarted
@@ -89,8 +88,10 @@ interface QSSceneAdapter { 
    /**

     * A view with the QS content ([QSContainerImpl]), managed by an instance of [QSImpl] tracked by

     * the interactor.

     *

     * A null value means that there is no inflated view yet. See [inflate].

     */

    val qsView: Flow<View>

    val qsView: StateFlow<View?>



    /** Sets the [MirrorController] in [QSImpl]. Set to `null` to remove. */

    fun setBrightnessMirrorController(mirrorController: MirrorController?)
@@ -141,14 +142,24 @@ interface QSSceneAdapter { 
            override val squishiness = { 1f }

        }



        /** State for appearing QQS from Lockscreen or Gone */

        data class UnsquishingQQS(override val squishiness: () -> Float) : State {

        /**

         * State for appearing QQS from Lockscreen or Gone.

         *

         * This should not be a data class, as it has a method parameter and even if it's the same

         * lambda the output value may have changed.

         */

        class UnsquishingQQS(override val squishiness: () -> Float) : State {

            override val isVisible = true

            override val expansion = 0f

        }



        /** State for appearing QS from Lockscreen or Gone, used in Split shade */

        data class UnsquishingQS(override val squishiness: () -> Float) : State {

        /**

         * State for appearing QS from Lockscreen or Gone, used in Split shade.

         *

         * This should not be a data class, as it has a method parameter and even if it's the same

         * lambda the output value may have changed.

         */

        class UnsquishingQS(override val squishiness: () -> Float) : State {

            override val isVisible = true

            override val expansion = 1f

        }
@@ -236,7 +247,10 @@ constructor( 


    private val _qsImpl: MutableStateFlow<QSImpl?> = MutableStateFlow(null)

    val qsImpl = _qsImpl.asStateFlow()

    override val qsView: Flow<View> = _qsImpl.map { it?.view }.filterNotNull()

    override val qsView: StateFlow<View?> =

        _qsImpl

            .map { it?.view }

            .stateIn(applicationScope, SharingStarted.WhileSubscribed(), _qsImpl.value?.view)



    override val qqsHeight: Int

        get() = qsImpl.value?.qqsHeight ?: 0

packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java

+12 −0
Original line number Diff line number Diff line
@@ -502,4 +502,16 @@ public class QSPanelControllerBaseTest extends SysuiTestCase { 
        verify(mQSPanel, times(2)).removeTile(any());

        verify(mQSPanel, times(2)).addTile(any());

    }



    @Test

    public void dettach_destroy_attach_tilesAreNotReadded() {

        when(mQSHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile));

        mController.setTiles();



        mController.onViewDetached();

        mController.destroy();

        mController.onViewAttached();



        assertThat(mController.mRecords).isEmpty();

    }

}

packages/SystemUI/tests/utils/src/com/android/systemui/qs/ui/adapter/FakeQSSceneAdapter.kt

+2 −2
Original line number Diff line number Diff line
@@ -19,8 +19,8 @@ package com.android.systemui.qs.ui.adapter 
import android.content.Context

import android.view.View

import com.android.systemui.settings.brightness.MirrorController

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.asStateFlow

import kotlinx.coroutines.flow.filterNotNull
@@ -41,7 +41,7 @@ class FakeQSSceneAdapter( 
    override val customizerAnimationDuration = _animationDuration.asStateFlow()



    private val _view = MutableStateFlow<View?>(null)

    override val qsView: Flow<View> = _view.filterNotNull()

    override val qsView: StateFlow<View?> = _view.asStateFlow()



    private val _state = MutableStateFlow<QSSceneAdapter.State?>(null)

    val state = _state.filterNotNull()