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

Commit b5dc1875 authored by Jorge Gil's avatar Jorge Gil
Browse files

Desks: Do not re-create desks from other users 

Fixes a crash when the re-create desk attempt fails for another user.
Prior to this change, the desk recreation was skipped as intented for
different users, but the DeskRecreationFactory returned the original
deskId, which caused the initializer to restore the desk with the old
deskId even if the desk was not re-created succesfully. Later on, when
trying to move a task into that non-existent desk, a crash would occur.

This change makes the factory return a null deskId in those cases, to
signal to the initializer not to restore the desk in the repository.

Proper multi-user desk restoration still needs to be implemented in the
future.

Flag: com.android.window.flags.enable_multiple_desktops_backend
Fix: 402607975
Bug: 400984250
Test: in an HSUM build, drag a task into desktop - observe no crash
Change-Id: I9f7f6ed1485b665be7f4088859c884a6daa4b13c
parent c6af79cf

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

+2 −2
Original line number Diff line number Diff line
@@ -287,8 +287,8 @@ class DesktopTasksController( 
                DeskRecreationFactory { deskUserId, destinationDisplayId, deskId ->

                    if (deskUserId != userId) {

                        // TODO: b/400984250 - add multi-user support for multi-desk restoration.

                        logW("Tried to recreated desk of another user.")

                        deskId

                        logW("Tried to re-create desk of another user.")

                        null

                    } else {

                        desksOrganizer.createDesk(destinationDisplayId)

                    }

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

+6 −4
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ import kotlinx.coroutines.flow.StateFlow 


/** Interface for initializing the [DesktopUserRepositories]. */

interface DesktopRepositoryInitializer {

    /** A factory used to recreate a desk from persistence. */

    /** A factory used to re-create a desk from persistence. */

    var deskRecreationFactory: DeskRecreationFactory



    /** A flow that emits true when the repository has been initialized. */
@@ -30,9 +30,11 @@ interface DesktopRepositoryInitializer { 
    /** Initialize the user repositories from a persistent data store. */

    fun initialize(userRepositories: DesktopUserRepositories)



    /** A factory for recreating desks. */

    /** A factory for re-creating desks. */

    fun interface DeskRecreationFactory {

        /** Recreates a restored desk and returns the new desk id. */

        suspend fun recreateDesk(userId: Int, destinationDisplayId: Int, deskId: Int): Int

        /**

         * Re-creates a restored desk and returns the new desk id, or null if re-creation failed.

         */

        suspend fun recreateDesk(userId: Int, destinationDisplayId: Int, deskId: Int): Int?

    }

}

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

+25 −9
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ class DesktopRepositoryInitializerImpl( 
                        desksToRestore.map { it.desktopId },

                        userId,

                    )

                    desksToRestore.forEach { persistentDesktop ->

                    for (persistentDesktop in desksToRestore) {

                        val maxTasks = getTaskLimit(persistentDesktop)

                        val displayId = persistentDesktop.displayId

                        val deskId = persistentDesktop.desktopId
@@ -81,17 +81,29 @@ class DesktopRepositoryInitializerImpl( 
                                destinationDisplayId = newDisplayId,

                                deskId = deskId,

                            )

                        if (newDeskId != null) {

                            logV(

                            "Recreated desk=%d in display=%d using new deskId=%d and displayId=%d",

                                "Re-created desk=%d in display=%d using new" +

                                    " deskId=%d and displayId=%d",

                                deskId,

                                displayId,

                                newDeskId,

                                newDisplayId,

                            )

                        if (newDeskId != deskId || newDisplayId != displayId) {

                        }

                        if (newDeskId == null || newDeskId != deskId || newDisplayId != displayId) {

                            logV("Removing obsolete desk from persistence under deskId=%d", deskId)

                            persistentRepository.removeDesktop(userId, deskId)

                        }

                        if (newDeskId == null) {

                            logW(

                                "Could not re-create desk=%d from display=%d in displayId=%d",

                                deskId,

                                displayId,

                                newDisplayId,

                            )

                            continue

                        }



                        // TODO: b/393961770 - [DesktopRepository] doesn't save desks to the

                        //  persistent repository until a task is added to them. Update it so that
@@ -177,6 +189,10 @@ class DesktopRepositoryInitializerImpl( 
        ProtoLog.v(WM_SHELL_DESKTOP_MODE, "%s: $msg", TAG, *arguments)

    }



    private fun logW(msg: String, vararg arguments: Any?) {

        ProtoLog.w(WM_SHELL_DESKTOP_MODE, "%s: $msg", TAG, *arguments)

    }



    /** A default implementation of [DeskRecreationFactory] that reuses the desk id. */

    private class DefaultDeskRecreationFactory : DeskRecreationFactory {

        override suspend fun recreateDesk(

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

+31 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import com.android.window.flags.Flags.FLAG_ENABLE_MULTIPLE_DESKTOPS_BACKEND 
import com.android.wm.shell.ShellTestCase

import com.android.wm.shell.common.ShellExecutor

import com.android.wm.shell.desktopmode.DesktopUserRepositories

import com.android.wm.shell.desktopmode.persistence.DesktopRepositoryInitializer.DeskRecreationFactory

import com.android.wm.shell.sysui.ShellController

import com.android.wm.shell.sysui.ShellInit

import com.google.common.truth.Truth.assertThat
@@ -242,6 +243,36 @@ class DesktopRepositoryInitializerTest : ShellTestCase() { 
                .inOrder()

        }



    @Test

    @EnableFlags(

        FLAG_ENABLE_DESKTOP_WINDOWING_PERSISTENCE,

        FLAG_ENABLE_DESKTOP_WINDOWING_HSUM,

        FLAG_ENABLE_MULTIPLE_DESKTOPS_BACKEND,

    )

    fun initWithPersistence_deskRecreationFailed_deskNotAdded() =

        runTest(StandardTestDispatcher()) {

            whenever(persistentRepository.getUserDesktopRepositoryMap())

                .thenReturn(mapOf(USER_ID_1 to desktopRepositoryState1))

            whenever(persistentRepository.getDesktopRepositoryState(USER_ID_1))

                .thenReturn(desktopRepositoryState1)

            whenever(persistentRepository.readDesktop(USER_ID_1, DESKTOP_ID_1)).thenReturn(desktop1)

            whenever(persistentRepository.readDesktop(USER_ID_1, DESKTOP_ID_2)).thenReturn(desktop2)



            // Make [DESKTOP_ID_2] re-creation fail.

            repositoryInitializer.deskRecreationFactory =

                DeskRecreationFactory { userId, destinationDisplayId, deskId ->

                    if (deskId == DESKTOP_ID_2) {

                        null

                    } else {

                        deskId

                    }

                }

            repositoryInitializer.initialize(desktopUserRepositories)



            assertThat(desktopUserRepositories.getProfile(USER_ID_1).getDeskIds(DEFAULT_DISPLAY))

                .containsExactly(DESKTOP_ID_1)

        }



    @After

    fun tearDown() {

        datastoreScope.cancel()