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

Commit ed980e67 authored by lyn's avatar lyn
Browse files

Fix bundle icon reshow after shade close 

The fix decouples logical state from the UI lifecycle.

Add new observer to BundleInteractor that
watches for shade collapse then updates [lastCollapseTime]

Wrap updates for [state] and [lastCollapseTime] in BundleRowScope
to ensure update completion even if view detaches.

Bind CoroutineScope in BundleRowComponent

Fixes: 430203178
Test: BundleInteractorTest
Flag: com.android.systemui.notification_bundle_ui

Change-Id: I2a4f673b6f3fe9c605b226bd663c32d4223dd9a0
parent 1ffe4a73

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/domain/interactor/BundleInteractorTest.kt

+97 −2
Original line number Original line Diff line number Diff line
@@ -33,6 +33,8 @@ import com.android.systemui.controls.dagger.ControlsComponentTest.Companion.eq 
import com.android.systemui.controls.ui.ControlActionCoordinatorImplTest.Companion.any

import com.android.systemui.controls.ui.ControlActionCoordinatorImplTest.Companion.any

import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.testScope

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.shade.domain.interactor.ShadeInteractor

import com.android.systemui.shade.domain.interactor.shadeInteractor

import com.android.systemui.statusbar.notification.row.data.model.AppData

import com.android.systemui.statusbar.notification.row.data.model.AppData

import com.android.systemui.statusbar.notification.row.data.repository.BundleRepository

import com.android.systemui.statusbar.notification.row.data.repository.BundleRepository

import com.android.systemui.statusbar.notification.row.data.repository.testBundleRepository

import com.android.systemui.statusbar.notification.row.data.repository.testBundleRepository
@@ -41,9 +43,11 @@ import com.android.systemui.statusbar.notification.row.icon.appIconProvider 
import com.android.systemui.statusbar.notification.row.icon.mockAppIconProvider

import com.android.systemui.statusbar.notification.row.icon.mockAppIconProvider

import com.android.systemui.statusbar.notification.shared.NotificationBundleUi

import com.android.systemui.statusbar.notification.shared.NotificationBundleUi

import com.android.systemui.testKosmos

import com.android.systemui.testKosmos

import com.android.systemui.util.time.FakeSystemClock

import com.android.systemui.util.time.systemClock

import com.google.common.truth.Truth.assertThat

import com.google.common.truth.Truth.assertThat

import kotlin.test.Test

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import kotlinx.coroutines.test.runTest
@@ -58,6 +62,7 @@ import org.mockito.junit.MockitoRule 
import org.mockito.kotlin.mock

import org.mockito.kotlin.mock

import org.mockito.kotlin.whenever

import org.mockito.kotlin.whenever

import platform.test.motion.compose.runMonotonicClockTest

import platform.test.motion.compose.runMonotonicClockTest

import kotlin.test.Test





@OptIn(ExperimentalCoroutinesApi::class)

@OptIn(ExperimentalCoroutinesApi::class)

@SmallTest

