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

Commit db0a6024 authored by Mady Mellor's avatar Mady Mellor
Browse files

canBubble only true when it can actually bubble 

If the notif is unable to present as a bubble:
  - set the record#canBubble value to false
  - null out bubble metadata if present (we show bubble
    button if there is metadata, so shouldn't show it
    if it can't present for some reason)

Because bubble metadata is now null'd out we don't have
the shortcutId to check / update the tracked shortcut
list, so instead if it's null check for that entry in
the list and remove it. Also fixes one spot where we
weren't unregistering the listener before and adds a
test for it.

Test: atest ShortcutHelperTest BubbleExtractorTest NotificationManagerServiceTest
Bug: 152883583
Change-Id: I29e993c01a8fb9fcdf2386b18a94adc712fe101f
parent a55133da

services/core/java/com/android/server/notification/BubbleExtractor.java

+19 −8
Original line number Diff line number Diff line
@@ -75,9 +75,19 @@ public class BubbleExtractor implements NotificationSignalExtractor { 
                mConfig.getBubblePreference(

                        record.getSbn().getPackageName(), record.getSbn().getUid());

        NotificationChannel recordChannel = record.getChannel();

        boolean canPresentAsBubble = canPresentAsBubble(record)

                && !mActivityManager.isLowRamDevice()

                && record.isConversation()

                && (record.getNotification().flags & FLAG_FOREGROUND_SERVICE) == 0;



        if (!mConfig.bubblesEnabled() || bubblePreference == BUBBLE_PREFERENCE_NONE) {

        if (!mConfig.bubblesEnabled()

                || bubblePreference == BUBBLE_PREFERENCE_NONE

                || !canPresentAsBubble) {

            record.setAllowBubble(false);

            if (!canPresentAsBubble) {

                // clear out bubble metadata since it can't be used

                record.getNotification().setBubbleMetadata(null);

            }

        } else if (recordChannel == null) {

            // the app is allowed but there's no channel to check

            record.setAllowBubble(true);
@@ -86,14 +96,15 @@ public class BubbleExtractor implements NotificationSignalExtractor { 
        } else if (bubblePreference == BUBBLE_PREFERENCE_SELECTED) {

            record.setAllowBubble(recordChannel.canBubble());

        }

        if (DBG) {

            Slog.d(TAG, "record: " + record.getKey()

                    + " appPref: " + bubblePreference

                    + " canBubble: " + record.canBubble()

                    + " canPresentAsBubble: " + canPresentAsBubble

                    + " flagRemoved: " + record.isFlagBubbleRemoved());

        }



        final boolean fulfillsPolicy = record.canBubble()

                && record.isConversation()

                && !mActivityManager.isLowRamDevice()

                && (record.getNotification().flags & FLAG_FOREGROUND_SERVICE) == 0;

        final boolean applyFlag = fulfillsPolicy

                && canPresentAsBubble(record)

                && !record.isFlagBubbleRemoved();

        final boolean applyFlag = record.canBubble() && !record.isFlagBubbleRemoved();

        if (applyFlag) {

            record.getNotification().flags |= FLAG_BUBBLE;

        } else {

services/core/java/com/android/server/notification/ShortcutHelper.java

+31 −9
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import android.content.pm.ShortcutServiceInternal; 
import android.os.Binder;

import android.os.Handler;

import android.os.UserHandle;

import android.text.TextUtils;

import android.util.Slog;



import com.android.internal.annotations.VisibleForTesting;
@@ -111,6 +112,15 @@ public class ShortcutHelper { 
                    }

                    if (!foundShortcut) {

                        bubbleKeysToRemove.add(shortcutBubbles.get(shortcutId));

                        shortcutBubbles.remove(shortcutId);

                        if (shortcutBubbles.isEmpty()) {

                            mActiveShortcutBubbles.remove(packageName);

                            if (mLauncherAppsCallbackRegistered

                                    && mActiveShortcutBubbles.isEmpty()) {

                                mLauncherAppsService.unregisterCallback(mLauncherAppsCallback);

                                mLauncherAppsCallbackRegistered = false;

                            }

                        }

                    }

                }

            }
@@ -209,15 +219,16 @@ public class ShortcutHelper { 
     * @param removedNotification true if this notification is being removed

     * @param handler handler to register the callback with

     */

    void maybeListenForShortcutChangesForBubbles(NotificationRecord r, boolean removedNotification,

    void maybeListenForShortcutChangesForBubbles(NotificationRecord r,

            boolean removedNotification,

            Handler handler) {

        final String shortcutId = r.getNotification().getBubbleMetadata() != null

                ? r.getNotification().getBubbleMetadata().getShortcutId()

                : null;

        if (shortcutId == null) {

            return;

        }

        if (r.getNotification().isBubbleNotification() && !removedNotification) {

        if (!removedNotification

                && !TextUtils.isEmpty(shortcutId)

                && r.getShortcutInfo() != null

                && r.getShortcutInfo().getId().equals(shortcutId)) {

            // Must track shortcut based bubbles in case the shortcut is removed

            HashMap<String, String> packageBubbles = mActiveShortcutBubbles.get(

                    r.getSbn().getPackageName());
@@ -235,11 +246,22 @@ public class ShortcutHelper { 
            HashMap<String, String> packageBubbles = mActiveShortcutBubbles.get(

                    r.getSbn().getPackageName());

            if (packageBubbles != null) {

                if (!TextUtils.isEmpty(shortcutId)) {

                    packageBubbles.remove(shortcutId);

                } else {

                    // Check if there was a matching entry

                    for (String pkgShortcutId : packageBubbles.keySet()) {

                        String entryKey = packageBubbles.get(pkgShortcutId);

                        if (r.getKey().equals(entryKey)) {

                            // No longer has shortcut id so remove it

                            packageBubbles.remove(pkgShortcutId);

                        }

                    }

            if (packageBubbles != null && packageBubbles.isEmpty()) {

                }

                if (packageBubbles.isEmpty()) {

                    mActiveShortcutBubbles.remove(r.getSbn().getPackageName());

                }

            }

            if (mLauncherAppsCallbackRegistered && mActiveShortcutBubbles.isEmpty()) {

                mLauncherAppsService.unregisterCallback(mLauncherAppsCallback);

                mLauncherAppsCallbackRegistered = false;

services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java

+43 −8
Original line number Diff line number Diff line
@@ -26,6 +26,8 @@ import static android.content.pm.ActivityInfo.RESIZE_MODE_RESIZEABLE; 
import static android.content.pm.ActivityInfo.RESIZE_MODE_UNRESIZEABLE;



import static junit.framework.Assert.assertFalse;

import static junit.framework.Assert.assertNotNull;

import static junit.framework.Assert.assertNull;

import static junit.framework.Assert.assertTrue;



import static org.mockito.ArgumentMatchers.any;
@@ -166,6 +168,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_ALL /* app */,

                ALLOW_BUBBLE_OFF /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);

        mBubbleExtractor.process(r);
@@ -178,6 +182,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_ALL /* app */,

                DEFAULT_ALLOW_BUBBLE /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -190,6 +196,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_ALL /* app */,

                ALLOW_BUBBLE_ON /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -202,6 +210,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(false /* feature */,

                BUBBLE_PREFERENCE_ALL /* app */,

                ALLOW_BUBBLE_ON /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -215,6 +225,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_NONE /* app */,

                ALLOW_BUBBLE_ON /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -228,6 +240,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_NONE /* app */,

                DEFAULT_ALLOW_BUBBLE /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -241,6 +255,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_SELECTED /* app */,

                DEFAULT_ALLOW_BUBBLE /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -254,6 +270,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_SELECTED /* app */,

                ALLOW_BUBBLE_OFF /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);

        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -267,6 +285,9 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(true /* feature */,

                BUBBLE_PREFERENCE_SELECTED /* app */,

                ALLOW_BUBBLE_ON /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);



        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -279,6 +300,9 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        setUpBubblesEnabled(false /* feature */,

                BUBBLE_PREFERENCE_SELECTED /* app */,

                ALLOW_BUBBLE_ON /* channel */);

        when(mActivityManager.isLowRamDevice()).thenReturn(false);

        setUpShortcutBubble(true /* isValid */);



        NotificationRecord r = getNotificationRecord(true /* bubble */);



        mBubbleExtractor.process(r);
@@ -305,6 +329,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertNotNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -320,6 +345,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertNotNull(r.getNotification().getBubbleMetadata());

        assertTrue(r.getNotification().isBubbleNotification());

    }
@@ -335,6 +361,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertNotNull(r.getNotification().getBubbleMetadata());

        assertTrue(r.getNotification().isBubbleNotification());

    }
@@ -350,7 +377,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        r.setShortcutInfo(null);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -366,7 +394,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        r.setShortcutInfo(null);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -381,7 +410,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        NotificationRecord r = getNotificationRecord(true /* bubble */);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -395,7 +425,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        NotificationRecord r = getNotificationRecord(false /* bubble */);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -414,7 +445,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 


        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -429,7 +461,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        NotificationRecord r = getNotificationRecord(true /* bubble */);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -445,7 +478,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        NotificationRecord r = getNotificationRecord(true /* bubble */);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }
@@ -462,7 +496,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { 
        NotificationRecord r = getNotificationRecord(true /* bubble */);

        mBubbleExtractor.process(r);



        assertTrue(r.canBubble());

        assertFalse(r.canBubble());

        assertNull(r.getNotification().getBubbleMetadata());

        assertFalse(r.getNotification().isBubbleNotification());

    }

}

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+4 −6
Original line number Diff line number Diff line
@@ -6135,8 +6135,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
                "tag", mUid, 0, nb.build(), new UserHandle(mUid), null, 0);

        NotificationRecord nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel);







        // Test: Send the bubble notification

        mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(),

                nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId());
@@ -6168,12 +6166,12 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        verify(mLauncherApps, times(1)).unregisterCallback(launcherAppsCallback.getValue());



        // We're no longer a bubble

        Notification notif2 = mService.getNotificationRecord(

                nr.getSbn().getKey()).getNotification();

        assertFalse(notif2.isBubbleNotification());

        NotificationRecord notif2 = mService.getNotificationRecord(

                nr.getSbn().getKey());

        assertNull(notif2.getShortcutInfo());

        assertFalse(notif2.getNotification().isBubbleNotification());

    }





    @Test

    public void testNotificationBubbles_shortcut_stopListeningWhenNotifRemoved()

            throws RemoteException {

services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java

+46 −4
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ import org.mockito.Mock; 
import org.mockito.MockitoAnnotations;



import java.util.ArrayList;

import java.util.Collections;

import java.util.List;



@SmallTest
@@ -73,6 +74,8 @@ public class ShortcutHelperTest extends UiServiceTestCase { 
    StatusBarNotification mSbn;

    @Mock

    Notification.BubbleMetadata mBubbleMetadata;

    @Mock

    ShortcutInfo mShortcutInfo;



    ShortcutHelper mShortcutHelper;
@@ -86,13 +89,13 @@ public class ShortcutHelperTest extends UiServiceTestCase { 
        when(mNr.getSbn()).thenReturn(mSbn);

        when(mSbn.getPackageName()).thenReturn(PKG);

        when(mNr.getNotification()).thenReturn(mNotif);

        when(mNr.getShortcutInfo()).thenReturn(mShortcutInfo);

        when(mShortcutInfo.getId()).thenReturn(SHORTCUT_ID);

        when(mNotif.getBubbleMetadata()).thenReturn(mBubbleMetadata);

        when(mBubbleMetadata.getShortcutId()).thenReturn(SHORTCUT_ID);

    }



    private LauncherApps.Callback addShortcutBubbleAndVerifyListener() {

        when(mNotif.isBubbleNotification()).thenReturn(true);



        mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr,

                false /* removed */,

                null /* handler */);
@@ -124,12 +127,40 @@ public class ShortcutHelperTest extends UiServiceTestCase { 
    }



    @Test

    public void testBubbleNoLongerBubble_listenerRemoved() {

    public void testBubbleNoLongerHasBubbleMetadata_listenerRemoved() {

        // First set it up to listen

        addShortcutBubbleAndVerifyListener();



        // Then make it not a bubble

        when(mNotif.isBubbleNotification()).thenReturn(false);

        when(mNotif.getBubbleMetadata()).thenReturn(null);

        mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr,

                false /* removed */,

                null /* handler */);



        verify(mLauncherApps, times(1)).unregisterCallback(any());

    }



    @Test

    public void testBubbleNoLongerHasShortcutId_listenerRemoved() {

        // First set it up to listen

        addShortcutBubbleAndVerifyListener();



        // Clear out shortcutId

        when(mBubbleMetadata.getShortcutId()).thenReturn(null);

        mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr,

                false /* removed */,

                null /* handler */);



        verify(mLauncherApps, times(1)).unregisterCallback(any());

    }



    @Test

    public void testNotifNoLongerHasShortcut_listenerRemoved() {

        // First set it up to listen

        addShortcutBubbleAndVerifyListener();



        // Clear out shortcutId

        when(mNr.getShortcutInfo()).thenReturn(null);

        mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr,

                false /* removed */,

                null /* handler */);
@@ -137,6 +168,17 @@ public class ShortcutHelperTest extends UiServiceTestCase { 
        verify(mLauncherApps, times(1)).unregisterCallback(any());

    }



    @Test

    public void testOnShortcutsChanged_listenerRemoved() {

        // First set it up to listen

        LauncherApps.Callback callback = addShortcutBubbleAndVerifyListener();



        // App shortcuts are removed:

        callback.onShortcutsChanged(PKG, Collections.emptyList(),  mock(UserHandle.class));



        verify(mLauncherApps, times(1)).unregisterCallback(any());

    }



    @Test

    public void testListenerNotifiedOnShortcutRemoved() {

        LauncherApps.Callback callback = addShortcutBubbleAndVerifyListener();