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

Commit 9c6f4ed7 authored by Massimo Carli's avatar Massimo Carli
Browse files

[47/n] Simplify Letterbox Surface lifecycle 

Removing the Letterbox surfaces created when a letterboxed activity
appears in a Task can be done when the Task itself is removed.
In the existing LetterboxLifecycleController is much easier to
just handle creation, visibility and resizing of the letterbox
surfaces.

Also moving the resizing of the Letterbox surfaces later in the
lifecycle logic, allows to handle the case when the surfaces have size 0.
In that case the Crop would not happen causing an unwanted behaviour
of a surface filling the task completely which is obvious in case
of multiple surfaces for a transparent activity.

Flag: com.android.window.flags.app_compat_refactoring
Bug: 409951687
Test: atest WMShellUnitTests:LetterboxLifecycleControllerImplTest
Test: atest WMShellUnitTests:MigrationLetterboxTransitionObserverTest
Test: atest WMShellUnitTests:DelegateLetterboxTransitionObserverTest

Change-Id: If411b45f25aec74447e34753cb3997a22b8352f1
parent a6a5c0cf

libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterbox/lifecycle/LetterboxLifecycleControllerImpl.kt

+32 −35
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package com.android.wm.shell.compatui.letterbox.lifecycle



import android.view.SurfaceControl

import com.android.wm.shell.common.transition.TransitionStateHolder

import com.android.wm.shell.compatui.letterbox.LetterboxController

import com.android.wm.shell.compatui.letterbox.LetterboxControllerStrategy
@@ -26,7 +25,6 @@ import com.android.wm.shell.compatui.letterbox.LetterboxControllerStrategy 
 */