@SmallTest
@@ -69,9 +74,11 @@ class BundleInteractorTest : SysuiTestCase() { 




    private val kosmos = testKosmos()

    private val kosmos = testKosmos()

    private val testScope = kosmos.testScope

    private val testScope = kosmos.testScope

    private val fakeSystemClock = FakeSystemClock()





    private val testBundleRepository: BundleRepository = kosmos.testBundleRepository

    private val testBundleRepository: BundleRepository = kosmos.testBundleRepository



    private val mockShadeInteractor = mock<ShadeInteractor>()

    private val isShadeFullyCollapsedFlow = MutableStateFlow(false)

    private lateinit var underTest: BundleInteractor

    private lateinit var underTest: BundleInteractor





    private val drawable1: Drawable = ColorDrawable(Color.RED)

    private val drawable1: Drawable = ColorDrawable(Color.RED)
@@ -81,6 +88,9 @@ class BundleInteractorTest : SysuiTestCase() { 
    @Before

    @Before

    fun setUp() {

    fun setUp() {

        kosmos.appIconProvider = kosmos.mockAppIconProvider

        kosmos.appIconProvider = kosmos.mockAppIconProvider

        whenever(mockShadeInteractor.isShadeFullyCollapsed).thenReturn(isShadeFullyCollapsedFlow)

        kosmos.shadeInteractor = mockShadeInteractor

        kosmos.systemClock = fakeSystemClock

        underTest = kosmos.bundleInteractor

        underTest = kosmos.bundleInteractor

    }

    }
@@ -293,4 +303,89 @@ class BundleInteractorTest : SysuiTestCase() { 
        // Assert

        // Assert

        assertThat(underTest.state?.currentScene).isEqualTo(BundleHeader.Scenes.Collapsed)

        assertThat(underTest.state?.currentScene).isEqualTo(BundleHeader.Scenes.Collapsed)

    }

    }



    @Test

    fun setTargetScene_whenCollapsing_updatesLastCollapseTime() = runMonotonicClockTest {

        // Arrange

        val testTime = 20000L

        fakeSystemClock.setUptimeMillis(testTime)

        underTest.state =

            MutableSceneTransitionLayoutState(

                initialScene = BundleHeader.Scenes.Expanded,

                motionScheme = MotionScheme.standard(),

            )

        underTest.composeScope = this



        // Act

        underTest.setTargetScene(BundleHeader.Scenes.Collapsed)

        testScope.runCurrent()



        // Assert

        assertThat(testBundleRepository.lastCollapseTime).isEqualTo(testTime)

    }



    @Test

    fun setTargetScene_whenExpanding_doesNotUpdateLastCollapseTime() = runMonotonicClockTest {

        // Arrange

        val initialTime = 11000L

        testBundleRepository.lastCollapseTime = initialTime

        fakeSystemClock.setUptimeMillis(20000L)

        underTest.state =

            MutableSceneTransitionLayoutState(

                initialScene = BundleHeader.Scenes.Collapsed,

                motionScheme = MotionScheme.standard(),

            )

        underTest.composeScope = this



        // Act

        underTest.setTargetScene(BundleHeader.Scenes.Expanded)

        testScope.runCurrent()



        // Assert

        assertThat(testBundleRepository.lastCollapseTime).isEqualTo(initialTime)

    }



    @Test

    fun observeShadeState_whenShadeCollapsesOnExpandedBundle_updatesState() =

        testScope.runTest {

            // Arrange

            val shadeState =

                MutableSceneTransitionLayoutState(

                    initialScene = BundleHeader.Scenes.Expanded,

                    motionScheme = MotionScheme.standard(),

                )

            testBundleRepository.state = shadeState

            val testTime = 20000L

            fakeSystemClock.setUptimeMillis(testTime)



            // Act

            isShadeFullyCollapsedFlow.value = true

            runCurrent()



            // Assert

            assertThat(testBundleRepository.lastCollapseTime).isEqualTo(testTime)

            assertThat(shadeState.currentScene).isEqualTo(BundleHeader.Scenes.Collapsed)

        }



    @Test

    fun observeShadeState_whenShadeCollapsesOnCollapsedBundle_doesNothing() =

        testScope.runTest {

            // Arrange

            val shadeState =

                MutableSceneTransitionLayoutState(

                    initialScene = BundleHeader.Scenes.Collapsed,

                    motionScheme = MotionScheme.standard(),

                )

            testBundleRepository.state = shadeState

            val initialTime = 11000L

            testBundleRepository.lastCollapseTime = initialTime

            fakeSystemClock.setUptimeMillis(20000L)



            // Act

            isShadeFullyCollapsedFlow.value = true

            runCurrent()



            // Assert

            assertThat(testBundleRepository.lastCollapseTime).isEqualTo(initialTime)

        }

}
 
}

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/ui/viewmodel/BundleHeaderViewModelTest.kt

+25 −18
Original line number Original line Diff line number Diff line
@@ -17,6 +17,8 @@ 
package com.android.systemui.statusbar.notification.row.ui.viewmodel

package com.android.systemui.statusbar.notification.row.ui.viewmodel





import android.platform.test.annotations.EnableFlags

import android.platform.test.annotations.EnableFlags

import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi

import androidx.compose.material3.MotionScheme

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest

import androidx.test.filters.SmallTest

import com.android.compose.animation.scene.MutableSceneTransitionLayoutState

import com.android.compose.animation.scene.MutableSceneTransitionLayoutState
@@ -24,9 +26,10 @@ import com.android.systemui.SysuiTestCase 
import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.testScope

import com.android.systemui.lifecycle.activateIn

import com.android.systemui.lifecycle.activateIn

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.statusbar.notification.row.domain.interactor.BundleInteractor

