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

Commit 859ccdbd authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Ensure that launched Views implement LaunchableView 

This CL throws an exception when a GhostedViewLaunchAnimatorController,
ActivityLaunchAnimator.Controller or DialogLaunchAnimator.Controller is
created for a View that does not implement LaunchableView, which can
lead to unexpected visibility bugs if the View visibility is changed
during a launch animation.

Bug: 243636422
Test: DialogLaunchAnimatorTest
Test: ActivityLaunchAnimatorTest
Test: GhostedViewLaunchAnimatorControllerTest
Test: Launched all dialogs and activities animated by DialogLaunchAnimator and ActivityLaunchAnimator
Change-Id: I1269089d3b1473e838d75b5b63659c2907493c0c
parent 6dead15c

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

+13 −1
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ import androidx.annotation.BinderThread 
import androidx.annotation.UiThread

import com.android.internal.annotations.VisibleForTesting

import com.android.internal.policy.ScreenDecorationsUtils

import java.lang.IllegalArgumentException

import kotlin.math.roundToInt



private const val TAG = "ActivityLaunchAnimator"
@@ -338,13 +339,24 @@ class ActivityLaunchAnimator( 
             * Return a [Controller] that will animate and expand [view] into the opening window.

             *

             * Important: The view must be attached to a [ViewGroup] when calling this function and

             * during the animation. For safety, this method will return null when it is not.

             * during the animation. For safety, this method will return null when it is not. The

             * view must also implement [LaunchableView], otherwise this method will throw.

             *

             * Note: The background of [view] should be a (rounded) rectangle so that it can be

             * properly animated.

             */

            @JvmStatic

            fun fromView(view: View, cujType: Int? = null): Controller? {

                // Make sure the View we launch from implements LaunchableView to avoid visibility

                // issues.

                if (view !is LaunchableView) {

                    throw IllegalArgumentException(

                        "An ActivityLaunchAnimator.Controller was created from a View that does " +

                            "not implement LaunchableView. This can lead to subtle bugs where the" +

                            " visibility of the View we are launching from is not what we expected."

                    )

                }



                if (view.parent !is ViewGroup) {

                    Log.e(

                        TAG,

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

+27 −23
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ import com.android.systemui.animation.back.BackAnimationSpec 
import com.android.systemui.animation.back.applyTo

import com.android.systemui.animation.back.floatingSystemSurfacesForSysUi

import com.android.systemui.animation.back.onBackAnimationCallbackFrom

import java.lang.IllegalArgumentException

import kotlin.math.roundToInt



private const val TAG = "DialogLaunchAnimator"
@@ -157,12 +158,23 @@ constructor( 
             * Create a [Controller] that can animate [source] to and from a dialog.

             *

             * Important: The view must be attached to a [ViewGroup] when calling this function and

             * during the animation. For safety, this method will return null when it is not.

             * during the animation. For safety, this method will return null when it is not. The

             * view must also implement [LaunchableView], otherwise this method will throw.

             *

             * Note: The background of [view] should be a (rounded) rectangle so that it can be

             * properly animated.

             */

            fun fromView(source: View, cuj: DialogCuj? = null): Controller? {

                // Make sure the View we launch from implements LaunchableView to avoid visibility

                // issues.

                if (source !is LaunchableView) {

                    throw IllegalArgumentException(

                        "A DialogLaunchAnimator.Controller was created from a View that does not " +

                            "implement LaunchableView. This can lead to subtle bugs where the " +

                            "visibility of the View we are launching from is not what we expected."

                    )

                }



                if (source.parent !is ViewGroup) {

                    Log.e(

                        TAG,
@@ -249,23 +261,6 @@ constructor( 
            }

                ?: controller



        if (

            animatedParent == null &&

                controller is ViewDialogLaunchAnimatorController &&

                controller.source !is LaunchableView

        ) {

            // Make sure the View we launch from implements LaunchableView to avoid visibility

            // issues. Given that we don't own dialog decorViews so we can't enforce it for launches

            // from a dialog.

            // TODO(b/243636422): Throw instead of logging to enforce this.

            Log.w(

                TAG,

                "A dialog was launched from a View that does not implement LaunchableView. This " +

                    "can lead to subtle bugs where the visibility of the View we are " +

                    "launching from is not what we expected."

            )

        }



        // Make sure we don't run the launch animation from the same source twice at the same time.

        if (openedDialogs.any { it.controller.sourceIdentity == controller.sourceIdentity }) {

            Log.e(
@@ -613,10 +608,16 @@ private class AnimatedDialog( 
                }



                // Animate that view with the background. Throw if we didn't find one, because

                // otherwise

                // it's not clear what we should animate.

                // otherwise it's not clear what we should animate.

                if (viewGroupWithBackground == null) {

                    error("Unable to find ViewGroup with background")

                }



                if (viewGroupWithBackground !is LaunchableView) {

                    error("The animated ViewGroup with background must implement LaunchableView")

                }



                viewGroupWithBackground

                    ?: throw IllegalStateException("Unable to find ViewGroup with background")

            } else {

                // We will make the dialog window (and therefore its DecorView) fullscreen to make

                // it possible to animate outside its bounds.
@@ -639,7 +640,7 @@ private class AnimatedDialog( 
                    FrameLayout.LayoutParams(MATCH_PARENT, MATCH_PARENT)

                )



                val dialogContentWithBackground = FrameLayout(dialog.context)

                val dialogContentWithBackground = LaunchableFrameLayout(dialog.context)

                dialogContentWithBackground.background = decorView.background



                // Make the window background transparent. Note that setting the window (or
@@ -720,7 +721,10 @@ private class AnimatedDialog( 


        // Make the background view invisible until we start the animation. We use the transition

        // visibility like GhostView does so that we don't mess up with the accessibility tree (see

        // b/204944038#comment17).

        // b/204944038#comment17). Given that this background implements LaunchableView, we call

        // setShouldBlockVisibilityChanges() early so that the current visibility (VISIBLE) is

        // restored at the end of the animation.

        dialogContentWithBackground.setShouldBlockVisibilityChanges(true)

        dialogContentWithBackground.setTransitionVisibility(View.INVISIBLE)



        // Make sure the dialog is visible instantly and does not do any window animation.

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

+12 −1
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import android.view.ViewGroup 
import android.view.ViewGroupOverlay

import android.widget.FrameLayout

import com.android.internal.jank.InteractionJankMonitor

import java.lang.IllegalArgumentException

import java.util.LinkedList

import kotlin.math.min

import kotlin.math.roundToInt
@@ -46,7 +47,8 @@ private const val TAG = "GhostedViewLaunchAnimatorController" 
 * of the ghosted view.

 *

 * Important: [ghostedView] must be attached to a [ViewGroup] when calling this function and during

 * the animation.

 * the animation. It must also implement [LaunchableView], otherwise an exception will be thrown

 * during this controller instantiation.

 *

 * Note: Avoid instantiating this directly and call [ActivityLaunchAnimator.Controller.fromView]

 * whenever possible instead.
@@ -101,6 +103,15 @@ constructor( 
    private val background: Drawable?



    init {

        // Make sure the View we launch from implements LaunchableView to avoid visibility issues.

        if (ghostedView !is LaunchableView) {

            throw IllegalArgumentException(

                "A GhostedViewLaunchAnimatorController was created from a View that does not " +

                    "implement LaunchableView. This can lead to subtle bugs where the visibility " +

                    "of the View we are launching from is not what we expected."

            )

        }



        /** Find the first view with a background in [view] and its children. */

        fun findBackground(view: View): Drawable? {

            if (view.background != null) {

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

0 → 100644
+53 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.animation



import android.content.Context

import android.util.AttributeSet

import android.widget.FrameLayout



/** A [FrameLayout] that also implements [LaunchableView]. */

open class LaunchableFrameLayout : FrameLayout, LaunchableView {

    private val delegate =

        LaunchableViewDelegate(

            this,

            superSetVisibility = { super.setVisibility(it) },

        )



    constructor(context: Context) : super(context)

    constructor(context: Context, attrs: AttributeSet?) : super(context, attrs)

    constructor(

        context: Context,

        attrs: AttributeSet?,

        defStyleAttr: Int

    ) : super(context, attrs, defStyleAttr)



    constructor(

        context: Context,

        attrs: AttributeSet?,

        defStyleAttr: Int,

        defStyleRes: Int

    ) : super(context, attrs, defStyleAttr, defStyleRes)



    override fun setShouldBlockVisibilityChanges(block: Boolean) {

        delegate.setShouldBlockVisibilityChanges(block)

    }



    override fun setVisibility(visibility: Int) {

        delegate.setVisibility(visibility)

    }

}

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

+1 −1
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ import com.android.internal.jank.InteractionJankMonitor 
/** A [DialogLaunchAnimator.Controller] that can animate a [View] from/to a dialog. */

class ViewDialogLaunchAnimatorController

internal constructor(

    internal val source: View,

    private val source: View,

    override val cuj: DialogCuj?,

) : DialogLaunchAnimator.Controller {

    override val viewRoot: ViewRootImpl?