class LetterboxLifecycleControllerImpl(

    private val letterboxController: LetterboxController,

    private val transitionStateHolder: TransitionStateHolder,

    private val letterboxModeStrategy: LetterboxControllerStrategy

) : LetterboxLifecycleController {
@@ -36,15 +34,10 @@ class LetterboxLifecycleControllerImpl( 
        finishTransaction: SurfaceControl.Transaction

    ) {

        val key = event.letterboxKey()

        // Each [LetterboxController] will handle its own Surfaces and will be responsible to

        // avoid the creation happens twice or that some visibility/size change operation

        // happens on missing surfaces.

        with(letterboxController) {

            when (event.type) {

                LetterboxLifecycleEventType.CLOSE -> {

                    if (!transitionStateHolder.isRecentsTransitionRunning()) {

                        // For the other types of close we need to check recents.

                        destroyLetterboxSurface(key, finishTransaction)

                    }

                }

                else -> {

            if (event.letterboxBounds != null) {

                // In this case the top Activity is letterboxed.

                letterboxModeStrategy.configureLetterboxMode()
@@ -56,21 +49,25 @@ class LetterboxLifecycleControllerImpl( 
                        event.containerToken

                    )

                }

                        updateLetterboxSurfaceBounds(

            }

            updateLetterboxSurfaceVisibility(

                key,

                startTransaction,

                            event.taskBounds,

                            event.letterboxBounds

                visible = event.letterboxBounds != null

            )

                    } else {

                        updateLetterboxSurfaceVisibility(

            // This happens after the visibility update because it needs to

            // check if the surfaces to show have empty bounds. When that happens

            // the clipAndCrop() doesn't actually work because cropping an empty

            // Rect means "do not crop" with the result of a surface filling the

            // task completely.

            if (event.letterboxBounds != null) {

                updateLetterboxSurfaceBounds(

                    key,

                    startTransaction,

                            visible = false

                    event.taskBounds,

                    event.letterboxBounds

                )

            }

        }

    }

}
 
    }

}
 
 No newline at end of file

libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterbox/lifecycle/LetterboxLifecycleEvent.kt

+1 −0
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ fun Change.asLetterboxLifecycleEventType() = when { 
/**

 * Creates a [LetterboxLifecycleEvent] from the information in a [Change].

 */

// TODO(b/375339716): Clean code and improve readability.

fun Change.toLetterboxLifecycleEvent(): LetterboxLifecycleEvent {

    val taskBounds = Rect(

        endRelOffset.x,

libs/WindowManager/Shell/src/com/android/wm/shell/dagger/LetterboxModule.java

+1 −3
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.android.wm.shell.dagger; 


import android.annotation.NonNull;



import com.android.wm.shell.common.transition.TransitionStateHolder;

import com.android.wm.shell.compatui.letterbox.DelegateLetterboxTransitionObserver;

import com.android.wm.shell.compatui.letterbox.LetterboxControllerStrategy;

import com.android.wm.shell.compatui.letterbox.MixedLetterboxController;
@@ -73,10 +72,9 @@ public abstract class LetterboxModule { 
    @Provides

    static LetterboxLifecycleController provideLetterboxLifecycleController(

            @NonNull MixedLetterboxController letterboxController,

            @NonNull TransitionStateHolder transitionStateHolder,

            @NonNull LetterboxControllerStrategy letterboxControllerStrategy

    ) {

        return new LetterboxLifecycleControllerImpl(letterboxController, transitionStateHolder,

        return new LetterboxLifecycleControllerImpl(letterboxController,

                letterboxControllerStrategy);

    }

}

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterbox/MigrationLetterboxTransitionObserverTest.kt

+13 −67
Original line number Diff line number Diff line
@@ -22,31 +22,27 @@ import android.platform.test.annotations.DisableFlags 
import android.platform.test.annotations.EnableFlags

import android.testing.AndroidTestingRunner

import android.view.SurfaceControl

import android.view.WindowManager.TRANSIT_CLOSE

import android.view.WindowManager

import android.window.WindowContainerToken

import androidx.test.filters.SmallTest

import com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn

import com.android.window.flags.Flags

import com.android.wm.shell.ShellTestCase

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

import com.android.wm.shell.common.transition.TransitionStateHolder

import com.android.wm.shell.compatui.letterbox.lifecycle.FakeLetterboxLifecycleEventFactory

import com.android.wm.shell.compatui.letterbox.lifecycle.LetterboxLifecycleController

import com.android.wm.shell.compatui.letterbox.lifecycle.LetterboxLifecycleControllerImpl

import com.android.wm.shell.compatui.letterbox.lifecycle.toLetterboxLifecycleEvent

import com.android.wm.shell.recents.RecentsTransitionHandler

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

import com.android.wm.shell.transition.Transitions

import com.android.wm.shell.util.TransitionObserverInputBuilder

import com.android.wm.shell.util.executeTransitionObserverTest

import java.util.function.Consumer

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.kotlin.any

import org.mockito.kotlin.doReturn

import org.mockito.kotlin.eq

import org.mockito.kotlin.mock

import org.mockito.kotlin.verify

import java.util.function.Consumer



/**

 * Tests for [LetterboxTransitionObserver] using [LetterboxLifecycleControllerImpl].
@@ -116,6 +112,7 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
                inputBuilder {

                    buildTransitionInfo()

                    r.createTopActivityChange(

                        mode = WindowManager.TRANSIT_OPEN,

                        inputBuilder = this,

                        isLetterboxed = true,

                        letterboxBounds = Rect(500, 0, 1500, 1000),
@@ -128,7 +125,7 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
                validateOutput {

                    r.creationEventDetected(expected = true)

                    r.configureStrategyInvoked(expected = true)

                    r.visibilityEventDetected(expected = false)

                    r.visibilityEventDetected(expected = true, visible = true)

                    r.destroyEventDetected(expected = false)

                    r.updateSurfaceBoundsEventDetected(

                        expected = true,
@@ -148,7 +145,11 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 


                inputBuilder {

                    buildTransitionInfo()

                    r.createTopActivityChange(inputBuilder = this, isLetterboxed = false)

                    r.createTopActivityChange(

                        mode = WindowManager.TRANSIT_OPEN,

                        inputBuilder = this,

                        isLetterboxed = false

                    )

                }



                validateOutput {
@@ -161,44 +162,6 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
        }

    }



    @Test

    fun `When closing change with no recents running letterbox surfaces are destroyed`() {

        runTestScenario { r ->

            executeTransitionObserverTest(observerFactory = r.observerFactory) {

                r.invokeShellInit()



                inputBuilder {

                    buildTransitionInfo()

                    r.configureRecentsState(running = false)

                    r.createClosingChange(inputBuilder = this)

                }



                validateOutput {

                    r.destroyEventDetected(expected = true)

                }

            }

        }

    }



    @Test

    fun `When closing change and recents are running letterbox surfaces are not destroyed`() {

        runTestScenario { r ->

            executeTransitionObserverTest(observerFactory = r.observerFactory) {

                r.invokeShellInit()



                inputBuilder {

                    buildTransitionInfo()

                    r.createClosingChange(inputBuilder = this)

                    r.configureRecentsState(running = true)

                }



                validateOutput {

                    r.destroyEventDetected(expected = false)

                }

            }

        }

    }



    /**

     * Runs a test scenario providing a Robot.

     */
@@ -222,7 +185,6 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
        private val transitions: Transitions

        private val letterboxController: LetterboxController

        private val letterboxObserver: DelegateLetterboxTransitionObserver

        private val transitionStateHolder: TransitionStateHolder

        private val letterboxStrategy: LetterboxControllerStrategy

        private val letterboxLifecycleContoller: LetterboxLifecycleController

        private var letterboxLifecycleEventFactory: FakeLetterboxLifecycleEventFactory
@@ -235,12 +197,8 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
            transitions = mock<Transitions>()

            letterboxController = mock<LetterboxController>()

            letterboxStrategy = mock<LetterboxControllerStrategy>()

            transitionStateHolder =

                TransitionStateHolder(shellInit, mock<RecentsTransitionHandler>())

            spyOn(transitionStateHolder)

            letterboxLifecycleContoller = LetterboxLifecycleControllerImpl(

                letterboxController,

                transitionStateHolder,

                letterboxStrategy

            )

            letterboxLifecycleEventFactory = FakeLetterboxLifecycleEventFactory(
@@ -270,10 +228,6 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
            consumer(letterboxLifecycleEventFactory)

        }



        fun configureRecentsState(running: Boolean) {

            doReturn(running).`when`(transitionStateHolder).isRecentsTransitionRunning()

        }



        fun creationEventDetected(

            expected: Boolean,

            displayId: Int = DISPLAY_ID,
@@ -332,6 +286,7 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
            verify(letterboxStrategy, expected.asMode()).configureLetterboxMode()



        fun createTopActivityChange(

            mode: Int = WindowManager.TRANSIT_NONE,

            inputBuilder: TransitionObserverInputBuilder,

            isLetterboxed: Boolean = true,

            letterboxBounds: Rect? = null,
@@ -343,6 +298,7 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
        ) {

            inputBuilder.addChange(

                inputBuilder.createChange(

                    changeMode = mode,

                    changeTaskInfo = inputBuilder.createTaskInfo().apply {

                        appCompatTaskInfo.isTopActivityLetterboxed = isLetterboxed

                        appCompatTaskInfo.topActivityLetterboxBounds = letterboxBounds
@@ -353,18 +309,8 @@ class MigrationLetterboxTransitionObserverTest : ShellTestCase() { 
                    endRelOffset.x = taskPosition.x

                    endRelOffset.y = taskPosition.y

                    endAbsBounds.set(Rect(0, 0, taskWidth, taskHeight))

                })

                }



        fun createClosingChange(

            inputBuilder: TransitionObserverInputBuilder,

            displayId: Int = DISPLAY_ID,

            taskId: Int = TASK_ID

        ) {

            inputBuilder.addChange(changeTaskInfo = inputBuilder.createTaskInfo().apply {

                this.taskId = taskId

                this.displayId = displayId

            }, changeMode = TRANSIT_CLOSE)

            )

        }

    }

}

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterbox/lifecycle/LetterboxLifecycleControllerImplTest.kt

+2 −48
Original line number Diff line number Diff line
@@ -23,18 +23,16 @@ import android.view.SurfaceControl.Transaction 
import android.window.WindowContainerToken

import androidx.test.filters.SmallTest

import com.android.wm.shell.ShellTestCase

import com.android.wm.shell.common.transition.TransitionStateHolder

import com.android.wm.shell.compatui.letterbox.LetterboxController

import com.android.wm.shell.compatui.letterbox.LetterboxControllerStrategy

import com.android.wm.shell.compatui.letterbox.LetterboxKey

import com.android.wm.shell.compatui.letterbox.asMode

import java.util.function.Consumer

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.kotlin.doReturn

import org.mockito.kotlin.eq

import org.mockito.kotlin.mock

import org.mockito.kotlin.verify

import java.util.function.Consumer



/**

 * Tests for [LetterboxLifecycleControllerImpl].
@@ -55,32 +53,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 
        }

    }



    @Test

    fun `Letterbox surfaces is destroyed when CLOSE and isRecentsTransitionRunning is false`() {

        runTestScenario { r ->

            r.configureIsRecentsTransitionRunning(running = false)

            r.invokeLifecycleControllerWith(

                r.createLifecycleEvent(

                    type = LetterboxLifecycleEventType.CLOSE

                )

            )

            r.verifyDestroyLetterboxSurface(expected = true)

        }

    }



    @Test

    fun `Letterbox surfaces is NOT destroyed when CLOSE and isRecentsTransitionRunning is true`() {

        runTestScenario { r ->

            r.configureIsRecentsTransitionRunning(running = true)

            r.invokeLifecycleControllerWith(

                r.createLifecycleEvent(

                    type = LetterboxLifecycleEventType.CLOSE

                )

            )

            r.verifyDestroyLetterboxSurface(expected = false)

        }

    }



    @Test

    fun `Letterbox is hidden with OPEN Transition but not letterboxed`() {

        runTestScenario { r ->
@@ -149,7 +121,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 


        private val lifecycleController: LetterboxLifecycleControllerImpl

        private val letterboxController: LetterboxController

        private val transitionStateHolder: TransitionStateHolder

        private val letterboxModeStrategy: LetterboxControllerStrategy

        private val startTransaction: Transaction

        private val finishTransaction: Transaction
@@ -169,7 +140,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 


        init {

            letterboxController = mock<LetterboxController>()

            transitionStateHolder = mock<TransitionStateHolder>()

            letterboxModeStrategy = mock<LetterboxControllerStrategy>()

            startTransaction = mock<Transaction>()

            finishTransaction = mock<Transaction>()
@@ -177,7 +147,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 
            leash = mock<SurfaceControl>()

            lifecycleController = LetterboxLifecycleControllerImpl(

                letterboxController,

                transitionStateHolder,

                letterboxModeStrategy

            )

        }
@@ -200,10 +169,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 
            leash = letterboxActivityLeash

        )



        fun configureIsRecentsTransitionRunning(running: Boolean) {

            doReturn(running).`when`(transitionStateHolder).isRecentsTransitionRunning()

        }



        fun invokeLifecycleControllerWith(event: LetterboxLifecycleEvent) {

            lifecycleController.onLetterboxLifecycleEvent(

                event,
@@ -212,17 +177,6 @@ class LetterboxLifecycleControllerImplTest : ShellTestCase() { 
            )

        }



        fun verifyDestroyLetterboxSurface(

            expected: Boolean,

            displayId: Int = DISPLAY_ID,

            taskId: Int = TASK_ID

        ) {

            verify(

                letterboxController,

                expected.asMode()

            ).destroyLetterboxSurface(eq(LetterboxKey(displayId, taskId)), eq(finishTransaction))

        }



        fun verifyCreateLetterboxSurface(

            expected: Boolean,

            displayId: Int = DISPLAY_ID,