import com.android.systemui.statusbar.notification.shared.NotificationBundleUi

import com.android.systemui.statusbar.notification.shared.NotificationBundleUi

import com.android.systemui.testKosmos

import com.android.systemui.testKosmos

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.flow.MutableStateFlow

import org.junit.Before

import org.junit.Before

import org.junit.Rule

import org.junit.Rule

import org.junit.Test

import org.junit.Test
@@ -39,52 +42,56 @@ import org.mockito.kotlin.whenever 




@SmallTest

@SmallTest

@RunWith(AndroidJUnit4::class)

@RunWith(AndroidJUnit4::class)

@OptIn(ExperimentalMaterial3ExpressiveApi::class)

@EnableFlags(NotificationBundleUi.FLAG_NAME)

@EnableFlags(NotificationBundleUi.FLAG_NAME)

class BundleHeaderViewModelTest : SysuiTestCase() {

class BundleHeaderViewModelTest : SysuiTestCase() {





    @get:Rule val rule: MockitoRule = MockitoJUnit.rule()

    @get:Rule val rule: MockitoRule = MockitoJUnit.rule()



    private val kosmos = testKosmos()

    private val kosmos = testKosmos()





    @Mock lateinit var mockSceneTransitionLayoutState: MutableSceneTransitionLayoutState

    @Mock private lateinit var mockBundleInteractor: BundleInteractor

    @Mock lateinit var mockComposeScope: CoroutineScope



    private lateinit var underTest: BundleHeaderViewModel

    private lateinit var underTest: BundleHeaderViewModel





    @Before

    @Before

    fun setup() {

    fun setup() {

        underTest = kosmos.bundleHeaderViewModelFactory.create()

        whenever(mockBundleInteractor.previewIcons).thenReturn(MutableStateFlow(emptyList()))

        underTest = BundleHeaderViewModel(mockBundleInteractor)

        underTest.activateIn(kosmos.testScope)

        underTest.activateIn(kosmos.testScope)



        underTest.state = mockSceneTransitionLayoutState

        underTest.composeScope = mockComposeScope

    }

    }





    @Test

    @Test

    fun onHeaderClicked_toggles_expansion_state_to_expanded() {

    fun onHeaderClicked_toggles_expansion_state_to_expanded() {

        // Arrange

        // Arrange

        whenever(mockSceneTransitionLayoutState.currentScene)

        val state =

            .thenReturn(BundleHeader.Scenes.Collapsed)

            MutableSceneTransitionLayoutState(

                initialScene = BundleHeader.Scenes.Collapsed,

                motionScheme = MotionScheme.standard(),

            )

        underTest.state = state

        whenever(mockBundleInteractor.state).thenReturn(state)





        // Act

        // Act

        underTest.onHeaderClicked()

        underTest.onHeaderClicked()





        // Assert

        // Assert

        verify(mockSceneTransitionLayoutState)

        verify(mockBundleInteractor).setTargetScene(BundleHeader.Scenes.Expanded)

            .setTargetScene(BundleHeader.Scenes.Expanded, mockComposeScope)

    }

    }





    @Test

    @Test

    fun onHeaderClicked_toggles_expansion_state_to_collapsed() {

    fun onHeaderClicked_toggles_expansion_state_to_collapsed() {

        // Arrange

        // Arrange

        whenever(mockSceneTransitionLayoutState.currentScene)

        val state =

            .thenReturn(BundleHeader.Scenes.Expanded)

            MutableSceneTransitionLayoutState(

                initialScene = BundleHeader.Scenes.Expanded,

                motionScheme = MotionScheme.standard(),

            )

        underTest.state = state

        whenever(mockBundleInteractor.state).thenReturn(state)





        // Act

        // Act

        underTest.onHeaderClicked()

        underTest.onHeaderClicked()





        // Assert

        // Assert

        verify(mockSceneTransitionLayoutState)

        verify(mockBundleInteractor).setTargetScene(BundleHeader.Scenes.Collapsed)

            .setTargetScene(BundleHeader.Scenes.Collapsed, mockComposeScope)

    }

    }

}
 
}
 
 No newline at end of file

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/BundleBarn.kt

+23 −1
Original line number Original line Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.systemui.statusbar.notification.collection.render

package com.android.systemui.statusbar.notification.collection.render





import android.content.Context

import android.content.Context

import android.view.View

import android.view.ViewGroup

import android.view.ViewGroup

