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

Commit 6f64d029 authored by Hawkwood Glazier's avatar Hawkwood Glazier
Browse files

Do not remove new animators when previous animator is ended 

PropertyAnimator removes the animator from the view tag when it ends.
In rare cases, this was causing a previous animator to remove a newly
set animator from the tags, instead of the completed animator. This
occurred because the subsequent animation was triggered from the
onAnimationEnd callback of a previous animator. This would cause the
new animator to be set into the view's tags, before the other callback
unset those tags.

In this case, the KeyguardStatusView hide animation is being removed
from the view tags. While that is running, KeyguardVisibilityHelper
attempts and fails to cancel the hide animation when it determined the
view should instead be shown. As a result, the view is hidden after the
hide animation runs to it's conclusion.

There are two races which have to occur in sequence for this this effect
to be triggered. First the animation which hides the view must start
before the previous animation removes itself from the tags. Next the
KeyguardVisibilityHelper must attempt to show the view again before that
hide animation completes, so that a cancellation is required. Neither of
these races occur consistently.

Bug: 290627350
Test: Manually checked issue reproduction
Change-Id: I2c59d58914d6eb0460712dcd3d6dd3c771368453
parent 8f23adcc

packages/SystemUI/src/com/android/systemui/statusbar/notification/PropertyAnimator.java

+9 −6
Original line number Diff line number Diff line
@@ -114,18 +114,21 @@ public class PropertyAnimator { 
                || previousAnimator.getAnimatedFraction() == 0)) {

            animator.setStartDelay(properties.delay);

        }

        if (listener != null) {

            animator.addListener(listener);

        }

        // remove the tag when the animation is finished

        animator.addListener(new AnimatorListenerAdapter() {

            @Override

            public void onAnimationEnd(Animator animation) {

                Animator existing = (Animator) view.getTag(animatorTag);

                if (existing == animation) {

                    view.setTag(animatorTag, null);

                    view.setTag(animationStartTag, null);

                    view.setTag(animationEndTag, null);

                }

            }

        });

        if (listener != null) {

            animator.addListener(listener);

        }

        ViewState.startAnimator(animator, listener);

        view.setTag(animatorTag, animator);

        view.setTag(animationStartTag, currentValue);

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/PropertyAnimatorTest.java

+36 −3
Original line number Diff line number Diff line
@@ -18,10 +18,16 @@ package com.android.systemui.statusbar.notification; 


import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.verify;



import android.animation.Animator;

import android.animation.AnimatorListenerAdapter;

import android.animation.ValueAnimator;

import android.test.suitebuilder.annotation.SmallTest;
@@ -33,8 +39,8 @@ import android.view.View; 
import android.view.animation.Interpolator;



import com.android.app.animation.Interpolators;

import com.android.systemui.res.R;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.res.R;

import com.android.systemui.statusbar.notification.stack.AnimationFilter;

import com.android.systemui.statusbar.notification.stack.AnimationProperties;

import com.android.systemui.statusbar.notification.stack.ViewState;
@@ -85,7 +91,7 @@ public class PropertyAnimatorTest extends SysuiTestCase { 
            return mEffectiveProperty;

        }

    };

    private AnimatorListenerAdapter mFinishListener = mock(AnimatorListenerAdapter.class);

    private AnimatorListenerAdapter mFinishListener;

    private AnimationProperties mAnimationProperties = new AnimationProperties() {

        @Override

        public AnimationFilter getAnimationFilter() {
@@ -104,6 +110,7 @@ public class PropertyAnimatorTest extends SysuiTestCase { 
    @Before

    public void setUp() {

        mView = new View(getContext());

        mFinishListener = mock(AnimatorListenerAdapter.class);

    }



    @Test
@@ -228,6 +235,32 @@ public class PropertyAnimatorTest extends SysuiTestCase { 
        assertTrue(animator.getListeners().contains(mFinishListener));

    }



    @Test

    public void testListenerCallbackOrderAndTagState() {

        mAnimationFilter.reset();

        mAnimationFilter.animate(mProperty.getProperty());

        mAnimationProperties.setCustomInterpolator(mEffectiveProperty, mTestInterpolator);

        mAnimationProperties.setDuration(500);



        // Validates that the onAnimationEnd function set by PropertyAnimator was run first.

        doAnswer(invocation -> {

            assertNull(mView.getTag(mProperty.getAnimatorTag()));

            return null;

        })

                .when(mFinishListener)

                .onAnimationEnd(any(Animator.class), anyBoolean());



        // Begin the animation and verify it set state correctly

        PropertyAnimator.startAnimation(mView, mProperty, 200f, mAnimationProperties);

        ValueAnimator animator = ViewState.getChildTag(mView, mProperty.getAnimatorTag());

        assertNotNull(animator);

        assertNotNull(mView.getTag(mProperty.getAnimatorTag()));



        // Terminate the animation to run end runners, and validate they executed.

        animator.end();

        verify(mFinishListener).onAnimationEnd(animator, false);

    }



    @Test

    public void testIsAnimating() {

        mAnimationFilter.reset();