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

Commit 8eb5ac27 authored by Luca Zuccarini's avatar Luca Zuccarini Committed by Android (Google) Code Review
Browse files

Merge "Clean up the controller reference after execution." into main

parents a35c6afd c085f48b
Loading
Loading
Loading
Loading
+28 −20
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
package com.android.systemui.animation

import android.app.ActivityManager
import android.app.ActivityOptions
import android.app.ActivityTaskManager
import android.app.PendingIntent
import android.app.TaskInfo
@@ -434,8 +435,7 @@ constructor(
                    private fun cleanUp() {
                        cleanUpRunnable?.run()
                    }
                },
                initializeLazily = longLivedReturnAnimationsEnabled(),
                }
            )

        // mTypeSet and mModes match back signals only, and not home. This is on purpose, because
@@ -478,8 +478,8 @@ constructor(
    /** Create a new animation [Runner] controlled by [controller]. */
    @VisibleForTesting
    @JvmOverloads
    fun createRunner(controller: Controller, initializeLazily: Boolean = false): Runner {
        if (initializeLazily) assertLongLivedReturnAnimations()
    fun createRunner(controller: Controller, longLived: Boolean = false): Runner {
        if (longLived) assertLongLivedReturnAnimations()

        // Make sure we use the modified timings when animating a dialog into an app.
        val transitionAnimator =
@@ -489,13 +489,7 @@ constructor(
                transitionAnimator
            }

        return Runner(
            controller,
            callback!!,
            transitionAnimator,
            lifecycleListener,
            initializeLazily,
        )
        return Runner(controller, callback!!, transitionAnimator, lifecycleListener, longLived)
    }

    interface PendingIntentStarter {
@@ -699,7 +693,7 @@ constructor(
            }
        val launchRemoteTransition =
            RemoteTransition(
                OriginTransition(createRunner(controller, initializeLazily = true)),
                OriginTransition(createRunner(controller, longLived = true)),
                "${cookie}_launchTransition",
            )
        transitionRegister.register(launchFilter, launchRemoteTransition, includeTakeover = true)
@@ -721,7 +715,7 @@ constructor(
            }
        val returnRemoteTransition =
            RemoteTransition(
                OriginTransition(createRunner(returnController, initializeLazily = true)),
                OriginTransition(createRunner(returnController, longLived = true)),
                "${cookie}_returnTransition",
            )
        transitionRegister.register(returnFilter, returnRemoteTransition, includeTakeover = true)
@@ -910,14 +904,22 @@ constructor(

    @VisibleForTesting
    inner class Runner(
        private val controller: Controller,
        /**
         * This can hold a reference to a view, so it needs to be cleaned up and can't be held on to
         * forever when ![longLived].
         */
        private var controller: Controller?,
        private val callback: Callback,
        /** The animator to use to animate the window transition. */
        private val transitionAnimator: TransitionAnimator,
        /** Listener for animation lifecycle events. */
        private val listener: Listener? = null,
        /** Whether the internal [delegate] should be initialized lazily. */
        private val initializeLazily: Boolean = false,
        /**
         * Whether the internal should be kept around after execution for later usage. IMPORTANT:
         * should always be false if this [Runner] is to be used directly with [ActivityOptions]
         * (i.e. for ephemeral launches), or the controller will leak its view.
         */
        private val longLived: Boolean = false,
    ) : IRemoteAnimationRunner.Stub() {
        // This is being passed across IPC boundaries and cycles (through PendingIntentRecords,
        // etc.) are possible. So we need to make sure we drop any references that might
@@ -926,7 +928,7 @@ constructor(

        init {
            delegate = null
            if (!initializeLazily) {
            if (!longLived) {
                // Ephemeral launches bundle the runner with the launch request (instead of being
                // registered ahead of time for later use). This means that there could be a timeout
                // between creation and invocation, so the delegate needs to exist from the
@@ -1004,16 +1006,17 @@ constructor(

        @AnyThread
        private fun maybeSetUp() {
            if (!initializeLazily || delegate != null) return
            if (!longLived || delegate != null) return
            createDelegate()
        }

        @AnyThread
        private fun createDelegate() {
            if (controller == null) return
            delegate =
                AnimationDelegate(
                    mainExecutor,
                    controller,
                    controller!!,
                    callback,
                    DelegatingAnimationCompletionListener(listener, this::dispose),
                    transitionAnimator,
@@ -1025,7 +1028,12 @@ constructor(
        fun dispose() {
            // Drop references to animation controller once we're done with the animation
            // to avoid leaking.
            mainExecutor.execute { delegate = null }
            mainExecutor.execute {
                delegate = null
                // When long lived, the same Runner can be used more than once. In this case we need
                // to keep the controller around so we can rebuild the delegate on demand.
                if (!longLived) controller = null
            }
        }
    }

+5 −5
Original line number Diff line number Diff line
@@ -404,7 +404,7 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() {
    @Test
    fun creatingRunnerWithLazyInitializationThrows_whenTheFlagsAreDisabled() {
        assertThrows(IllegalStateException::class.java) {
            activityTransitionAnimator.createRunner(controller, initializeLazily = true)
            activityTransitionAnimator.createRunner(controller, longLived = true)
        }
    }

@@ -414,7 +414,7 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() {
    )
    @Test
    fun runnerCreatesDelegateLazily_whenPostingTimeouts() {
        val runner = activityTransitionAnimator.createRunner(controller, initializeLazily = true)
        val runner = activityTransitionAnimator.createRunner(controller, longLived = true)
        assertNull(runner.delegate)
        runner.postTimeouts()
        assertNotNull(runner.delegate)
@@ -426,7 +426,7 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() {
    )
    @Test
    fun runnerCreatesDelegateLazily_onAnimationStart() {
        val runner = activityTransitionAnimator.createRunner(controller, initializeLazily = true)
        val runner = activityTransitionAnimator.createRunner(controller, longLived = true)
        assertNull(runner.delegate)

        // The delegate is cleaned up after execution (which happens in another thread), so what we
@@ -458,7 +458,7 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() {
    )
    @Test
    fun runnerCreatesDelegateLazily_onAnimationTakeover() {
        val runner = activityTransitionAnimator.createRunner(controller, initializeLazily = true)
        val runner = activityTransitionAnimator.createRunner(controller, longLived = true)
        assertNull(runner.delegate)

        // The delegate is cleaned up after execution (which happens in another thread), so what we
@@ -489,7 +489,7 @@ class ActivityTransitionAnimatorTest : SysuiTestCase() {
    )
    @Test
    fun animationTakeoverThrows_whenTheFlagsAreDisabled() {
        val runner = activityTransitionAnimator.createRunner(controller, initializeLazily = false)
        val runner = activityTransitionAnimator.createRunner(controller, longLived = false)
        assertThrows(IllegalStateException::class.java) {
            runner.takeOverAnimation(
                arrayOf(fakeWindow()),