import androidx.compose.runtime.Composable

import androidx.compose.runtime.Composable

import androidx.compose.runtime.DisposableEffect

import androidx.compose.runtime.DisposableEffect
@@ -25,6 +26,7 @@ import androidx.lifecycle.Lifecycle 
import androidx.lifecycle.repeatOnLifecycle

import androidx.lifecycle.repeatOnLifecycle

import com.android.compose.theme.PlatformTheme

import com.android.compose.theme.PlatformTheme

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Main

import com.android.systemui.initOnBackPressedDispatcherOwner

import com.android.systemui.initOnBackPressedDispatcherOwner

import com.android.systemui.lifecycle.rememberViewModel

import com.android.systemui.lifecycle.rememberViewModel

import com.android.systemui.lifecycle.repeatWhenAttached

import com.android.systemui.lifecycle.repeatWhenAttached
@@ -47,6 +49,10 @@ import com.android.systemui.statusbar.notification.row.ui.viewmodel.BundleHeader 
import com.android.systemui.statusbar.notification.stack.NotificationListContainer

import com.android.systemui.statusbar.notification.stack.NotificationListContainer

import com.android.systemui.util.time.SystemClock

import com.android.systemui.util.time.SystemClock

import dagger.Lazy

import dagger.Lazy

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.SupervisorJob

import kotlinx.coroutines.cancel

import javax.inject.Inject

import javax.inject.Inject

import javax.inject.Provider

