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

Commit fbb08fc9 authored by Jiaming Liu's avatar Jiaming Liu
Browse files

[Bubbles] Fix flicker when collasping and expanding bubble 

The bubble TaskView is removed after collasping, and it is added
back when expanded. However, if this happens quickly, the expand
animation may have a flicker due to TaskView visibility. This CL
avoids the removal of the TaskView if expand animation starts
before the completion of collasp animation by adding reference
counting of animating bubbles.

Bug: 403612574
Test: atest BubbleBarLayerViewTest; Manual
Flag: EXEMPT bugfix
Change-Id: I1891f8ad312b2dbc550a39abc7d6e878e46354de
parent 44faab4a

libs/WindowManager/Shell/multivalentTests/src/com/android/wm/shell/bubbles/bar/BubbleBarLayerViewTest.kt

+93 −0
Original line number Diff line number Diff line
@@ -257,7 +257,10 @@ class BubbleBarLayerViewTest { 
        val bubble = createBubble("first")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(bubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isFalse()



        // Scrim, dismiss view and expanded view

        assertThat(bubbleBarLayerView.childCount).isEqualTo(3)
@@ -270,14 +273,20 @@ class BubbleBarLayerViewTest { 
        val secondBubble = createBubble("second")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(firstBubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isFalse()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.removeBubble(firstBubble) {} }

        // Expanded view is removed when bubble is removed

        assertThat(firstBubble.bubbleBarExpandedView).isNull()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(secondBubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isFalse()



        assertThat(bubbleBarLayerView.children.count { it is BubbleBarExpandedView }).isEqualTo(1)

        assertThat(bubbleBarLayerView.children.last()).isEqualTo(secondBubble.bubbleBarExpandedView)
@@ -289,10 +298,18 @@ class BubbleBarLayerViewTest { 
        val secondBubble = createBubble("second")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(firstBubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isFalse()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(secondBubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isTrue()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isFalse()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isFalse()



        assertThat(bubbleBarLayerView.children.count { it is BubbleBarExpandedView }).isEqualTo(1)

        assertThat(bubbleBarLayerView.children.last()).isEqualTo(secondBubble.bubbleBarExpandedView)
@@ -441,6 +458,76 @@ class BubbleBarLayerViewTest { 
        assertThat(bubble.bubbleBarExpandedView!!.x).isEqualTo(previousX)

    }



    @Test

    fun testSwitchBubblesQuickly() {

        val firstBubble = createBubble("first")

        val secondBubble = createBubble("second")

        val thirdBubble = createBubble("third")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(firstBubble) }

        waitForExpandedViewAnimation()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(secondBubble) }

        waitForExpandedViewAnimation()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(thirdBubble) }

        waitForExpandedViewAnimation()



        // Switch from 3rd -> 2nd, then immediately followed by 2nd -> 1st, without waiting for

        // animation completion.

        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(secondBubble) }

        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(firstBubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isTrue()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isTrue()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(thirdBubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(firstBubble)).isFalse()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(secondBubble)).isFalse()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(thirdBubble)).isFalse()



        assertThat(bubbleBarLayerView.children.count { it is BubbleBarExpandedView }).isEqualTo(1)

        assertThat(bubbleBarLayerView.children.last()).isEqualTo(firstBubble.bubbleBarExpandedView)

    }



    @Test

    fun testCollapse() {

        val bubble = createBubble("first")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(bubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isTrue()



        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isFalse()



        getInstrumentation().runOnMainSync { bubbleBarLayerView.collapse() }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isTrue()



        waitForCollapseViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isFalse()



        assertThat(bubbleBarLayerView.children.count { it is BubbleBarExpandedView }).isEqualTo(0)

    }



    @Test

    fun testCollapseThenImmediatelyExpand() {

        val bubble = createBubble("first")



        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(bubble) }

        waitForExpandedViewAnimation()



        // Without waiting for animation completion, we collapse then immediately expand.

        getInstrumentation().runOnMainSync { bubbleBarLayerView.collapse() }

        getInstrumentation().runOnMainSync { bubbleBarLayerView.showExpandedView(bubble) }

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isTrue()



        // waitForExpandedViewAnimation() also covers the collapse animation.

        waitForExpandedViewAnimation()

        assertThat(bubbleBarLayerView.isAnimatingBubbleTracked(bubble)).isFalse()



        assertThat(bubbleBarLayerView.children.count { it is BubbleBarExpandedView }).isEqualTo(1)

    }



    private fun createBubble(key: String): Bubble {

        val bubble = FakeBubbleFactory.createChatBubble(context, key).also {

            testBubblesList.add(it)
@@ -484,6 +571,12 @@ class BubbleBarLayerViewTest { 
        getInstrumentation().waitForIdleSync()

    }



    private fun waitForCollapseViewAnimation() {

        // Collapse animation also uses both the animator and physical animator, so waiting for

        // collapse animation is the same as waiting for the expand animation.

        waitForExpandedViewAnimation()

    }



    private fun View.dispatchTouchEvent(eventTime: Long, action: Int, point: PointF) {

        val event = MotionEvent.obtain(0L, eventTime, action, point.x, point.y, 0)

        getInstrumentation().runOnMainSync { dispatchTouchEvent(event) }

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/bar/BubbleBarLayerView.java

+39 −26
Original line number Diff line number Diff line
@@ -110,10 +110,10 @@ public class BubbleBarLayerView extends FrameLayout 
    private Insets mInsets;



    /**

     * Tracks bubbles used in switch animations. Ideally, we should track bubbles used in all types

     * of animations, but so far it only seems helpful for switch animations.

     * Tracks bubbles used in animations. We keep animating bubbles attached until the end of all

     * animations.

     */

    private final ReferenceCounter<BubbleViewProvider> mSwitchAnimationBubbleTracker =

    private final ReferenceCounter<BubbleViewProvider> mAnimatingBubbleTracker =

            new ReferenceCounter();



    public BubbleBarLayerView(Context context, BubbleController controller, BubbleData bubbleData,
@@ -354,9 +354,9 @@ public class BubbleBarLayerView extends FrameLayout 
                // Add temporary references to the previous and the current bubbles to prevent

                // them from being removed when canceling ongoing animations. References will be

                // added again when starting the switch animation.

                mSwitchAnimationBubbleTracker.increment(previousBubble, b);

                mAnimatingBubbleTracker.increment(previousBubble, b);

                mAnimationHelper.cancelAnimations();

                mSwitchAnimationBubbleTracker.decrement(previousBubble, b);

                mAnimatingBubbleTracker.decrement(previousBubble, b);



                // Need to check again because cancelAnimations might remove it from the parent.

                // TODO(b/403612574) use reference tracking for other animations.
@@ -459,7 +459,11 @@ public class BubbleBarLayerView extends FrameLayout 
        if (!mIsExpanded || mExpandedBubble == null) {

            throw new IllegalStateException("Can't animateExpand without expnaded state");

        }

        final Runnable afterAnimation = () -> {

        final BubbleViewProvider expandedBubble = mExpandedBubble;

        mAnimatingBubbleTracker.increment(previousBubble, expandedBubble);

        final Runnable endRunnable = () -> {

            mAnimatingBubbleTracker.decrement(previousBubble, expandedBubble);

            ensureAnimationEndingState();

            if (mExpandedView == null) return;

            // Touch delegate for the menu

            BubbleBarHandleView view = mExpandedView.getHandleView();
@@ -476,15 +480,9 @@ public class BubbleBarLayerView extends FrameLayout 
        };



        if (previousBubble != null) {

            final BubbleViewProvider expandedBubble = mExpandedBubble;

            mSwitchAnimationBubbleTracker.increment(previousBubble, expandedBubble);

            mAnimationHelper.animateSwitch(previousBubble, expandedBubble, () -> {

                mSwitchAnimationBubbleTracker.decrement(previousBubble, expandedBubble);

                ensureSwitchAnimationEndingState();

                afterAnimation.run();

            });

            mAnimationHelper.animateSwitch(previousBubble, expandedBubble, endRunnable);

        } else {

            mAnimationHelper.animateExpansion(mExpandedBubble, afterAnimation);

            mAnimationHelper.animateExpansion(expandedBubble, endRunnable);

        }

    }
@@ -522,8 +520,15 @@ public class BubbleBarLayerView extends FrameLayout 
        if (!mIsExpanded || mExpandedBubble == null) {

            throw new IllegalStateException("Can't animateExpand without expanded state");

        }

        final BubbleViewProvider expandedBubble = mExpandedBubble;

        mAnimatingBubbleTracker.increment(expandedBubble);

        final Runnable endRunnable = () -> {

            mAnimatingBubbleTracker.decrement(expandedBubble);

            ensureAnimationEndingState();

            animFinish.run();

        };

        mAnimationHelper.animateConvert(mExpandedBubble, startT, startBounds, startScale, snapshot,

                taskLeash, animFinish);

                taskLeash, endRunnable);

    }



    public void removeBubble(@NonNull Bubble bubble, @NonNull Runnable endAction) {
@@ -568,10 +573,12 @@ public class BubbleBarLayerView extends FrameLayout 
            return;

        }

        mIsExpanded = false;

        final BubbleBarExpandedView viewToRemove = mExpandedView;

        final BubbleViewProvider bubbleToCollapse = mExpandedBubble;

        mEducationViewController.hideEducation(/* animated = */ true);

        Runnable runnable = () -> {

            removeView(viewToRemove);

        mAnimatingBubbleTracker.increment(bubbleToCollapse);

        final Runnable endRunnable = () -> {

            mAnimatingBubbleTracker.decrement(bubbleToCollapse);

            ensureAnimationEndingState();

            if (endAction != null) {

                endAction.run();

            }
@@ -580,12 +587,13 @@ public class BubbleBarLayerView extends FrameLayout 
            }

        };

        if (mDragController != null && mDragController.isStuckToDismiss()) {

            mAnimationHelper.animateDismiss(runnable);

            mAnimationHelper.animateDismiss(endRunnable);

        } else {

            mAnimationHelper.animateCollapse(runnable);

            mAnimationHelper.animateCollapse(endRunnable);

        }

        mBubbleController.getSysuiProxy().onStackExpandChanged(false);

        mExpandedView = null;

        mExpandedBubble = null;

        mDragController = null;

        setTouchDelegate(null);

        showScrim(false);
@@ -720,16 +728,21 @@ public class BubbleBarLayerView extends FrameLayout 
        setupDragZoneFactory();

    }



    /** Ensures that only the expanded bubble is added at the end of all switch animations. */

    private void ensureSwitchAnimationEndingState() {

        if (!mSwitchAnimationBubbleTracker.hasReferences()) {

            // All switch animations are done, so we remove bubbles except for the expanded one.

            mSwitchAnimationBubbleTracker.forEach(bubble -> {

    /** Ensures that only the expanded bubble is added at the end of all animations. */

    private void ensureAnimationEndingState() {

        if (!mAnimatingBubbleTracker.hasReferences()) {

            // All animations are done, so we remove bubbles except for the expanded one.

            mAnimatingBubbleTracker.forEach(bubble -> {

                if (bubble != mExpandedBubble) {

                    removeView(bubble.getBubbleBarExpandedView());

                }

            });

            mSwitchAnimationBubbleTracker.clear();

            mAnimatingBubbleTracker.clear();

        }

    }



    @VisibleForTesting

    boolean isAnimatingBubbleTracked(@NonNull BubbleViewProvider bubble) {

        return mAnimatingBubbleTracker.isTracked(bubble);

    }

}

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/util/ReferenceCounter.java

+9 −0
Original line number Diff line number Diff line
@@ -88,4 +88,13 @@ public class ReferenceCounter<T> { 
    public void clear() {

        mReferences.clear();

    }



    /**

     * Returns whether the item is tracked. An item is tracked even if its reference count is

     * zero.

     */

    @VisibleForTesting

    public boolean isTracked(@NonNull T item) {

        return mReferences.containsKey(item);

    }

}