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

Commit 35168d16 authored by Steve Elliott's avatar Steve Elliott
Browse files

fix stale bundle header state after open QS 

Occasionally, [ShadeInteractor.isShadeFullyCollapsed] will emit `true`
when QS is fully expanded, which was causing us to update the bundle
header to collapsed, even though the bundle itself (tracked in the
GroupExpansionManager) is still expanded.

After some digging, the [snapTo] used inside of [BundleInteractor] was
determined to be unnecessary, so by moving the [lastCollapseTime] update
to the code path where the bundle is collapsed when the shade is closed,
we can remove the usage of [isShadeFullyCollapsed] entirely.

Flag: com.android.systemui.notification_bundle_ui
Fixes: 435245683
Test: manual
Change-Id: Iaa75b5e0576579d08a172c9bcf8db03642c5af47
parent 1933f823

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

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

import com.android.systemui.kosmos.testScope

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.repository.BundleRepository

import com.android.systemui.statusbar.notification.row.data.repository.testBundleRepository
@@ -46,8 +44,8 @@ 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 kotlin.test.Test

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.test.runCurrent

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

import org.mockito.kotlin.whenever

import platform.test.motion.compose.runMonotonicClockTest

import kotlin.test.Test



@OptIn(ExperimentalCoroutinesApi::class)

