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

Commit f48b9369 authored by Mady Mellor's avatar Mady Mellor Committed by Android (Google) Code Review
Browse files

Merge changes I7d30f486,Icb19b152 into main 

* changes:
  Fix an issue with bubble being dismissed and added back at the same time
  Add a bugfix flag for a race when updating a bubble being removed
parents 1a419a28 2b77439d

libs/WindowManager/Shell/aconfig/multitasking.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -292,3 +292,13 @@ flag { 
        purpose: PURPOSE_BUGFIX

    }

}



flag {

    name: "fix_bubbles_add_same_bubble_being_removed"

    namespace: "multitasking"

    description: "Fixes a race condition where a bubble gets an update while being animated out"

    bug: "420487074"

    metadata {

        purpose: PURPOSE_BUGFIX

    }

}
 
 No newline at end of file

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

+269 −12
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.wm.shell.bubbles



import android.animation.AnimatorTestRule

import android.content.Context

import android.content.Intent

import android.content.pm.ShortcutInfo
@@ -68,6 +69,7 @@ import java.util.function.Consumer 
class BubbleStackViewTest {



    @get:Rule val setFlagsRule = SetFlagsRule()

    @get:Rule val animatorTestRule: AnimatorTestRule = AnimatorTestRule(this)



    private val context = ApplicationProvider.getApplicationContext<Context>()

    private lateinit var positioner: BubblePositioner
@@ -640,6 +642,7 @@ class BubbleStackViewTest { 
    @DisableFlags(Flags.FLAG_ENABLE_OPTIONAL_BUBBLE_OVERFLOW)

    @Test

    fun testCreateStackView_noOverflowContents_hasOverflow() {

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView =

                    BubbleStackView(

                            context,
@@ -651,7 +654,7 @@ class BubbleStackViewTest { 
                            { sysuiProxy },

                            shellExecutor

                    )



        }

        assertThat(bubbleData.overflowBubbles).isEmpty()

        val bubbleOverflow = bubbleData.overflow

        assertThat(bubbleStackView.getBubbleIndex(bubbleOverflow)).isGreaterThan(-1)
@@ -837,6 +840,260 @@ class BubbleStackViewTest { 
        assertThat(expandedViewContainer.visibility).isEqualTo(View.VISIBLE)

    }



    @Test

    fun removeBubble_notExpanded() {

        val bubble1 = createAndInflateChatBubble("key1")

        val bubble2 = createAndInflateChatBubble("key2")

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble1)

            bubbleStackView.addBubble(bubble2)

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(2)

        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // dismiss it in data so it's in the overflow

            bubbleData.dismissBubbleWithKey(bubble1.key, Bubbles.DISMISS_USER_GESTURE)

            // remove it from the stack

            bubbleStackView.removeBubble(bubble1)

            shellExecutor.flushAll()

        }



        // Check that proper changes to removed bubble happened

        assertThat(bubble1.expandedView).isNull()

        // still have bubbles + this was overflowed so should have icon view

        assertThat(bubble1.iconView).isNotNull()



        // And the bubble that is still in the stack is not affected

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

    }



    @Test

    fun removeBubble_notExpanded_notOverflowed() {

        val bubble1 = createAndInflateChatBubble("key1")

        val bubble2 = createAndInflateChatBubble("key2")

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble1)

            bubbleStackView.addBubble(bubble2)

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(2)

        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from stack + data, don't overflow it

            bubbleData.dismissBubbleWithKey(bubble1.key, Bubbles.DISMISS_NO_LONGER_BUBBLE)

            // remove it from the stack

            bubbleStackView.removeBubble(bubble1)

