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

Commit 5c5b6218 authored by Mady Mellor's avatar Mady Mellor
Browse files

If the update is non-interruptive don't re-bubble it

I haven't been able to reproduce this outside of tests on acloud
devices, but if you visit a bubble for the first time (causes an
update to the notif) and then dismiss it sometimes the user dismiss
can happen before  we get the update back from notification manager
service and the bubble will get re-bubbled incorrectly.

To fix this, if a bubble is in the overflow (or active) and there's
an update to the notification that is non-interruptive, we shouldn't
re-bubble that content because there wasn't a new message, just some
change on the notification.

Bug: 238896626
Test: atest BubblesTest
Test: atest --no-bazel-mode --iterations 20 PlatformScenarioTests:android.platform.test.scenario.sysui.bubble.ExpandAndDragToDismissTest
      (with vadim's changes & other CLc)
Change-Id: If0b7b3d3d1875ddd1df6a1412db8c42456de35c5
parent 55524d2a
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -452,6 +452,7 @@ public class Bubble implements BubbleViewProvider {
     */
    void setEntry(@NonNull final BubbleEntry entry) {
        Objects.requireNonNull(entry);
        boolean showingDotPreviously = showDot();
        mLastUpdated = entry.getStatusBarNotification().getPostTime();
        mIsBubble = entry.getStatusBarNotification().getNotification().isBubbleNotification();
        mPackageName = entry.getStatusBarNotification().getPackageName();
@@ -498,6 +499,10 @@ public class Bubble implements BubbleViewProvider {
        mShouldSuppressNotificationDot = entry.shouldSuppressNotificationDot();
        mShouldSuppressNotificationList = entry.shouldSuppressNotificationList();
        mShouldSuppressPeek = entry.shouldSuppressPeek();
        if (showingDotPreviously != showDot()) {
            // This will update the UI if needed
            setShowDot(showDot());
        }
    }

    @Nullable
+22 −4
Original line number Diff line number Diff line
@@ -1055,21 +1055,28 @@ public class BubbleController implements ConfigurationChangeListener {
    public void updateBubble(BubbleEntry notif, boolean suppressFlyout, boolean showInShade) {
        // If this is an interruptive notif, mark that it's interrupted
        mSysuiProxy.setNotificationInterruption(notif.getKey());
        if (!notif.getRanking().isTextChanged()
        boolean isNonInterruptiveNotExpanding = !notif.getRanking().isTextChanged()
                && (notif.getBubbleMetadata() != null
                && !notif.getBubbleMetadata().getAutoExpandBubble())
                && !notif.getBubbleMetadata().getAutoExpandBubble());
        if (isNonInterruptiveNotExpanding
                && mBubbleData.hasOverflowBubbleWithKey(notif.getKey())) {
            // Update the bubble but don't promote it out of overflow
            Bubble b = mBubbleData.getOverflowBubbleWithKey(notif.getKey());
            if (notif.isBubble()) {
                notif.setFlagBubble(false);
            }
            b.setEntry(notif);
            updateNotNotifyingEntry(b, notif, showInShade);
        } else if (mBubbleData.hasAnyBubbleWithKey(notif.getKey())
                && isNonInterruptiveNotExpanding) {
            Bubble b = mBubbleData.getAnyBubbleWithkey(notif.getKey());
            if (b != null) {
                updateNotNotifyingEntry(b, notif, showInShade);
            }
        } else if (mBubbleData.isSuppressedWithLocusId(notif.getLocusId())) {
            // Update the bubble but don't promote it out of overflow
            Bubble b = mBubbleData.getSuppressedBubbleWithKey(notif.getKey());
            if (b != null) {
                b.setEntry(notif);
                updateNotNotifyingEntry(b, notif, showInShade);
            }
        } else {
            Bubble bubble = mBubbleData.getOrCreateBubble(notif, null /* persistedBubble */);
@@ -1079,12 +1086,23 @@ public class BubbleController implements ConfigurationChangeListener {
                if (bubble.shouldAutoExpand()) {
                    bubble.setShouldAutoExpand(false);
                }
                mImpl.mCachedState.updateBubbleSuppressedState(bubble);
            } else {
                inflateAndAdd(bubble, suppressFlyout, showInShade);
            }
        }
    }

    void updateNotNotifyingEntry(Bubble b, BubbleEntry entry, boolean showInShade) {
        boolean isBubbleSelected = Objects.equals(b, mBubbleData.getSelectedBubble());
        boolean isBubbleExpandedAndSelected = isStackExpanded() && isBubbleSelected;
        b.setEntry(entry);
        boolean suppress = isBubbleExpandedAndSelected || !showInShade || !b.showInShade();
        b.setSuppressNotification(suppress);
        b.setShowDot(!isBubbleExpandedAndSelected);
        mImpl.mCachedState.updateBubbleSuppressedState(b);
    }

    @VisibleForTesting
    public void inflateAndAdd(Bubble bubble, boolean suppressFlyout, boolean showInShade) {
        // Lazy init stack view when a bubble is created
+51 −0
Original line number Diff line number Diff line
@@ -1433,6 +1433,57 @@ public class BubblesTest extends SysuiTestCase {
        assertThat(mBubbleData.hasBubbleInStackWithKey(mBubbleEntry.getKey())).isFalse();
    }

    /**
     * Verifies that if a bubble is in the overflow and a non-interruptive notification update
     * comes in for it, it stays in the overflow but the entry is updated.
     */
    @Test
    public void testNonInterruptiveUpdate_doesntBubbleFromOverflow() {
        mEntryListener.onEntryAdded(mRow);
        mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true);
        assertBubbleNotificationNotSuppressedFromShade(mBubbleEntry);

        // Dismiss the bubble so it's in the overflow
        mBubbleController.removeBubble(
                mRow.getKey(), Bubbles.DISMISS_USER_GESTURE);
        assertThat(mBubbleData.hasOverflowBubbleWithKey(mRow.getKey())).isTrue();

        // Update the entry to not show in shade
        setMetadataFlags(mRow,
                Notification.BubbleMetadata.FLAG_SUPPRESS_NOTIFICATION, /* enableFlag= */ true);
        mBubbleController.updateBubble(mBubbleEntry,
                /* suppressFlyout= */ false, /* showInShade= */ true);

        // Check that the update was applied - shouldn't be show in shade
        assertBubbleNotificationSuppressedFromShade(mBubbleEntry);
        // Check that it wasn't inflated (1 because it would've been inflated via onEntryAdded)
        verify(mBubbleController, times(1)).inflateAndAdd(
                any(Bubble.class), anyBoolean(), anyBoolean());
    }

    /**
     * Verifies that if a bubble is active, and a non-interruptive notification update comes in for
     * it, it doesn't trigger a new inflate and add for that bubble.
     */
    @Test
    public void testNonInterruptiveUpdate_doesntTriggerInflate() {
        mEntryListener.onEntryAdded(mRow);
        mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true);
        assertBubbleNotificationNotSuppressedFromShade(mBubbleEntry);

        // Update the entry to not show in shade
        setMetadataFlags(mRow,
                Notification.BubbleMetadata.FLAG_SUPPRESS_NOTIFICATION, /* enableFlag= */ true);
        mBubbleController.updateBubble(mBubbleEntry,
                /* suppressFlyout= */ false, /* showInShade= */ true);

        // Check that the update was applied - shouldn't be show in shade
        assertBubbleNotificationSuppressedFromShade(mBubbleEntry);
        // Check that it wasn't inflated (1 because it would've been inflated via onEntryAdded)
        verify(mBubbleController, times(1)).inflateAndAdd(
                any(Bubble.class), anyBoolean(), anyBoolean());
    }

    /**
     * Verifies that if a bubble is in the overflow and a non-interruptive notification update
     * comes in for it with FLAG_BUBBLE that the flag is removed.