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

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

Fix leak with transitions when views get removed

Transitions, when started, add an OnPreDrawListener to the current
ViewTreeObserver (which is global to the view hierarchy). This listener
is removed when the listener is called.

It is possible to add this listener and then remove the view from
the hierarchy before the listener is called. This could result in
either the listener not getting called at all (since there was no
drawing event) or (in the case of this bug) the listener getting called
when the sceneRoot had no AttachInfo (which is the case when that
root has been removed from the hierarchy). This results in the listener
trying to remove itself from a *different* ViewTreeObserver than the one
it added itself to, leaving the actual listener still sitting on a list
of listeners in that original VTO. This can result in a growing list of
listeners and a growing amount of work that gets done on every frame.
It can also lead to a serious memory leak, since the objects referred to
by the transition may be non-trivial (as in the case of this bug).

The fix is to add another mechanism for the listener to get removed.
Specifically, we now listen for detach events on the sceneRoot. If that
view gets detached before the listener is called, then we have a chance to
remove it from the correct VTO before the AttachInfo becomes null.

Issue #11307391 keyguard is slow after updating to krt16c and playing music

Change-Id: I108413ea2f18f5351df0a11d4ae56fec0b4aa154
parent 24ba3234
Loading
Loading
Loading
Loading
+18 −9
Original line number Diff line number Diff line
@@ -994,15 +994,7 @@ public abstract class Transition implements Cloneable {
     * false otherwise
     */
    void captureValues(ViewGroup sceneRoot, boolean start) {
        if (start) {
            mStartValues.viewValues.clear();
            mStartValues.idValues.clear();
            mStartValues.itemIdValues.clear();
        } else {
            mEndValues.viewValues.clear();
            mEndValues.idValues.clear();
            mEndValues.itemIdValues.clear();
        }
        clearValues(start);
        if (mTargetIds.size() > 0 || mTargets.size() > 0) {
            if (mTargetIds.size() > 0) {
                for (int i = 0; i < mTargetIds.size(); ++i) {
@@ -1054,6 +1046,23 @@ public abstract class Transition implements Cloneable {
        }
    }

    /**
     * Clear valuesMaps for specified start/end state
     *
     * @param start true if the start values should be cleared, false otherwise
     */
    void clearValues(boolean start) {
        if (start) {
            mStartValues.viewValues.clear();
            mStartValues.idValues.clear();
            mStartValues.itemIdValues.clear();
        } else {
            mEndValues.viewValues.clear();
            mEndValues.idValues.clear();
            mEndValues.itemIdValues.clear();
        }
    }

    /**
     * Recursive method which captures values for an entire view hierarchy,
     * starting at some root view. Transitions without targetIDs will use this
+82 −38
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package android.transition;
import android.content.Context;
import android.util.ArrayMap;
import android.util.Log;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewTreeObserver;

@@ -205,47 +206,90 @@ public class TransitionManager {

    private static void sceneChangeRunTransition(final ViewGroup sceneRoot,
            final Transition transition) {
        if (transition != null) {
            final ViewTreeObserver observer = sceneRoot.getViewTreeObserver();
            final ViewTreeObserver.OnPreDrawListener listener =
                    new ViewTreeObserver.OnPreDrawListener() {
        if (transition != null && sceneRoot != null) {
            MultiListener listener = new MultiListener(transition, sceneRoot);
            sceneRoot.addOnAttachStateChangeListener(listener);
            sceneRoot.getViewTreeObserver().addOnPreDrawListener(listener);
        }
    }

    /**
     * This private utility class is used to listen for both OnPreDraw and
     * OnAttachStateChange events. OnPreDraw events are the main ones we care
     * about since that's what triggers the transition to take place.
     * OnAttachStateChange events are also important in case the view is removed
     * from the hierarchy before the OnPreDraw event takes place; it's used to
     * clean up things since the OnPreDraw listener didn't get called in time.
     */
    private static class MultiListener implements ViewTreeObserver.OnPreDrawListener,
            View.OnAttachStateChangeListener {

        Transition mTransition;
        ViewGroup mSceneRoot;

        MultiListener(Transition transition, ViewGroup sceneRoot) {
            mTransition = transition;
            mSceneRoot = sceneRoot;
        }

        private void removeListeners() {
            mSceneRoot.getViewTreeObserver().removeOnPreDrawListener(this);
            mSceneRoot.removeOnAttachStateChangeListener(this);
        }

        @Override
        public void onViewAttachedToWindow(View v) {
        }

        @Override
        public void onViewDetachedFromWindow(View v) {
            removeListeners();

            sPendingTransitions.remove(mSceneRoot);
            ArrayList<Transition> runningTransitions = getRunningTransitions().get(mSceneRoot);
            if (runningTransitions != null && runningTransitions.size() > 0) {
                for (Transition runningTransition : runningTransitions) {
                    runningTransition.resume();
                }
            }
            mTransition.clearValues(true);
        }

        @Override
        public boolean onPreDraw() {
                    sceneRoot.getViewTreeObserver().removeOnPreDrawListener(this);
                    sPendingTransitions.remove(sceneRoot);
            removeListeners();
            sPendingTransitions.remove(mSceneRoot);
            // Add to running list, handle end to remove it
            final ArrayMap<ViewGroup, ArrayList<Transition>> runningTransitions =
                    getRunningTransitions();
                    ArrayList<Transition> currentTransitions = runningTransitions.get(sceneRoot);
            ArrayList<Transition> currentTransitions = runningTransitions.get(mSceneRoot);
            ArrayList<Transition> previousRunningTransitions = null;
            if (currentTransitions == null) {
                currentTransitions = new ArrayList<Transition>();
                        runningTransitions.put(sceneRoot, currentTransitions);
                runningTransitions.put(mSceneRoot, currentTransitions);
            } else if (currentTransitions.size() > 0) {
                previousRunningTransitions = new ArrayList<Transition>(currentTransitions);
            }
                    currentTransitions.add(transition);
                    transition.addListener(new Transition.TransitionListenerAdapter() {
            currentTransitions.add(mTransition);
            mTransition.addListener(new Transition.TransitionListenerAdapter() {
                @Override
                public void onTransitionEnd(Transition transition) {
                    ArrayList<Transition> currentTransitions =
                                    runningTransitions.get(sceneRoot);
                            runningTransitions.get(mSceneRoot);
                    currentTransitions.remove(transition);
                }
            });
                    transition.captureValues(sceneRoot, false);
            mTransition.captureValues(mSceneRoot, false);
            if (previousRunningTransitions != null) {
                for (Transition runningTransition : previousRunningTransitions) {
                    runningTransition.resume();
                }
            }
                    transition.playTransition(sceneRoot);
            mTransition.playTransition(mSceneRoot);

            return true;
        }
    };
            observer.addOnPreDrawListener(listener);
        }
    }

    private static void sceneChangeSetup(ViewGroup sceneRoot, Transition transition) {

+1 −1
Original line number Diff line number Diff line
@@ -65,7 +65,7 @@ public class KeyguardTransportControlView extends FrameLayout {
    protected static final boolean DEBUG = false;
    protected static final String TAG = "TransportControlView";

    private static final boolean ANIMATE_TRANSITIONS = false;
    private static final boolean ANIMATE_TRANSITIONS = true;

    private ViewGroup mMetadataContainer;
    private ViewGroup mInfoContainer;