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

Commit c199efbe authored by Chet Haase's avatar Chet Haase
Browse files

DO NOT MERGE Revert "Fix activity leak bug"

Revert "Fix test cleanup issue in AnimatorLeakTest"

Revert submission 20610032-cherrypick-AnimatorLeak fixes-4lh72bu61o

Reason for revert: Startup regression found in post-submit
Reverted Changes:
Icdd6023f6:Fix activity leak bug
Ieac851550:Fix test cleanup issue in AnimatorLeakTest

This is a tentative revert to see whether it addresses the regression while I continue chasing the underlying problem

Bug: 261518932
Test: presubmit
Change-Id: Ica9a75b94c8c260cceb6719f8f6b95ba67d4e356
parent 93f2744c
Loading
Loading
Loading
Loading
+35 −40
Original line number Diff line number Diff line
@@ -19,10 +19,10 @@ package android.animation;
import android.os.SystemClock;
import android.os.SystemProperties;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
import android.view.Choreographer;

import java.lang.ref.WeakReference;
import java.util.ArrayList;

/**
@@ -40,7 +40,7 @@ import java.util.ArrayList;
public class AnimationHandler {

    private static final String TAG = "AnimationHandler";
    private static final boolean LOCAL_LOGV = false;
    private static final boolean LOCAL_LOGV = true;

    /**
     * Internal per-thread collections used to avoid set collisions as animations start and end
@@ -78,7 +78,7 @@ public class AnimationHandler {
     * 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 ArrayList<WeakReference<Object>> mAnimatorRequestors = new ArrayList<>();
    private final ArraySet<Object> mAnimatorRequestors = new ArraySet<>();

    private final Choreographer.FrameCallback mFrameCallback = new Choreographer.FrameCallback() {
        @Override
@@ -141,9 +141,24 @@ public class AnimationHandler {
     * tracking obsolete+enabled requestors.
     */
    public static void removeRequestor(Object requestor) {
        getInstance().requestAnimatorsEnabledImpl(false, requestor);
        getInstance().removeRequestorImpl(requestor);
    }

    private void removeRequestorImpl(Object requestor) {
        // Also request disablement, in case that requestor was the sole object keeping
        // animators un-paused
        long startTime = System.nanoTime();
        requestAnimatorsEnabled(false, requestor);
        Log.d(TAG, "removeRequestorImpl called requestAnimatorsEnabled after " + 
              (System.nanoTime() - startTime));
        mAnimatorRequestors.remove(requestor);
        Log.d(TAG, "removeRequestorImpl removed requestor after " + 
              (System.nanoTime() - startTime));
        if (LOCAL_LOGV) {
            Log.v(TAG, "removeRequestor for " + requestor);
            Log.v(TAG, "removeRequestorImpl for " + requestor);
            for (int i = 0; i < mAnimatorRequestors.size(); ++i) {
                Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i));
            }
        }
    }

@@ -161,44 +176,25 @@ public class AnimationHandler {
    }

    private void requestAnimatorsEnabledImpl(boolean enable, Object requestor) {
        long startTime = System.nanoTime();
        boolean wasEmpty = mAnimatorRequestors.isEmpty();
        setAnimatorPausingEnabled(isPauseBgAnimationsEnabledInSystemProperties());
        synchronized (mAnimatorRequestors) {
            // Only store WeakRef objects to avoid leaks
        Log.d(TAG, "requestAnimatorsEnabledImpl called setAnimatorPausingEnabled after " + 
              (System.nanoTime() - startTime));
        if (enable) {
                // First, check whether such a reference is already on the list
                WeakReference<Object> weakRef = null;
                for (int i = mAnimatorRequestors.size() - 1; i >= 0; --i) {
                    WeakReference<Object> ref = mAnimatorRequestors.get(i);
                    Object referent = ref.get();
                    if (referent == requestor) {
                        weakRef = ref;
                    } else if (referent == null) {
                        // Remove any reference that has been cleared
                        mAnimatorRequestors.remove(i);
                    }
                }
                if (weakRef == null) {
                    weakRef = new WeakReference<>(requestor);
                    mAnimatorRequestors.add(weakRef);
                }
            mAnimatorRequestors.add(requestor);
        } else {
                for (int i = mAnimatorRequestors.size() - 1; i >= 0; --i) {
                    WeakReference<Object> ref = mAnimatorRequestors.get(i);
                    Object referent = ref.get();
                    if (referent == requestor || referent == null) {
                        // remove requested item or item that has been cleared
                        mAnimatorRequestors.remove(i);
                    }
                }
                // If a reference to the requestor wasn't in the list, nothing to remove
            }
            mAnimatorRequestors.remove(requestor);
        }
        Log.d(TAG, "requestAnimatorsEnabledImpl added/removed after " + 
              (System.nanoTime() - startTime));
        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();
            Log.d(TAG, "requestAnimatorsEnabledImpl resumed, returning after " + 
                  (System.nanoTime() - startTime));
            return;
        }
        boolean isEmpty = mAnimatorRequestors.isEmpty();
@@ -213,13 +209,12 @@ public class AnimationHandler {
                        Animator.getBackgroundPauseDelay());
            }
        }
        Log.d(TAG, "requestAnimatorsEnabledImpl post was/is check after " + 
              (System.nanoTime() - startTime));
        if (LOCAL_LOGV) {
            Log.v(TAG, (enable ? "enable" : "disable") + " animators for " + requestor
                    + " with pauseDelay of " + Animator.getBackgroundPauseDelay());
            Log.v(TAG, enable ? "enable" : "disable" + " animators for " + requestor);
            for (int i = 0; i < mAnimatorRequestors.size(); ++i) {
                Log.v(TAG, "animatorRequestors " + i + " = "
                        + mAnimatorRequestors.get(i) + " with referent "
                        + mAnimatorRequestors.get(i).get());
                Log.v(TAG, "animatorRequesters " + i + " = " + mAnimatorRequestors.valueAt(i));
            }
        }
    }
+2 −11
Original line number Diff line number Diff line
@@ -1409,11 +1409,7 @@ public final class ViewRootImpl implements ViewParent,
                mFirstPostImeInputStage = earlyPostImeStage;
                mPendingInputEventQueueLengthCounterName = "aq:pending:" + counterSuffix;

                if (!mRemoved || !mAppVisible) {
                AnimationHandler.requestAnimatorsEnabled(mAppVisible, this);
                } else if (LOCAL_LOGV) {
                    Log.v(mTag, "setView() enabling visibility when removed");
                }
            }
        }
    }
@@ -1751,12 +1747,7 @@ public final class ViewRootImpl implements ViewParent,
            if (!mAppVisible) {
                WindowManagerGlobal.trimForeground();
            }
            // Only enable if the window is not already removed (via earlier call to doDie())
            if (!mRemoved || !mAppVisible) {
            AnimationHandler.requestAnimatorsEnabled(mAppVisible, this);
            } else if (LOCAL_LOGV) {
                Log.v(mTag, "handleAppVisibility() enabling visibility when removed");
            }
        }
    }