@SmallTest
@@ -77,8 +74,6 @@ class BundleInteractorTest : SysuiTestCase() { 
    private val fakeSystemClock = FakeSystemClock()



    private val testBundleRepository: BundleRepository = kosmos.testBundleRepository

    private val mockShadeInteractor = mock<ShadeInteractor>()

    private val isShadeFullyCollapsedFlow = MutableStateFlow(false)

    private lateinit var underTest: BundleInteractor



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

    fun setUp() {

        kosmos.appIconProvider = kosmos.mockAppIconProvider

        whenever(mockShadeInteractor.isShadeFullyCollapsed).thenReturn(isShadeFullyCollapsedFlow)

        kosmos.shadeInteractor = mockShadeInteractor

        kosmos.systemClock = fakeSystemClock

        underTest = kosmos.bundleInteractor

    }
@@ -296,7 +289,7 @@ class BundleInteractorTest : SysuiTestCase() { 
    }



    @Test

    fun setTargetScene_whenCollapsing_updatesLastCollapseTime() = runMonotonicClockTest {

    fun setExpansionState_whenCollapsing_updatesLastCollapseTime() = runMonotonicClockTest {

        // Arrange

        val testTime = 20000L

        fakeSystemClock.setUptimeMillis(testTime)
@@ -308,7 +301,7 @@ class BundleInteractorTest : SysuiTestCase() { 
        underTest.composeScope = this



        // Act

        underTest.setTargetScene(BundleHeader.Scenes.Collapsed)

        underTest.setExpansionState(isExpanded = false)

        testScope.runCurrent()



        // Assert
@@ -316,7 +309,7 @@ class BundleInteractorTest : SysuiTestCase() { 
    }



    @Test

    fun setTargetScene_whenExpanding_doesNotUpdateLastCollapseTime() = runMonotonicClockTest {

    fun setExpansionState_whenExpanding_doesNotUpdateLastCollapseTime() = runMonotonicClockTest {

        // Arrange

        val initialTime = 11000L

        testBundleRepository.lastCollapseTime = initialTime
@@ -329,54 +322,10 @@ class BundleInteractorTest : SysuiTestCase() { 
        underTest.composeScope = this



        // Act

        underTest.setTargetScene(BundleHeader.Scenes.Expanded)

        underTest.setExpansionState(isExpanded = true)

        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/src/com/android/systemui/statusbar/notification/collection/render/BundleBarn.kt

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



import android.content.Context

import android.view.View

import android.view.ViewGroup

import androidx.compose.runtime.Composable

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

import com.android.compose.theme.PlatformTheme

import com.android.systemui.dagger.SysUISingleton

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

import com.android.systemui.initOnBackPressedDispatcherOwner

import com.android.systemui.lifecycle.rememberViewModel

import com.android.systemui.lifecycle.repeatWhenAttached
@@ -51,10 +49,6 @@ import com.android.systemui.util.time.SystemClock 
import dagger.Lazy

import javax.inject.Inject

import javax.inject.Provider

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.SupervisorJob

import kotlinx.coroutines.cancel



/** Class that handles inflating BundleEntry view and controller, for use by NodeSpecBuilder. */

@SysUISingleton
@@ -62,14 +56,13 @@ class BundleBarn 
@Inject

constructor(

    private val rowComponent: ExpandableNotificationRowComponent.Builder,

    private val bundleRowComponentBuilder: BundleRowComponent.Builder,

    private val bundleRowComponentFactory: BundleRowComponent.Factory,

    private val rowInflaterTaskProvider: Provider<RowInflaterTask>,

    private val listContainer: NotificationListContainer,

    @ShadeDisplayAware val context: Context,

    val systemClock: SystemClock,

    val logger: RowInflaterTaskLogger,

    val userTracker: UserTracker,

    @Main private val mainDispatcher: CoroutineDispatcher,

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

    private val iconManager: IconManager,

) : PipelineDumpable {
@@ -121,34 +114,18 @@ constructor( 
    }



    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 =

            bundleRowComponentBuilder

                .bindBundleRepository(bundleEntry.bundleRepository)

                .bindScope(scope)

                .build()



            bundleRowComponentFactory.create(repository = bundleEntry.bundleRepository)

        val headerComposeView = ComposeView(context)

        row.setBundleHeaderView(headerComposeView)

        val viewModelFactory = bundleRowComponent.bundleViewModelFactory()

        headerComposeView.repeatWhenAttached {

            repeatOnLifecycle(Lifecycle.State.CREATED) {

                headerComposeView.initOnBackPressedDispatcherOwner(lifecycle)

                headerComposeView.setContent {

                    HeaderComposeViewContent(

                        row = row,

                        bundleHeaderViewModelFactory =

                            bundleRowComponent.bundleViewModelFactory()::create,

                        bundleHeaderViewModelFactory = viewModelFactory::create,

                    )

                }

            }

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

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

import dagger.BindsInstance

import dagger.Subcomponent

import kotlinx.coroutines.CoroutineScope



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

@Subcomponent
@@ -29,12 +28,8 @@ interface BundleRowComponent { 


    fun bundleViewModelFactory(): BundleHeaderViewModel.Factory



    @Subcomponent.Builder

    interface Builder {

        @BindsInstance fun bindBundleRepository(repository: BundleRepository): Builder



        @BindsInstance fun bindScope(scope: CoroutineScope): Builder



        fun build(): BundleRowComponent

    @Subcomponent.Factory

    interface Factory {

        fun create(@BindsInstance repository: BundleRepository): BundleRowComponent

    }

}

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

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

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

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.data.model.AppData

import com.android.systemui.statusbar.notification.row.data.repository.BundleRepository
@@ -39,11 +38,8 @@ import com.android.systemui.utils.coroutines.flow.mapLatestConflated 
import javax.inject.Inject

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Job

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.launch

import kotlinx.coroutines.withContext



/** Provides functionality for UI to interact with a Notification Bundle. */
@@ -56,8 +52,6 @@ constructor( 
    private val context: Context,

    @Background private val backgroundDispatcher: CoroutineDispatcher,

    private val systemClock: SystemClock,

    private val shadeInteractor: ShadeInteractor,

    @BundleRowScope private val scope: CoroutineScope,

) {

    @get:StringRes

    val titleText: Int
@@ -86,27 +80,6 @@ constructor( 
                numberOfChildrenContentDescription,

            )



    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. */

    private fun filterByCollapseTime(

        rawAppDataList: List<AppData>,
@@ -149,22 +122,19 @@ constructor( 
            if (isExpanded) BundleHeader.Scenes.Expanded else BundleHeader.Scenes.Collapsed,

            composeScope!!,

        )

        if (!isExpanded) {

            repository.lastCollapseTime = systemClock.uptimeMillis()

        }

    }



    fun setTargetScene(scene: SceneKey) {

        sceneTargetJob?.cancel()



        sceneTargetJob =

            scope.launch {

        state?.setTargetScene(scene, composeScope!!)



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

                // [scene]

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

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

            repository.lastCollapseTime = systemClock.uptimeMillis()

        }

    }

    }



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

        return try {

packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/notification/row/domain/BundleInteractorKosmos.kt

+0 −6
Original line number Diff line number Diff line
@@ -19,8 +19,6 @@ package com.android.systemui.statusbar.notification.row.domain 
import android.content.applicationContext

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.testDispatcher

import com.android.systemui.kosmos.testScope

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

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

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

import com.android.systemui.statusbar.notification.row.icon.appIconProvider
@@ -34,9 +32,5 @@ val Kosmos.bundleInteractor by 
            context = applicationContext,

            backgroundDispatcher = testDispatcher,

            systemClock = systemClock,

            shadeInteractor = shadeInteractor,

            // Use background task that is allowed to run for the duration of the test; this task

            // will be canceled automatically

            scope = testScope.backgroundScope

        )

    }