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

Commit 9abc6419 authored by Massimo Carli's avatar Massimo Carli Committed by Android (Google) Code Review
Browse files

Merge "[47/n] Simplify Letterbox Surface lifecycle" into main

parents 2c47df20 9c6f4ed7
Loading
Loading
Loading
Loading
+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
+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,
+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);
    }
}
+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)
            )
        }
    }
}
+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,