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

Commit 454e0ffc authored by Luca Zuccarini's avatar Luca Zuccarini
Browse files

Prevent a crash in the Animation library. 

It seems that we can't rely on the leash being valid just because
we had an `isValid` check before the reparenting takes place, so we
need to wrap the call in a try/catch.

Note that given that the reparenting _can_ fail, we need the rest of
the animation to behave accordingly. `startAnimation` is called
before `onTransitionAnimationStart()`, and we used to pass a boolean
argument to control how the fades work. Since this value is
contingent on whether reparenting actually succeeds, the parameter
is now a lambda that checks `reparent` in real time and outputs the
correct boolean accordingly.

Fix: b/412640764
Flag: com.android.systemui.move_transition_animation_layer
Test: atest ActivityTransitionAnimator TransitionAnimator + manual testing of the core usages
Change-Id: Ice2c0a204db693fe7f1d86f8e16a2c855fb897cc
parent 92b95d68

packages/SystemUI/animation/src/com/android/systemui/animation/ActivityTransitionAnimator.kt

+28 −17
Original line number Diff line number Diff line
@@ -1477,18 +1477,6 @@ constructor( 
                transitionAnimator.isExpandingFullyAbove(controller.transitionContainer, endState)

            val windowState = startingWindowState ?: controller.windowAnimatorState



            // We only reparent launch animations. In current integrations, returns are not affected

            // by the issue solved by reparenting, and they present additional problems when the

            // view lives in the Status Bar.

            // TODO(b/397646693): remove this exception.

            val isEligibleForReparenting = controller.isLaunching

            val viewRoot = controller.transitionContainer.viewRootImpl

            val skipReparenting =

                skipReparentTransaction || !window.leash.isValid || viewRoot == null

            if (moveTransitionAnimationLayer() && isEligibleForReparenting && !skipReparenting) {

                reparent = true

            }



            // We animate the opening window and delegate the view expansion to [this.controller].

            val delegate = this.controller

            val controller =
@@ -1556,14 +1544,36 @@ constructor( 
                            )

                        }



                        if (reparent) {

                        // We only reparent launch animations. In current integrations, returns are

                        // not affected by the issue solved by reparenting, and they present

                        // additional problems when the view lives in the Status Bar.

                        // TODO(b/397646693): remove this exception.

                        val isEligibleForReparenting = controller.isLaunching

                        val viewRoot = controller.transitionContainer.viewRootImpl

                        val skipReparenting =

                            skipReparentTransaction || !window.leash.isValid || viewRoot == null

                        if (

                            moveTransitionAnimationLayer() &&

                                isEligibleForReparenting &&

                                !skipReparenting

                        ) {

                            // Ensure that the launching window is rendered above the view's window,

                            // so it is not obstructed.

                            // Note that it is possible that the leash gets released between the

                            // check above and the call below. For this reason, we still need to

                            // wrap the transaction in a try/catch and set the value of [reparent]

                            // accordingly.

                            // TODO(b/397180418): re-use the start transaction once the

                            //  RemoteAnimation wrapper is cleaned up.

                            try {

                                SurfaceControl.Transaction().use {

                                    it.reparent(window.leash, viewRoot.surfaceControl).apply()

                                }

                                reparent = true

                            } catch (e: IllegalStateException) {

                                Log.e(TAG, "Failed to reparent transition leash: already released")

                                reparent = false

                            }

                        }



                        if (startTransaction != null) {
@@ -1636,18 +1646,19 @@ constructor( 
                } else {

                    null

                }

            val fadeWindowBackgroundLayer =

            val shouldFadeWindowBackgroundLayer = {

                if (reparent) {

                    false

                } else {

                    !controller.isBelowAnimatingWindow

                }

            }

            animation =

                transitionAnimator.startAnimation(

                    controller,

                    endState,

                    windowBackgroundColor,

                    fadeWindowBackgroundLayer = fadeWindowBackgroundLayer,

                    shouldFadeWindowBackgroundLayer = shouldFadeWindowBackgroundLayer,

                    drawHole = !controller.isBelowAnimatingWindow,

                    startVelocity = velocityPxPerS,

                    startFrameTime = windowState?.timestamp ?: -1,

packages/SystemUI/animation/src/com/android/systemui/animation/TransitionAnimator.kt

+17 −17
Original line number Diff line number Diff line
@@ -505,11 +505,11 @@ class TransitionAnimator( 
     * layer with [windowBackgroundColor] will fade in then (optionally) fade out above the

     * expanding view, and should be the same background color as the opening (or closing) window.

     *

     * If [fadeWindowBackgroundLayer] is true, then this intermediary layer will fade out during the

     * second half of the animation (if [Controller.isLaunching] or fade in during the first half of

     * the animation (if ![Controller.isLaunching]), and will have SRC blending mode (ultimately

     * punching a hole in the [transition container][Controller.transitionContainer]) iff [drawHole]

     * is true.

     * If [shouldFadeWindowBackgroundLayer] returns true, then this intermediary layer will fade out

     * during the second half of the animation (if [Controller.isLaunching] or fade in during the

     * first half of the animation (if ![Controller.isLaunching]), and will have SRC blending mode

     * (ultimately punching a hole in the [transition container][Controller.transitionContainer])

     * iff [drawHole] is true.

     *

     * TODO(b/397646693): remove drawHole altogether.

     *
@@ -522,7 +522,7 @@ class TransitionAnimator( 
        controller: Controller,

        endState: State,

        windowBackgroundColor: Int,

        fadeWindowBackgroundLayer: Boolean = true,

        shouldFadeWindowBackgroundLayer: () -> Boolean = { true },

        drawHole: Boolean = false,

        startVelocity: PointF? = null,

        startFrameTime: Long = -1,
@@ -545,7 +545,7 @@ class TransitionAnimator( 
                controller.createAnimatorState(),

                endState,

                windowBackgroundLayer,

                fadeWindowBackgroundLayer,

                shouldFadeWindowBackgroundLayer,

                drawHole,

                startVelocity,

                startFrameTime,
@@ -559,7 +559,7 @@ class TransitionAnimator( 
        startState: State,

        endState: State,

        windowBackgroundLayer: GradientDrawable,

        fadeWindowBackgroundLayer: Boolean = true,

        shouldFadeWindowBackgroundLayer: () -> Boolean = { true },

        drawHole: Boolean = false,

        startVelocity: PointF? = null,

        startFrameTime: Long = -1,
@@ -591,7 +591,7 @@ class TransitionAnimator( 
                transitionContainerOverlay,

                openingWindowSyncView,

                openingWindowSyncViewOverlay,

                fadeWindowBackgroundLayer,

                shouldFadeWindowBackgroundLayer,

                drawHole,

                moveBackgroundLayerWhenAppVisibilityChanges,

            )
@@ -605,7 +605,7 @@ class TransitionAnimator( 
                transitionContainerOverlay,

                openingWindowSyncView,

                openingWindowSyncViewOverlay,

                fadeWindowBackgroundLayer,

                shouldFadeWindowBackgroundLayer,

                drawHole,

                moveBackgroundLayerWhenAppVisibilityChanges,

            )
@@ -625,7 +625,7 @@ class TransitionAnimator( 
        transitionContainerOverlay: ViewGroupOverlay,

        openingWindowSyncView: View? = null,

        openingWindowSyncViewOverlay: ViewOverlay? = null,

        fadeWindowBackgroundLayer: Boolean = true,

        shouldFadeWindowBackgroundLayer: () -> Boolean = { true },

        drawHole: Boolean = false,

        moveBackgroundLayerWhenAppVisibilityChanges: Boolean = false,

    ): Animation {
@@ -747,7 +747,7 @@ class TransitionAnimator( 
                state,

                linearProgress,

                container,

                fadeWindowBackgroundLayer,

                shouldFadeWindowBackgroundLayer,

                drawHole,

                controller.isLaunching,

                useSpring = false,
@@ -777,7 +777,7 @@ class TransitionAnimator( 
        transitionContainerOverlay: ViewGroupOverlay,

        openingWindowSyncView: View?,

        openingWindowSyncViewOverlay: ViewOverlay?,

        fadeWindowBackgroundLayer: Boolean = true,

        shouldFadeWindowBackgroundLayer: () -> Boolean = { true },

        drawHole: Boolean = false,

        moveBackgroundLayerWhenAppVisibilityChanges: Boolean = false,

    ): Animation {
@@ -869,7 +869,7 @@ class TransitionAnimator( 
                newState,

                state.scale,

                container,

                fadeWindowBackgroundLayer,

                shouldFadeWindowBackgroundLayer,

                drawHole,

                isLaunching = false,

                useSpring = true,
@@ -1124,7 +1124,7 @@ class TransitionAnimator( 
        state: State,

        linearProgress: Float,

        transitionContainer: View,

        fadeWindowBackgroundLayer: Boolean,

        shouldFadeWindowBackgroundLayer: () -> Boolean,

        drawHole: Boolean,

        isLaunching: Boolean,

        useSpring: Boolean,
@@ -1193,7 +1193,7 @@ class TransitionAnimator( 
                val alpha =

                    interpolators.contentBeforeFadeOutInterpolator.getInterpolation(fadeInProgress)

                drawable.alpha = (alpha * 0xFF).roundToInt()

            } else if (fadeWindowBackgroundLayer) {

            } else if (shouldFadeWindowBackgroundLayer()) {

                val alpha =

                    1 -

                        interpolators.contentAfterFadeInInterpolator.getInterpolation(
@@ -1212,7 +1212,7 @@ class TransitionAnimator( 
                drawable.alpha = 0xFF

            }

        } else {

            if (fadeInProgress < 1 && fadeWindowBackgroundLayer) {

            if (fadeInProgress < 1 && shouldFadeWindowBackgroundLayer()) {

                val alpha =

                    interpolators.contentBeforeFadeOutInterpolator.getInterpolation(fadeInProgress)

                drawable.alpha = (alpha * 0xFF).roundToInt()

packages/SystemUI/tests/src/com/android/systemui/animation/TransitionAnimatorTest.kt

+1 −1
Original line number Diff line number Diff line
@@ -209,7 +209,7 @@ class TransitionAnimatorTest( 
                controller.createAnimatorState(),

                endState,

                backgroundLayer,

                fadeWindowBackgroundLayer,

                { fadeWindowBackgroundLayer },

                drawHole,

                startVelocity = startVelocity,

            )