From b99cc5b4c85ea1688b87d91c1d20439e65ff8c79 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Mon, 2 May 2022 22:41:53 +0000 Subject: [PATCH 1/4] Pause animators when app is not visible Because animators are not tied to the lifecycle of any UI elements, it is possible for an app to go into the background and for the animators to continue running. Ideally, the app would track the lifecycle of the activity/etc and pause or disable the animators, but it is common for this to not happen, causing the animators to continue spinning when the app does not need them. The animators are not causing as much work as for a foreground activity (since they do not cause any re-rendering), but they cause work nonetheless by keeping Choreographer awake to continue pulsing frames. The ideal fix would be to introduce new API for animators that tied them to lifecycle concepts (View, Activity, etc). But that kind of fix would only be available for future versions of the platform, and does not address existing app code. A workaround for the current situation is to address the most egregious problems; infinite animators running on backgrounded apps. The fix here is exactly that: when an app's visible surface (either an activity or, for Wallpapers, a WallpaperService) is backgrounded, a request is sent to pause animators for that surface. When that surface comes to the foreground, a request is sent to resume those animators. Since all animators are handled on the same thread for the same process, in AnimationHandler, we should only ever pause animators when *all* surfaces for a process are not visible (and resume them when *any* surface becomes visible). Also, to mitigate any issues with thrashing animator state for apps which become only transiently backgrounded, we delay pausing for some time. Bug: 228598053 Bug: 233391022 Test: new AnimatorLeak CTS test, plus manual testing for activities and wallpapers Change-Id: I8b9f841cc80babb972244c724968a5c085a06b69 Merged-In: I8b9f841cc80babb972244c724968a5c085a06b69 --- core/api/test-current.txt | 5 + .../android/animation/AnimationHandler.java | 110 +++++++++++++++++- core/java/android/animation/Animator.java | 29 +++++ .../service/wallpaper/WallpaperService.java | 6 + core/java/android/view/ViewRootImpl.java | 5 + 5 files changed, 154 insertions(+), 1 deletion(-) diff --git a/core/api/test-current.txt b/core/api/test-current.txt index 48277fb3b488c..e5a7bc288560e 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -96,6 +96,11 @@ package android.accessibilityservice { package android.animation { + public abstract class Animator implements java.lang.Cloneable { + method public static long getBackgroundPauseDelay(); + method public static void setBackgroundPauseDelay(long); + } + public class ValueAnimator extends android.animation.Animator { method @MainThread public static void setDurationScale(@FloatRange(from=0) float); } diff --git a/core/java/android/animation/AnimationHandler.java b/core/java/android/animation/AnimationHandler.java index 260323fe2d10c..7f6df2261fcc8 100644 --- a/core/java/android/animation/AnimationHandler.java +++ b/core/java/android/animation/AnimationHandler.java @@ -18,6 +18,8 @@ package android.animation; import android.os.SystemClock; import android.util.ArrayMap; +import android.util.ArraySet; +import android.util.Log; import android.view.Choreographer; import java.util.ArrayList; @@ -35,10 +37,13 @@ import java.util.ArrayList; * @hide */ public class AnimationHandler { + + private static final String TAG = "AnimationHandler"; + private static final boolean LOCAL_LOGV = true; + /** * Internal per-thread collections used to avoid set collisions as animations start and end * while being processed. - * @hide */ private final ArrayMap mDelayedCallbackStartTime = new ArrayMap<>(); @@ -48,6 +53,26 @@ public class AnimationHandler { new ArrayList<>(); private AnimationFrameCallbackProvider mProvider; + /** + * This paused list is used to store animators forcibly paused when the activity + * went into the background (to avoid unnecessary background processing work). + * These animators should be resume()'d when the activity returns to the foreground. + */ + private final ArrayList mPausedAnimators = new ArrayList<>(); + + /** + * This structure is used to store the currently active objects (ViewRootImpls or + * WallpaperService.Engines) in the process. Each of these objects sends a request to + * AnimationHandler when it goes into the background (request to pause) or foreground + * (request to resume). Because all animators are managed by AnimationHandler on the same + * thread, it should only ever pause animators when *all* requestors are in the background. + * This list tracks the background/foreground state of all requestors and only ever + * pauses animators when all items are in the background (false). To simplify, we only ever + * store visible (foreground) requestors; if the set size reaches zero, there are no + * objects in the foreground and it is time to pause animators. + */ + private final ArraySet mAnimatorRequestors = new ArraySet<>(); + private final Choreographer.FrameCallback mFrameCallback = new Choreographer.FrameCallback() { @Override public void doFrame(long frameTimeNanos) { @@ -68,6 +93,89 @@ public class AnimationHandler { return sAnimatorHandler.get(); } + + /** + * This is called when a window goes away. We should remove + * it from the requestors list to ensure that we are counting requests correctly and not + * tracking obsolete+enabled requestors. + */ + public static void removeRequestor(Object requestor) { + getInstance().removeRequestorImpl(requestor); + } + + private void removeRequestorImpl(Object requestor) { + // Also request disablement, in case that requestor was the sole object keeping + // animators un-paused + requestAnimatorsEnabled(false, requestor); + mAnimatorRequestors.remove(requestor); + if (LOCAL_LOGV) { + Log.v(TAG, "removeRequestorImpl for " + requestor); + for (int i = 0; i < mAnimatorRequestors.size(); ++i) { + Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i)); + } + } + } + + /** + * This method is called from ViewRootImpl or WallpaperService when either a window is no + * longer visible (enable == false) or when a window becomes visible (enable == true). + * If animators are not properly disabled when activities are backgrounded, it can lead to + * unnecessary processing, particularly for infinite animators, as the system will continue + * to pulse timing events even though the results are not visible. As a workaround, we + * pause all un-paused infinite animators, and resume them when any window in the process + * becomes visible. + */ + public static void requestAnimatorsEnabled(boolean enable, Object requestor) { + getInstance().requestAnimatorsEnabledImpl(enable, requestor); + } + + private void requestAnimatorsEnabledImpl(boolean enable, Object requestor) { + boolean wasEmpty = mAnimatorRequestors.isEmpty(); + if (enable) { + mAnimatorRequestors.add(requestor); + } else { + mAnimatorRequestors.remove(requestor); + } + boolean isEmpty = mAnimatorRequestors.isEmpty(); + if (wasEmpty != isEmpty) { + // only paused/resume animators if there was a visibility change + if (!isEmpty) { + // If any requestors are enabled, resume currently paused animators + Choreographer.getInstance().removeFrameCallback(mPauser); + for (int i = mPausedAnimators.size() - 1; i >= 0; --i) { + mPausedAnimators.get(i).resume(); + } + mPausedAnimators.clear(); + } else { + // Wait before pausing to avoid thrashing animator state for temporary backgrounding + Choreographer.getInstance().postFrameCallbackDelayed(mPauser, + Animator.getBackgroundPauseDelay()); + } + } + if (LOCAL_LOGV) { + Log.v(TAG, enable ? "enable" : "disable" + " animators for " + requestor); + for (int i = 0; i < mAnimatorRequestors.size(); ++i) { + Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i)); + } + } + } + + private Choreographer.FrameCallback mPauser = frameTimeNanos -> { + if (mAnimatorRequestors.size() > 0) { + // something enabled animators since this callback was scheduled - bail + return; + } + for (int i = 0; i < mAnimationCallbacks.size(); ++i) { + Animator animator = ((Animator) mAnimationCallbacks.get(i)); + if (animator != null + && animator.getTotalDuration() == Animator.DURATION_INFINITE + && !animator.isPaused()) { + mPausedAnimators.add(animator); + animator.pause(); + } + } + }; + /** * By default, the Choreographer is used to provide timing for frame callbacks. A custom * provider can be used here to provide different timing pulse. diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index a8ff36aae0980..9e55d359b416f 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -18,6 +18,7 @@ package android.animation; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.TestApi; import android.compat.annotation.UnsupportedAppUsage; import android.content.pm.ActivityInfo.Config; import android.content.res.ConstantState; @@ -63,6 +64,34 @@ public abstract class Animator implements Cloneable { */ private AnimatorConstantState mConstantState; + /** + * backing field for backgroundPauseDelay property. This could be simply a hardcoded + * value in AnimationHandler, but it is useful to be able to change the value in tests. + */ + private static long sBackgroundPauseDelay = 10000; + + /** + * Sets the duration for delaying pausing animators when apps go into the background. + * Used by AnimationHandler when requested to pause animators. + * + * @hide + */ + @TestApi + public static void setBackgroundPauseDelay(long value) { + sBackgroundPauseDelay = value; + } + + /** + * Gets the duration for delaying pausing animators when apps go into the background. + * Used by AnimationHandler when requested to pause animators. + * + * @hide + */ + @TestApi + public static long getBackgroundPauseDelay() { + return sBackgroundPauseDelay; + } + /** * Starts this animation. If the animation has a nonzero startDelay, the animation will start * running after that delay elapses. A non-delayed animation will have its initial diff --git a/core/java/android/service/wallpaper/WallpaperService.java b/core/java/android/service/wallpaper/WallpaperService.java index d598017dacaa0..1e22856c1bde0 100644 --- a/core/java/android/service/wallpaper/WallpaperService.java +++ b/core/java/android/service/wallpaper/WallpaperService.java @@ -27,6 +27,7 @@ import static android.view.View.SYSTEM_UI_FLAG_VISIBLE; import static android.view.ViewRootImpl.LOCAL_LAYOUT; import static android.view.WindowManager.LayoutParams.TYPE_WALLPAPER; +import android.animation.AnimationHandler; import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.animation.ValueAnimator; @@ -1516,6 +1517,8 @@ public abstract class WallpaperService extends Service { mVisible = visible; reportVisibility(); if (mReportedVisible) processLocalColors(mPendingXOffset, mPendingXOffsetStep); + } else { + AnimationHandler.requestAnimatorsEnabled(visible, this); } } @@ -1544,6 +1547,7 @@ public abstract class WallpaperService extends Service { if (DEBUG) Log.v(TAG, "Freezing wallpaper after visibility update"); freeze(); } + AnimationHandler.requestAnimatorsEnabled(visible, this); } } } @@ -2072,6 +2076,8 @@ public abstract class WallpaperService extends Service { return; } + AnimationHandler.removeRequestor(this); + mDestroyed = true; if (mIWallpaperEngine.mDisplayManager != null) { diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 127c7b7a8dc94..b755f5325eeb8 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -90,6 +90,7 @@ import static android.view.inputmethod.InputMethodEditorTraceProto.InputMethodCl import static android.view.inputmethod.InputMethodEditorTraceProto.InputMethodClientsTraceProto.ClientSideProto.INSETS_CONTROLLER; import android.Manifest; +import android.animation.AnimationHandler; import android.animation.LayoutTransition; import android.annotation.AnyThread; import android.annotation.NonNull; @@ -1363,6 +1364,8 @@ public final class ViewRootImpl implements ViewParent, mFirstInputStage = nativePreImeStage; mFirstPostImeInputStage = earlyPostImeStage; mPendingInputEventQueueLengthCounterName = "aq:pending:" + counterSuffix; + + AnimationHandler.requestAnimatorsEnabled(mAppVisible, this); } } } @@ -1708,6 +1711,7 @@ public final class ViewRootImpl implements ViewParent, if (!mAppVisible) { WindowManagerGlobal.trimForeground(); } + AnimationHandler.requestAnimatorsEnabled(mAppVisible, this); } } @@ -8477,6 +8481,7 @@ public final class ViewRootImpl implements ViewParent, mInsetsController.onControlsChanged(null); mAdded = false; + AnimationHandler.removeRequestor(this); } WindowManagerGlobal.getInstance().doRemoveView(this); } From 161732cf18d560b87de3b017df49b1fb995fc8d1 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Tue, 17 May 2022 22:13:24 +0000 Subject: [PATCH 2/4] Allow system to disable behavior of pausing animators for bg apps This change adds a static method which can be called to disable the default behavior of pausing infinite animators when an app's windows are all in the background. This could potentially be used for global behavior of a system property to disable this behavior system wide. Bug: 232937493 Bug: 233391022 Test: Added new cts test to AnimatorLeakTest to verify behavior Change-Id: Idf4957e3968253228096671fde89f820311883e3 Merged-In: Idf4957e3968253228096671fde89f820311883e3 --- core/api/test-current.txt | 1 + .../android/animation/AnimationHandler.java | 33 ++++++++++++++++--- core/java/android/animation/Animator.java | 14 ++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/core/api/test-current.txt b/core/api/test-current.txt index e5a7bc288560e..1796c7b971a9a 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -98,6 +98,7 @@ package android.animation { public abstract class Animator implements java.lang.Cloneable { method public static long getBackgroundPauseDelay(); + method public static void setAnimatorPausingEnabled(boolean); method public static void setBackgroundPauseDelay(long); } diff --git a/core/java/android/animation/AnimationHandler.java b/core/java/android/animation/AnimationHandler.java index 7f6df2261fcc8..57ab55a53e468 100644 --- a/core/java/android/animation/AnimationHandler.java +++ b/core/java/android/animation/AnimationHandler.java @@ -53,6 +53,9 @@ public class AnimationHandler { new ArrayList<>(); private AnimationFrameCallbackProvider mProvider; + // Static flag which allows the pausing behavior to be globally disabled/enabled. + private static boolean sAnimatorPausingEnabled = true; + /** * This paused list is used to store animators forcibly paused when the activity * went into the background (to avoid unnecessary background processing work). @@ -93,6 +96,15 @@ public class AnimationHandler { return sAnimatorHandler.get(); } + /** + * Disable the default behavior of pausing infinite animators when + * apps go into the background. + * + * @param enable Enable (default behavior) or disable background pausing behavior. + */ + public static void setAnimatorPausingEnabled(boolean enable) { + sAnimatorPausingEnabled = enable; + } /** * This is called when a window goes away. We should remove @@ -136,16 +148,19 @@ public class AnimationHandler { } else { mAnimatorRequestors.remove(requestor); } + if (!sAnimatorPausingEnabled) { + // Resume any animators that have been paused in the meantime, otherwise noop + // Leave logic above so that if pausing gets re-enabled, the state of the requestors + // list is valid + resumeAnimators(); + return; + } boolean isEmpty = mAnimatorRequestors.isEmpty(); if (wasEmpty != isEmpty) { // only paused/resume animators if there was a visibility change if (!isEmpty) { // If any requestors are enabled, resume currently paused animators - Choreographer.getInstance().removeFrameCallback(mPauser); - for (int i = mPausedAnimators.size() - 1; i >= 0; --i) { - mPausedAnimators.get(i).resume(); - } - mPausedAnimators.clear(); + resumeAnimators(); } else { // Wait before pausing to avoid thrashing animator state for temporary backgrounding Choreographer.getInstance().postFrameCallbackDelayed(mPauser, @@ -160,6 +175,14 @@ public class AnimationHandler { } } + private void resumeAnimators() { + Choreographer.getInstance().removeFrameCallback(mPauser); + for (int i = mPausedAnimators.size() - 1; i >= 0; --i) { + mPausedAnimators.get(i).resume(); + } + mPausedAnimators.clear(); + } + private Choreographer.FrameCallback mPauser = frameTimeNanos -> { if (mAnimatorRequestors.size() > 0) { // something enabled animators since this callback was scheduled - bail diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index 9e55d359b416f..f69decb087f31 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -92,6 +92,20 @@ public abstract class Animator implements Cloneable { return sBackgroundPauseDelay; } + /** + * Sets the behavior of animator pausing when apps go into the background. + * This is exposed as a test API for verification, but is intended for use by internal/ + * platform code, potentially for use by a system property that could disable it + * system wide. + * + * @param enable Enable (default behavior) or disable background pausing behavior. + * @hide + */ + @TestApi + public static void setAnimatorPausingEnabled(boolean enable) { + AnimationHandler.setAnimatorPausingEnabled(enable); + } + /** * Starts this animation. If the animation has a nonzero startDelay, the animation will start * running after that delay elapses. A non-delayed animation will have its initial From 506ca2410e9b97020ac5696aafd2457259d81772 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Tue, 17 May 2022 13:15:55 +0000 Subject: [PATCH 3/4] Disable debug logging in AnimationHandler LOCAL_LOGV should be set to false Bug: 232914479 Bug: 233391022 Test: Manually tested to ensure logging is not happening Change-Id: Ie14cf3c0bacb50dd1e7422dc378d5195e4d9bec5 Merged-In: Ie14cf3c0bacb50dd1e7422dc378d5195e4d9bec5 --- core/java/android/animation/AnimationHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/animation/AnimationHandler.java b/core/java/android/animation/AnimationHandler.java index 57ab55a53e468..1cb2574ae8b75 100644 --- a/core/java/android/animation/AnimationHandler.java +++ b/core/java/android/animation/AnimationHandler.java @@ -39,7 +39,7 @@ import java.util.ArrayList; public class AnimationHandler { private static final String TAG = "AnimationHandler"; - private static final boolean LOCAL_LOGV = true; + private static final boolean LOCAL_LOGV = false; /** * Internal per-thread collections used to avoid set collisions as animations start and end From 063ae10a70aa27438f275e652e877897037aec83 Mon Sep 17 00:00:00 2001 From: Steven Terrell Date: Wed, 25 May 2022 19:05:19 +0000 Subject: [PATCH 4/4] Use System Property to Control Animator Pausing This change sets the default behavior of animator pausing by checking if a system property is set and useing that value if present else it defaults to true. This should allow the use of build properties to be able to toggle the behavior. Bug: 233391022 Test: Manual, adding logging to verify the values being set by the call to the sysprop library. Ignore-AOSP-First: Testing internal change, will cherry pick to AOSP. Change-Id: Ib1b34585c564bf4f310441c6856412a798022900 --- .../android/animation/AnimationHandler.java | 32 ++++++++++++++++++- core/java/android/animation/Animator.java | 1 + 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/core/java/android/animation/AnimationHandler.java b/core/java/android/animation/AnimationHandler.java index 1cb2574ae8b75..7e814af3451da 100644 --- a/core/java/android/animation/AnimationHandler.java +++ b/core/java/android/animation/AnimationHandler.java @@ -17,6 +17,7 @@ package android.animation; import android.os.SystemClock; +import android.os.SystemProperties; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; @@ -54,7 +55,10 @@ public class AnimationHandler { private AnimationFrameCallbackProvider mProvider; // Static flag which allows the pausing behavior to be globally disabled/enabled. - private static boolean sAnimatorPausingEnabled = true; + private static boolean sAnimatorPausingEnabled = isPauseBgAnimationsEnabledInSystemProperties(); + + // Static flag which prevents the system property from overriding sAnimatorPausingEnabled field. + private static boolean sOverrideAnimatorPausingSystemProperty = false; /** * This paused list is used to store animators forcibly paused when the activity @@ -96,6 +100,18 @@ public class AnimationHandler { return sAnimatorHandler.get(); } + /** + * System property that controls the behavior of pausing infinite animators when an app + * is moved to the background. + * + * @return the value of 'framework.pause_bg_animations.enabled' system property + */ + private static boolean isPauseBgAnimationsEnabledInSystemProperties() { + if (sOverrideAnimatorPausingSystemProperty) return sAnimatorPausingEnabled; + return SystemProperties + .getBoolean("framework.pause_bg_animations.enabled", true); + } + /** * Disable the default behavior of pausing infinite animators when * apps go into the background. @@ -106,6 +122,19 @@ public class AnimationHandler { sAnimatorPausingEnabled = enable; } + /** + * Prevents the setAnimatorPausingEnabled behavior from being overridden + * by the 'framework.pause_bg_animations.enabled' system property value. + * + * This is for testing purposes only. + * + * @param enable Enable or disable (default behavior) overriding the system + * property. + */ + public static void setOverrideAnimatorPausingSystemProperty(boolean enable) { + sOverrideAnimatorPausingSystemProperty = enable; + } + /** * This is called when a window goes away. We should remove * it from the requestors list to ensure that we are counting requests correctly and not @@ -143,6 +172,7 @@ public class AnimationHandler { private void requestAnimatorsEnabledImpl(boolean enable, Object requestor) { boolean wasEmpty = mAnimatorRequestors.isEmpty(); + setAnimatorPausingEnabled(isPauseBgAnimationsEnabledInSystemProperties()); if (enable) { mAnimatorRequestors.add(requestor); } else { diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index f69decb087f31..a9d14df8bcf4d 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -104,6 +104,7 @@ public abstract class Animator implements Cloneable { @TestApi public static void setAnimatorPausingEnabled(boolean enable) { AnimationHandler.setAnimatorPausingEnabled(enable); + AnimationHandler.setOverrideAnimatorPausingSystemProperty(!enable); } /**