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

Commit c5dd8204 authored by Luca Zuccarini's avatar Luca Zuccarini
Browse files

Add a delay before the transition leash is reparents after a launch.

This cleanup doesn't need to happen immediately, because its only
purpose is to avoid leaks. By adding a delay we can be reasonably
confident that it happens after the transition is done (i.e. after
the finish callback has been called), which avoids flickers.

Bug: 356065603
Flag: com.android.systemui.animation_library_delay_leash_cleanup
Test: atest ActivityTransitionAnimator + manual flicker check + Winscope leash cleanup check
Change-Id: I6974e73e782bbd204b4d196680258a40a03e7cc3
parent 450ac0b8
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -1994,3 +1994,13 @@ flag {
        purpose: PURPOSE_BUGFIX
    }
}

flag {
    name: "animation_library_delay_leash_cleanup"
    namespace: "systemui"
    description: "Clean up the transition leash with a delay to ensure that the finish callback happens first, avoiding flickers"
    bug: "356065603"
    metadata {
        purpose: PURPOSE_BUGFIX
    }
}
+33 −13
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ import com.android.app.animation.Interpolators
import com.android.internal.annotations.VisibleForTesting
import com.android.internal.policy.ScreenDecorationsUtils
import com.android.systemui.Flags.activityTransitionUseLargestWindow
import com.android.systemui.Flags.animationLibraryDelayLeashCleanup
import com.android.systemui.Flags.moveTransitionAnimationLayer
import com.android.systemui.Flags.translucentOccludingActivityFix
import com.android.systemui.animation.TransitionAnimator.Companion.assertLongLivedReturnAnimations
@@ -1596,20 +1597,39 @@ constructor(
                        iCallback?.invoke()

                        if (reparent) {
                            val cleanUpTransitionLeash: () -> Unit = {
                                // Relying on this try-catch is not great, but the existence of
                            // RemoteAnimationRunnerCompat means that we can't reliably assume that
                            // the transaction will be executed while the leash is still valid.
                            // TODO(b/397180418): remove once the RemoteAnimation wrapper is cleaned
                            //  up.
                                // RemoteAnimationRunnerCompat means that we can't reliably
                                // assume that the transaction will be executed while the leash
                                // is still valid.
                                // TODO(b/397180418): remove once the RemoteAnimation wrapper is
                                //  cleaned up.
                                try {
                                    // Reparent to null to avoid leaking the transition leash.
                                    // TODO(b/397180418): shouldn't be needed anymore once the
                                    //  RemoteAnimation wrapper is cleaned up.
                                    SurfaceControl.Transaction().use {
                                    it.reparent(window.leash, null).apply()
                                        it.reparent(window.leash, null)
                                        it.apply(/* sync */ false)
                                    }
                                } catch (e: IllegalStateException) {
                                Log.e(TAG, "Failed to clean up transition leash: already released")
                                    Log.e(
                                        TAG,
                                        "Failed to clean up transition leash: already released",
                                    )
                                }
                            }
                            if (animationLibraryDelayLeashCleanup()) {
                                // This cleanup is not time-sensitive as it is just to avoid leaking
                                // leashes. By delaying it, we make (reasonably) sure that the
                                // finish callback above is executed before the reparent
                                // transaction, which avoids flaky flickers. Unfortunately both are
                                // async, so we need to resort to this hacky solution. Fortunately
                                // none of this will be necessary as soon as b/397180418 is done.
                                Handler(Looper.getMainLooper())
                                    .postDelayed(cleanUpTransitionLeash, 500L)
                            } else {
                                cleanUpTransitionLeash()
                            }
                        }