            shellExecutor.flushAll()

        }



        // Check that proper changes to removed bubble happened

        assertThat(bubble1.expandedView).isNull()

        assertThat(bubble1.iconView).isNull() // not in overflow so no icon



        // And the bubble that is still in the stack is not affected

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

    }



    @Test

    fun removeLastBubble_notExpanded() {

        val bubble = createAndInflateBubble()

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble)

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

        assertThat(bubble.expandedView).isNotNull()

        assertThat(bubble.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from the stack + data

            bubbleData.dismissBubbleWithKey(bubble.key, Bubbles.DISMISS_USER_GESTURE)

            bubbleStackView.removeBubble(bubble)

            shellExecutor.flushAll()

        }



        // Last bubble removed, so everything is null

        assertThat(bubble.expandedView).isNull()

        assertThat(bubble.iconView).isNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(0)

    }



    @Test

    fun removeBubble_whileExpanded() {

        val bubble1 = createAndInflateChatBubble("key1")

        val bubble2 = createAndInflateChatBubble("key2")

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble1)

            bubbleStackView.addBubble(bubble2)

            bubbleStackView.setExpanded(true)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.isExpanded).isTrue()

        assertThat(bubbleStackView.bubbleCount).isEqualTo(2)

        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from the stack + data

            bubbleData.dismissBubbleWithKey(bubble2.key, Bubbles.DISMISS_USER_GESTURE)

            bubbleStackView.removeBubble(bubble2)

            // stack would also be told to select the next bubble

            bubbleStackView.setSelectedBubble(bubble1)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        // Check that proper changes to removed bubble happened

        assertThat(bubble2.expandedView).isNull()

        // still have bubbles + this was overflowed so should have icon view

        assertThat(bubble2.iconView).isNotNull()



        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

        assertThat(bubbleStackView.isExpanded).isTrue()

    }



    @Test

    fun removeBubble_whileExpanded_notOverflowed() {

        val bubble1 = createAndInflateChatBubble("key1")

        val bubble2 = createAndInflateChatBubble("key2")

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble1)

            bubbleStackView.addBubble(bubble2)

            bubbleStackView.setExpanded(true)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.isExpanded).isTrue()

        assertThat(bubbleStackView.bubbleCount).isEqualTo(2)

        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()

        assertThat(bubble2.expandedView).isNotNull()

        assertThat(bubble2.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from the stack + data; don't overflow it

            bubbleData.dismissBubbleWithKey(bubble2.key, Bubbles.DISMISS_NO_LONGER_BUBBLE)

            bubbleStackView.removeBubble(bubble2)

            // stack would also be told to select the next bubble

            bubbleStackView.setSelectedBubble(bubble1)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        // Check that proper changes to removed bubble happened

        assertThat(bubble2.expandedView).isNull()

        assertThat(bubble2.iconView).isNull() // not in overflow so null



        assertThat(bubble1.expandedView).isNotNull()

        assertThat(bubble1.iconView).isNotNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

        assertThat(bubbleStackView.isExpanded).isTrue()

    }



    @Test

    fun removeLastBubble_whileExpanded() {

        val bubble = createAndInflateBubble()

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble)

            bubbleStackView.setExpanded(true)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.isExpanded).isTrue()

        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

        assertThat(bubble.expandedView).isNotNull()

        assertThat(bubble.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from the stack + data

            bubbleData.dismissBubbleWithKey(bubble.key, Bubbles.DISMISS_USER_GESTURE)

            bubbleStackView.removeBubble(bubble)

            // stack would also be told to collapse when last bubble removed

            bubbleStackView.setExpanded(false)

            // Run the scrim animation

            animatorTestRule.advanceTimeBy(300)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        // Last bubble removed, so everything is null

        assertThat(bubble.expandedView).isNull()

        assertThat(bubble.iconView).isNull()



        assertThat(bubbleStackView.bubbleCount).isEqualTo(0)

    }



    @EnableFlags(Flags.FLAG_FIX_BUBBLES_ADD_SAME_BUBBLE_BEING_REMOVED)

    @Test

    fun removeLastBubble_whileExpanded_addBack() {

        val bubble = createAndInflateBubble()

        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            bubbleStackView.addBubble(bubble)

            bubbleStackView.setExpanded(true)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        assertThat(bubbleStackView.isExpanded).isTrue()

        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

        assertThat(bubble.expandedView).isNotNull()

        assertThat(bubble.iconView).isNotNull()



        InstrumentationRegistry.getInstrumentation().runOnMainSync {

            // remove it from the data + stack

            bubbleData.dismissBubbleWithKey(bubble.key, Bubbles.DISMISS_USER_GESTURE)

            bubbleStackView.removeBubble(bubble)

            // typically stack would also be told to collapse when last bubble removed

            bubbleStackView.setExpanded(false)

            // Start the scrim animation

            animatorTestRule.advanceTimeBy(100)

            // Add the same bubble back

            bubbleData.notificationEntryUpdated(bubble, false, true)

            bubbleStackView.addBubble(bubble)

            shellExecutor.flushAll()

        }

        InstrumentationRegistry.getInstrumentation().waitForIdleSync()



        // Check that proper changes to removed bubble happened

        assertThat(bubble.expandedView).isNotNull()

        assertThat(bubble.expandedView!!.contentAlpha).isEqualTo(1)

        assertThat(bubble.expandedView!!.alpha).isEqualTo(1)

        assertThat(bubble.iconView).isNotNull()

        assertThat(bubble.iconView!!.alpha).isEqualTo(1)

        assertThat(bubble.iconView!!.scaleX).isEqualTo(1)

        assertThat(bubble.iconView!!.scaleY).isEqualTo(1)

        assertThat(bubbleStackView.bubbleCount).isEqualTo(1)

    }



    private fun createAndInflateChatBubble(key: String): Bubble {

        val icon = Icon.createWithResource(context.resources, R.drawable.bubble_ic_overflow_button)

        val shortcutInfo = ShortcutInfo.Builder(context, "fakeId").setIcon(icon).build()

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

+90 −19
Original line number Diff line number Diff line
@@ -2231,6 +2231,17 @@ public class BubbleStackView extends FrameLayout 
        // expand / collapse on.

        bubble.getIconView().setTranslationX(mStackAnimationController.getStackPosition().x);



        // If it is currently being animated out (and newly added again here), remove the view

        // and add it back so that it animates in.

        if (Flags.fixBubblesAddSameBubbleBeingRemoved()

                && bubble.getIconView().getParent() != null) {

            mBubbleContainer.removeViewNoAnimation(bubble.getIconView());

            // If the view was being animated out, need to reset its state

            bubble.getIconView().setAlpha(1f);

            bubble.getIconView().setVisibility(VISIBLE);

            bubble.getIconView().setScaleX(1f);

            bubble.getIconView().setScaleY(1f);

        }

        mBubbleContainer.addView(bubble.getIconView(), 0,

                new FrameLayout.LayoutParams(mPositioner.getBubbleSize(),

                        mPositioner.getBubbleSize()));
@@ -2248,18 +2259,63 @@ public class BubbleStackView extends FrameLayout 


    // via BubbleData.Listener

    void removeBubble(Bubble bubble) {

        if (isExpanded() && getBubbleCount() == 1) {

        boolean isLastBubble = getBubbleCount() == 1;

        if (isExpanded() && isLastBubble) {

            mRemovingLastBubbleWhileExpanded = true;

            // We're expanded while the last bubble is being removed. Let the scrim animate away

            // and then remove our views (removing the icon view triggers the removal of the

            // bubble window so do that at the end of the animation so we see the scrim animate).

            BadgedImageView iconView = bubble.getIconView();

            BubbleExpandedView bev = bubble.getExpandedView();

            final BubbleViewProvider expandedBubbleBeforeScrim = mExpandedBubble;

            // Notify the stack anim controller before running the scrim animation. In case

            // another bubble gets added during it.

            mStackAnimationController.onLastBubbleRemoved();



            showScrim(false, () -> {

                mRemovingLastBubbleWhileExpanded = false;

                if (Flags.fixBubblesAddSameBubbleBeingRemoved()) {

                    boolean removedBubbleBackInStack = mBubbleData.hasBubbleInStackWithKey(

                            bubble.getKey());

                    // In the time it takes for the scrim animation, that bubble may have gotten

                    // a new update and be added back -- only remove the view if it's not in the

                    // stack.

                    if (!removedBubbleBackInStack) {

                        if (iconView != null) {

                            mBubbleContainer.removeView(iconView);

                        }

                        if (bev != null) {

                            mExpandedViewContainer.removeView(bev);

                        }

                    } else if (bev != null) {

                        // If we were dragging out, reset the expanded view in case the bubble gets

                        // re-added

                        bev.setContentAlpha(1f);

                        bev.setBackgroundAlpha(1f);

                    }



                    mExpandedViewContainer.setAnimationMatrix(null);

                    mExpandedViewTemporarilyHidden = false;



                    if (!removedBubbleBackInStack) {

                        // Not in the stack -- clean up all the views

                        bubble.cleanupViews();

                    }



                    // Bubble keys may not have changed if we receive an update to the same bubble.

                    // Compare bubble object instances to see if the expanded bubble has changed.

                    if (expandedBubbleBeforeScrim == mExpandedBubble && !removedBubbleBackInStack) {

                        // Only clear expanded bubble if it has not changed since the scrim

                        // animation started.

                        // Scrim animation can take some time run and it is possible for a new

                        // bubble to be added while the animation is running.

                        // This causes the expanded bubble to change. Make sure we only clear the

                        // expanded bubble if it did not change between when the scrim animation

                        // started and completed.

                        mExpandedBubble = null;

                        updateExpandedView();

                    }

                } else {

                    bubble.cleanupExpandedView();

                    if (iconView != null) {

                        mBubbleContainer.removeView(iconView);
@@ -2269,14 +2325,16 @@ public class BubbleStackView extends FrameLayout 
                    // Bubble keys may not have changed if we receive an update to the same bubble.

                    // Compare bubble object instances to see if the expanded bubble has changed.

                    if (expandedBubbleBeforeScrim == mExpandedBubble) {

                    // Only clear expanded bubble if it has not changed since the scrim animation

                    // started.

                    // Scrim animation can take some time run and it is possible for a new bubble

                    // to be added while the animation is running. This causes the expanded

                    // bubble to change. Make sure we only clear the expanded bubble if it did

                    // not change between when the scrim animation started and completed.

                        // Only clear expanded bubble if it has not changed since the scrim

                        // animation started.

                        // Scrim animation can take some time run and it is possible for a new

                        // bubble to be added while the animation is running.

                        // This causes the expanded bubble to change. Make sure we only clear the

                        // expanded bubble if it did not change between when the scrim animation

                        // started and completed.

                        mExpandedBubble = null;

                    }

                }

            });

            logBubbleEvent(bubble, FrameworkStatsLog.BUBBLE_UICHANGED__ACTION__DISMISSED);

            return;
@@ -2290,13 +2348,14 @@ public class BubbleStackView extends FrameLayout 
            if (v instanceof BadgedImageView

                    && ((BadgedImageView) v).getKey().equals(bubble.getKey())) {

                mBubbleContainer.removeViewAt(i);

                if (mBubbleData.hasOverflowBubbleWithKey(bubble.getKey())) {

                if (!isLastBubble && mBubbleData.hasOverflowBubbleWithKey(bubble.getKey())) {

                    // Overflow bubbles show the icon view so just clean up the expanded view.

                    bubble.cleanupExpandedView();

                } else {

                    bubble.cleanupViews();

                }

                updateExpandedView();

                if (getBubbleCount() == 0 && !isExpanded()) {

                if (isLastBubble && !isExpanded()) {

                    // This is the last bubble and the stack is collapsed

                    updateStackPosition();

                }
@@ -2735,6 +2794,12 @@ public class BubbleStackView extends FrameLayout 
                        mScaleOutSpringConfig)

                .addUpdateListener((target, values) ->

                        mExpandedViewContainer.setAnimationMatrix(mExpandedViewContainerMatrix))

                .withEndActions(() -> {

                    if (Flags.fixBubblesAddSameBubbleBeingRemoved()

                            && mExpandedViewTemporarilyHidden) {

                        mExpandedViewContainer.removeView(mExpandedBubble.getExpandedView());

                    }

                })

                .start();



        // Animate alpha from 1f to 0f.
@@ -2750,6 +2815,12 @@ public class BubbleStackView extends FrameLayout 
        }



        mExpandedViewTemporarilyHidden = false;

        if (Flags.fixBubblesAddSameBubbleBeingRemoved()

                && mExpandedBubble != null

                && mExpandedBubble.getExpandedView() != null

                && mExpandedBubble.getExpandedView().getParent() == null) {

            mExpandedViewContainer.addView(mExpandedBubble.getExpandedView());

        }



        PhysicsAnimator.getInstance(mExpandedViewContainerMatrix)

                .spring(AnimatableScaleMatrix.SCALE_X,

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/animation/PhysicsAnimationLayout.java

+5 −0
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import androidx.dynamicanimation.animation.DynamicAnimation; 
import androidx.dynamicanimation.animation.SpringAnimation;

import androidx.dynamicanimation.animation.SpringForce;



import com.android.wm.shell.Flags;

import com.android.wm.shell.R;



import java.util.ArrayList;
@@ -339,6 +340,10 @@ public class PhysicsAnimationLayout extends FrameLayout { 
    /** Removes the child view immediately. */

    public void removeViewNoAnimation(View view) {

        super.removeView(view);

        if (Flags.fixBubblesAddSameBubbleBeingRemoved()) {

            removeTransientView(view);

            cancelAnimationsOnView(view);

        }

        view.setTag(R.id.physics_animator_tag, null);

    }

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/animation/StackAnimationController.java

+4 −0
Original line number Diff line number Diff line
@@ -831,6 +831,10 @@ public class StackAnimationController extends 


    // TODO: do we need this & BubbleStackView#updateBadgesAndZOrder?

    private void updateBadgesAndZOrder(View v, int index) {

        if (v == null) {

            // View was removed

            return;

        }

        v.setZ(index < NUM_VISIBLE_WHEN_RESTING ? (mMaxBubbles * mElevation) - index : 0f);

        BadgedImageView bv = (BadgedImageView) v;

        if (index == 0) {