From 201ae49e99ff0e2bde69c760d936fcbff050cbb4 Mon Sep 17 00:00:00 2001 From: Uwais Ashraf Date: Fri, 3 Jan 2025 19:37:38 +0000 Subject: [PATCH] Make overlay run in background instead of unconfined (main) This means that when TaskOverlayViewModel.overlayState is combining flows it does it on background. If this combination results in an emission then there is still a message to main but if it does not emit (e.g. no change filtered by distinctUntilChanged()) then it will not result in a continuation on main. Bug: 387511656 Flag: com.android.launcher3.enable_refactor_task_thumbnail Test: Checking perfetto traces shows that this reduces the number of continuations on main significantly Test: Manual - overlay presence + select functionality Test: Presubmit (TAPL/image tests etc) Change-Id: I840cb86e91db5a0626191b5eab145978ef4192e9 --- .../recents/di/RecentsDependencies.kt | 1 + .../quickstep/task/util/TaskOverlayHelper.kt | 1 + .../task/viewmodel/TaskOverlayViewModel.kt | 8 +- .../viewmodel/TaskOverlayViewModelTest.kt | 162 ++++++++++-------- 4 files changed, 102 insertions(+), 70 deletions(-) diff --git a/quickstep/src/com/android/quickstep/recents/di/RecentsDependencies.kt b/quickstep/src/com/android/quickstep/recents/di/RecentsDependencies.kt index 9d8fc4f4b0..dd83af692f 100644 --- a/quickstep/src/com/android/quickstep/recents/di/RecentsDependencies.kt +++ b/quickstep/src/com/android/quickstep/recents/di/RecentsDependencies.kt @@ -191,6 +191,7 @@ class RecentsDependencies private constructor(private val appContext: Context) { recentsViewData = inject(), recentTasksRepository = inject(), getThumbnailPositionUseCase = inject(), + dispatcherProvider = inject(), ) } GetThumbnailUseCase::class.java -> GetThumbnailUseCase(taskRepository = inject()) diff --git a/quickstep/src/com/android/quickstep/task/util/TaskOverlayHelper.kt b/quickstep/src/com/android/quickstep/task/util/TaskOverlayHelper.kt index e6c8d27286..0f61b95942 100644 --- a/quickstep/src/com/android/quickstep/task/util/TaskOverlayHelper.kt +++ b/quickstep/src/com/android/quickstep/task/util/TaskOverlayHelper.kt @@ -74,6 +74,7 @@ class TaskOverlayHelper(val task: Task, val overlay: TaskOverlayFactory.TaskOver recentsViewData = RecentsDependencies.get(), getThumbnailPositionUseCase = RecentsDependencies.get(), recentTasksRepository = RecentsDependencies.get(), + dispatcherProvider = RecentsDependencies.get(), ) viewModel.overlayState .onEach { diff --git a/quickstep/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModel.kt b/quickstep/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModel.kt index 14359dbb30..81a904b3be 100644 --- a/quickstep/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModel.kt +++ b/quickstep/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModel.kt @@ -17,6 +17,7 @@ package com.android.quickstep.task.viewmodel import android.graphics.Matrix +import com.android.launcher3.util.coroutines.DispatcherProvider import com.android.quickstep.recents.data.RecentTasksRepository import com.android.quickstep.recents.usecase.GetThumbnailPositionUseCase import com.android.quickstep.recents.usecase.ThumbnailPositionState.MatrixScaling @@ -27,6 +28,7 @@ import com.android.quickstep.task.thumbnail.TaskOverlayUiState.Enabled import com.android.systemui.shared.recents.model.Task import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map /** View model for TaskOverlay */ @@ -35,11 +37,14 @@ class TaskOverlayViewModel( recentsViewData: RecentsViewData, private val getThumbnailPositionUseCase: GetThumbnailPositionUseCase, recentTasksRepository: RecentTasksRepository, + dispatcherProvider: DispatcherProvider, ) { val overlayState = combine( recentsViewData.overlayEnabled, - recentsViewData.settledFullyVisibleTaskIds.map { it.contains(task.key.id) }, + recentsViewData.settledFullyVisibleTaskIds + .map { it.contains(task.key.id) } + .distinctUntilChanged(), recentTasksRepository.getThumbnailById(task.key.id), ) { isOverlayEnabled, isFullyVisible, thumbnailData -> if (isOverlayEnabled && isFullyVisible) { @@ -52,6 +57,7 @@ class TaskOverlayViewModel( } } .distinctUntilChanged() + .flowOn(dispatcherProvider.background) fun getThumbnailPositionState(width: Int, height: Int, isRtl: Boolean): ThumbnailPositionState { val matrix: Matrix diff --git a/quickstep/tests/multivalentTests/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModelTest.kt b/quickstep/tests/multivalentTests/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModelTest.kt index 2e91f5c464..95504affd6 100644 --- a/quickstep/tests/multivalentTests/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModelTest.kt +++ b/quickstep/tests/multivalentTests/src/com/android/quickstep/task/viewmodel/TaskOverlayViewModelTest.kt @@ -22,6 +22,7 @@ import android.graphics.Bitmap import android.graphics.Color import android.graphics.Matrix import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.launcher3.util.TestDispatcherProvider import com.android.quickstep.recents.data.FakeTasksRepository import com.android.quickstep.recents.usecase.GetThumbnailPositionUseCase import com.android.quickstep.recents.usecase.ThumbnailPositionState.MatrixScaling @@ -33,7 +34,10 @@ import com.android.quickstep.task.viewmodel.TaskOverlayViewModel.ThumbnailPositi import com.android.systemui.shared.recents.model.Task import com.android.systemui.shared.recents.model.ThumbnailData import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith @@ -41,6 +45,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever /** Test for [TaskOverlayViewModel] */ +@OptIn(ExperimentalCoroutinesApi::class) @RunWith(AndroidJUnit4::class) class TaskOverlayViewModelTest { private val task = @@ -58,104 +63,123 @@ class TaskOverlayViewModelTest { private val recentsViewData = RecentsViewData() private val tasksRepository = FakeTasksRepository() private val mGetThumbnailPositionUseCase = mock() + private val dispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(dispatcher) private val systemUnderTest = - TaskOverlayViewModel(task, recentsViewData, mGetThumbnailPositionUseCase, tasksRepository) + TaskOverlayViewModel( + task, + recentsViewData, + mGetThumbnailPositionUseCase, + tasksRepository, + TestDispatcherProvider(dispatcher), + ) @Test - fun initialStateIsDisabled() = runTest { - assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) - } + fun initialStateIsDisabled() = + testScope.runTest { assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) } @Test - fun recentsViewOverlayDisabled_Disabled() = runTest { - recentsViewData.overlayEnabled.value = false - recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) + fun recentsViewOverlayDisabled_Disabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = false + recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) - assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) - } + assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) + } @Test - fun taskNotFullyVisible_Disabled() = runTest { - recentsViewData.overlayEnabled.value = true - recentsViewData.settledFullyVisibleTaskIds.value = setOf() + fun taskNotFullyVisible_Disabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = true + recentsViewData.settledFullyVisibleTaskIds.value = setOf() - assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) - } + assertThat(systemUnderTest.overlayState.first()).isEqualTo(Disabled) + } @Test - fun noThumbnail_Enabled() = runTest { - recentsViewData.overlayEnabled.value = true - recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) - task.isLocked = false + fun noThumbnail_Enabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = true + recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) + task.isLocked = false - assertThat(systemUnderTest.overlayState.first()) - .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = null)) - } + assertThat(systemUnderTest.overlayState.first()) + .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = null)) + } @Test - fun withThumbnail_RealSnapshot_NotLocked_Enabled() = runTest { - recentsViewData.overlayEnabled.value = true - recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) - tasksRepository.seedTasks(listOf(task)) - tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) - tasksRepository.setVisibleTasks(setOf(TASK_ID)) - thumbnailData.isRealSnapshot = true - task.isLocked = false + fun withThumbnail_RealSnapshot_NotLocked_Enabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = true + recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) + tasksRepository.seedTasks(listOf(task)) + tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) + tasksRepository.setVisibleTasks(setOf(TASK_ID)) + thumbnailData.isRealSnapshot = true + task.isLocked = false - assertThat(systemUnderTest.overlayState.first()) - .isEqualTo(Enabled(isRealSnapshot = true, thumbnail = thumbnailData.thumbnail)) - } + assertThat(systemUnderTest.overlayState.first()) + .isEqualTo(Enabled(isRealSnapshot = true, thumbnail = thumbnailData.thumbnail)) + } @Test - fun withThumbnail_RealSnapshot_Locked_Enabled() = runTest { - recentsViewData.overlayEnabled.value = true - recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) - tasksRepository.seedTasks(listOf(task)) - tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) - tasksRepository.setVisibleTasks(setOf(TASK_ID)) - thumbnailData.isRealSnapshot = true - task.isLocked = true + fun withThumbnail_RealSnapshot_Locked_Enabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = true + recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) + tasksRepository.seedTasks(listOf(task)) + tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) + tasksRepository.setVisibleTasks(setOf(TASK_ID)) + thumbnailData.isRealSnapshot = true + task.isLocked = true - assertThat(systemUnderTest.overlayState.first()) - .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = thumbnailData.thumbnail)) - } + assertThat(systemUnderTest.overlayState.first()) + .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = thumbnailData.thumbnail)) + } @Test - fun withThumbnail_FakeSnapshot_Enabled() = runTest { - recentsViewData.overlayEnabled.value = true - recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) - tasksRepository.seedTasks(listOf(task)) - tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) - tasksRepository.setVisibleTasks(setOf(TASK_ID)) - thumbnailData.isRealSnapshot = false - task.isLocked = false + fun withThumbnail_FakeSnapshot_Enabled() = + testScope.runTest { + recentsViewData.overlayEnabled.value = true + recentsViewData.settledFullyVisibleTaskIds.value = setOf(TASK_ID) + tasksRepository.seedTasks(listOf(task)) + tasksRepository.seedThumbnailData(mapOf(TASK_ID to thumbnailData)) + tasksRepository.setVisibleTasks(setOf(TASK_ID)) + thumbnailData.isRealSnapshot = false + task.isLocked = false - assertThat(systemUnderTest.overlayState.first()) - .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = thumbnailData.thumbnail)) - } + assertThat(systemUnderTest.overlayState.first()) + .isEqualTo(Enabled(isRealSnapshot = false, thumbnail = thumbnailData.thumbnail)) + } @Test - fun getThumbnailMatrix_MissingThumbnail() = runTest { - val isRtl = true + fun getThumbnailMatrix_MissingThumbnail() = + testScope.runTest { + val isRtl = true - whenever(mGetThumbnailPositionUseCase.run(TASK_ID, CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) - .thenReturn(MissingThumbnail) + whenever(mGetThumbnailPositionUseCase.run(TASK_ID, CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) + .thenReturn(MissingThumbnail) - assertThat(systemUnderTest.getThumbnailPositionState(CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) - .isEqualTo(ThumbnailPositionState(Matrix.IDENTITY_MATRIX, isRotated = false)) - } + assertThat( + systemUnderTest.getThumbnailPositionState(CANVAS_WIDTH, CANVAS_HEIGHT, isRtl) + ) + .isEqualTo(ThumbnailPositionState(Matrix.IDENTITY_MATRIX, isRotated = false)) + } @Test - fun getThumbnailMatrix_MatrixScaling() = runTest { - val isRtl = true - val isRotated = true + fun getThumbnailMatrix_MatrixScaling() = + testScope.runTest { + val isRtl = true + val isRotated = true - whenever(mGetThumbnailPositionUseCase.run(TASK_ID, CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) - .thenReturn(MatrixScaling(MATRIX, isRotated)) + whenever(mGetThumbnailPositionUseCase.run(TASK_ID, CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) + .thenReturn(MatrixScaling(MATRIX, isRotated)) - assertThat(systemUnderTest.getThumbnailPositionState(CANVAS_WIDTH, CANVAS_HEIGHT, isRtl)) - .isEqualTo(ThumbnailPositionState(MATRIX, isRotated)) - } + assertThat( + systemUnderTest.getThumbnailPositionState(CANVAS_WIDTH, CANVAS_HEIGHT, isRtl) + ) + .isEqualTo(ThumbnailPositionState(MATRIX, isRotated)) + } companion object { const val TASK_ID = 0