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

Commit a1c2d87b authored by Orhan Uysal's avatar Orhan Uysal
Browse files

Do not use defaultInstance for persistent repo 

In following cases we used default instances to populate the repo:
- If task details for a taskId is not contained in the persisted repo
- If there is a problem reading the persistent repository

These might result in populating the in-memory repo with non-existing
states that are not removed. Instead use null values so we never add any
tasks to repo in case persistent state is not correct.

Bug: 371955621
Test: Manual
Flag: EXEMPT Bugfix
Change-Id: Idbe0663cf143a489ceb596bc0b92690ebb1706ff
parent b82fcf5c

libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopRepository.kt

+6 −8
Original line number Diff line number Diff line
@@ -30,7 +30,6 @@ import androidx.core.util.valueIterator 
import com.android.internal.protolog.ProtoLog

import com.android.window.flags.Flags

import com.android.wm.shell.desktopmode.persistence.DesktopPersistentRepository

import com.android.wm.shell.desktopmode.persistence.DesktopTask

import com.android.wm.shell.desktopmode.persistence.DesktopTaskState

import com.android.wm.shell.protolog.ShellProtoLogGroup.WM_SHELL_DESKTOP_MODE

import com.android.wm.shell.shared.annotations.ShellMainThread
@@ -113,7 +112,8 @@ class DesktopRepository ( 
        if (!Flags.enableDesktopWindowingPersistence()) return

        //  TODO: b/365962554 - Handle the case that user moves to desktop before it's initialized

        mainCoroutineScope.launch {

            val desktop = persistentRepository.readDesktop()

            val desktop = persistentRepository.readDesktop() ?: return@launch



            val maxTasks =

                DesktopModeStatus.getMaxTaskLimit(context).takeIf { it > 0 }

                    ?: desktop.zOrderedTasksCount
@@ -121,13 +121,11 @@ class DesktopRepository ( 
            desktop.zOrderedTasksList

                // Reverse it so we initialize the repo from bottom to top.

                .reversed()

                .map { taskId ->

                    desktop.tasksByTaskIdMap.getOrDefault(

                        taskId,

                        DesktopTask.getDefaultInstance()

                    )

                .mapNotNull { taskId ->

                    desktop.tasksByTaskIdMap[taskId]?.takeIf {

                        it.desktopTaskState == DesktopTaskState.VISIBLE

                    }

                }

                .filter { task -> task.desktopTaskState == DesktopTaskState.VISIBLE }

                .take(maxTasks)

                .forEach { task ->

                    addOrMoveFreeformTaskToTop(desktop.displayId, task.taskId)

libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/persistence/DesktopPersistentRepository.kt

+6 −7
Original line number Diff line number Diff line
@@ -73,15 +73,14 @@ class DesktopPersistentRepository( 
     */

    private suspend fun getDesktopRepositoryState(

        userId: Int = DEFAULT_USER_ID

    ): DesktopRepositoryState =

    ): DesktopRepositoryState? =

        try {

            dataStoreFlow

                .first()

                .desktopRepoByUserMap

                .getOrDefault(userId, DesktopRepositoryState.getDefaultInstance())

                .desktopRepoByUserMap[userId]

        } catch (e: Exception) {

            Log.e(TAG, "Unable to read from datastore", e)

            DesktopRepositoryState.getDefaultInstance()

            null

        }



    /**
@@ -91,13 +90,13 @@ class DesktopPersistentRepository( 
    suspend fun readDesktop(

        userId: Int = DEFAULT_USER_ID,

        desktopId: Int = DEFAULT_DESKTOP_ID,

    ): Desktop =

    ): Desktop? =

        try {

            val repository = getDesktopRepositoryState(userId)

            repository.getDesktopOrThrow(desktopId)

            repository?.getDesktopOrThrow(desktopId)

        } catch (e: Exception) {

            Log.e(TAG, "Unable to get desktop info from persistent repository", e)

            Desktop.getDefaultInstance()

            null

        }



    /** Adds or updates a desktop stored in the datastore */

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/persistence/DesktopPersistentRepositoryTest.kt

+5 −5
Original line number Diff line number Diff line
@@ -115,8 +115,8 @@ class DesktopPersistentRepositoryTest : ShellTestCase() { 
                freeformTasksInZOrder = freeformTasksInZOrder)



            val actualDesktop = datastoreRepository.readDesktop(DEFAULT_USER_ID, DEFAULT_DESKTOP_ID)

            assertThat(actualDesktop.tasksByTaskIdMap).hasSize(2)

            assertThat(actualDesktop.getZOrderedTasks(0)).isEqualTo(2)

            assertThat(actualDesktop?.tasksByTaskIdMap).hasSize(2)

            assertThat(actualDesktop?.getZOrderedTasks(0)).isEqualTo(2)

        }

    }
@@ -138,7 +138,7 @@ class DesktopPersistentRepositoryTest : ShellTestCase() { 
                freeformTasksInZOrder = freeformTasksInZOrder)



            val actualDesktop = datastoreRepository.readDesktop(DEFAULT_USER_ID, DEFAULT_DESKTOP_ID)

            assertThat(actualDesktop.tasksByTaskIdMap[task.taskId]?.desktopTaskState)

            assertThat(actualDesktop?.tasksByTaskIdMap?.get(task.taskId)?.desktopTaskState)

                .isEqualTo(DesktopTaskState.MINIMIZED)

        }

    }
@@ -161,8 +161,8 @@ class DesktopPersistentRepositoryTest : ShellTestCase() { 
                freeformTasksInZOrder = freeformTasksInZOrder)



            val actualDesktop = datastoreRepository.readDesktop(DEFAULT_USER_ID, DEFAULT_DESKTOP_ID)

            assertThat(actualDesktop.tasksByTaskIdMap).isEmpty()

            assertThat(actualDesktop.zOrderedTasksList).isEmpty()

            assertThat(actualDesktop?.tasksByTaskIdMap).isEmpty()

            assertThat(actualDesktop?.zOrderedTasksList).isEmpty()

        }

    }