import javax.inject.Provider
@@ -63,6 +69,7 @@ constructor( 
    val systemClock: SystemClock,

    val systemClock: SystemClock,

    val logger: RowInflaterTaskLogger,

    val logger: RowInflaterTaskLogger,

    val userTracker: UserTracker,

    val userTracker: UserTracker,

    @Main private val mainDispatcher: CoroutineDispatcher,

    private val presenterLazy: Lazy<NotificationPresenter?>? = null,

    private val presenterLazy: Lazy<NotificationPresenter?>? = null,

    private val iconManager: IconManager,

    private val iconManager: IconManager,

) : PipelineDumpable {

) : PipelineDumpable {
@@ -114,8 +121,23 @@ constructor( 
    }

    }





    private fun initBundleHeaderView(bundleEntry: BundleEntry, row: ExpandableNotificationRow) {

    private fun initBundleHeaderView(bundleEntry: BundleEntry, row: ExpandableNotificationRow) {

        val scope = CoroutineScope(SupervisorJob() + mainDispatcher)

        row.addOnAttachStateChangeListener(

            object : View.OnAttachStateChangeListener {

                override fun onViewAttachedToWindow(v: View) {}

                override fun onViewDetachedFromWindow(v: View) {

                    scope.cancel()

                    row.removeOnAttachStateChangeListener(this)

                }

            }

        )



        val bundleRowComponent =

        val bundleRowComponent =

            bundleRowComponentBuilder.bindBundleRepository(bundleEntry.bundleRepository).build()

            bundleRowComponentBuilder

                .bindBundleRepository(bundleEntry.bundleRepository)

                .bindScope(scope)

                .build()



        val headerComposeView = ComposeView(context)

        val headerComposeView = ComposeView(context)

        row.setBundleHeaderView(headerComposeView)

        row.setBundleHeaderView(headerComposeView)

        headerComposeView.repeatWhenAttached {

        headerComposeView.repeatWhenAttached {

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/BundleRowComponent.kt

+3 −0
Original line number Original line Diff line number Diff line
@@ -20,6 +20,7 @@ import com.android.systemui.statusbar.notification.row.data.repository.BundleRep 
import com.android.systemui.statusbar.notification.row.ui.viewmodel.BundleHeaderViewModel

import com.android.systemui.statusbar.notification.row.ui.viewmodel.BundleHeaderViewModel

import dagger.BindsInstance

import dagger.BindsInstance

import dagger.Subcomponent

import dagger.Subcomponent

import kotlinx.coroutines.CoroutineScope





/** This dagger component is used to init the ViewModel and Interactors needed for a bundle row */

/** This dagger component is used to init the ViewModel and Interactors needed for a bundle row */

@Subcomponent

@Subcomponent
@@ -32,6 +33,8 @@ interface BundleRowComponent { 
    interface Builder {

    interface Builder {

        @BindsInstance fun bindBundleRepository(repository: BundleRepository): Builder

        @BindsInstance fun bindBundleRepository(repository: BundleRepository): Builder





        @BindsInstance fun bindScope(scope: CoroutineScope): Builder



        fun build(): BundleRowComponent

        fun build(): BundleRowComponent

    }

    }

}
 
}

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/domain/interactor/BundleInteractor.kt

+37 −4
Original line number Original line Diff line number Diff line
@@ -28,6 +28,7 @@ import com.android.compose.animation.scene.SceneKey 
import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.notifications.ui.composable.row.BundleHeader

import com.android.systemui.res.R

import com.android.systemui.res.R

import com.android.systemui.shade.domain.interactor.ShadeInteractor

import com.android.systemui.statusbar.notification.row.dagger.BundleRowScope

import com.android.systemui.statusbar.notification.row.dagger.BundleRowScope

import com.android.systemui.statusbar.notification.row.data.model.AppData

import com.android.systemui.statusbar.notification.row.data.model.AppData

import com.android.systemui.statusbar.notification.row.data.repository.BundleRepository

import com.android.systemui.statusbar.notification.row.data.repository.BundleRepository
@@ -35,12 +36,15 @@ import com.android.systemui.statusbar.notification.row.icon.AppIconProvider 
import com.android.systemui.util.icuMessageFormat

import com.android.systemui.util.icuMessageFormat

import com.android.systemui.util.time.SystemClock

import com.android.systemui.util.time.SystemClock

import com.android.systemui.utils.coroutines.flow.mapLatestConflated

import com.android.systemui.utils.coroutines.flow.mapLatestConflated

import javax.inject.Inject

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Job

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.launch

import kotlinx.coroutines.withContext

import kotlinx.coroutines.withContext

import javax.inject.Inject





/** Provides functionality for UI to interact with a Notification Bundle. */

/** Provides functionality for UI to interact with a Notification Bundle. */

@BundleRowScope

@BundleRowScope
@@ -52,6 +56,8 @@ constructor( 
    private val context: Context,

    private val context: Context,

    @Background private val backgroundDispatcher: CoroutineDispatcher,

    @Background private val backgroundDispatcher: CoroutineDispatcher,

    private val systemClock: SystemClock,

    private val systemClock: SystemClock,

    private val shadeInteractor: ShadeInteractor,

    @BundleRowScope private val scope: CoroutineScope,

) {

) {

    @get:StringRes

    @get:StringRes

    val titleText: Int

    val titleText: Int
@@ -72,6 +78,27 @@ constructor( 
                numberOfChildren ?: 0,

                numberOfChildren ?: 0,

            )

            )





    private var sceneTargetJob: Job? = null



    init {

        observeShadeState()

    }



    private fun observeShadeState() {

        scope.launch {

            shadeInteractor.isShadeFullyCollapsed

                .filter { isCollapsed -> isCollapsed } // Only act when it becomes true

                .collect {

                    if (repository.state?.currentScene == BundleHeader.Scenes.Expanded) {

                        repository.lastCollapseTime = systemClock.uptimeMillis()



                        // Use snapTo() since the UI is already gone and no animation is needed.

                        repository.state?.snapTo(BundleHeader.Scenes.Collapsed)

                    }

                }

        }

    }



    /** Filters the list of AppData based on time of last collapse by user. */

    /** Filters the list of AppData based on time of last collapse by user. */

    private fun filterByCollapseTime(

    private fun filterByCollapseTime(

        rawAppDataList: List<AppData>,

        rawAppDataList: List<AppData>,
@@ -123,11 +150,17 @@ constructor( 
    }

    }





    fun setTargetScene(scene: SceneKey) {

    fun setTargetScene(scene: SceneKey) {

        sceneTargetJob?.cancel()



        sceneTargetJob = scope.launch {

            state?.setTargetScene(scene, composeScope!!)

            state?.setTargetScene(scene, composeScope!!)

        if (state?.currentScene == BundleHeader.Scenes.Collapsed) {



            // [setTargetScene] does not immediately update [currentScene] so we must check [scene]

            if (scene == BundleHeader.Scenes.Collapsed) {

                repository.lastCollapseTime = systemClock.uptimeMillis()

                repository.lastCollapseTime = systemClock.uptimeMillis()

            }

            }

        }

        }

    }





    private fun fetchAppIcon(appData: AppData): Drawable? {

    private fun fetchAppIcon(appData: AppData): Drawable? {

        return try {

        return try {