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

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

Remove headsUpCommon, simplifies some code, adds tests 

Previously, canHeadsUpCommon was shared between shouldHeadsUp and
shouldBubbleUp.

Some of the shared logic was not appropriate for bubbles.
  - suppressPeek shouldn't apply to bubbles in this manner (this was
    causing the bubble notification to be hidden from the shade when
    "don't pop notif on screen" is enabled in DND.
  - mUseHeadsUp is tied to the device allowing HUNs, which shouldn't
    impact if bubbles appear or not

canAlertCommon: common checks for HUNs in all states & bubbles
canAlertAwakeCommon: common checks for HUNs when device is awake & bubbles

This CL:
- removes canHeadsUpCommon & moves appropriate checks into shouldHeadsUp
- defaults showInShade to be true, allowing canAlertCommon to work
  correctly for bubbles
- adds tests for NotificationInterruptionStateProvider
- removes some unused code in ExpandableNotificationRow

Test: atest NotificationInterruptionStateProviderTest
Test: manual - turn "don't pop notif on screen" on in DND
             - post a bubble
             => note that the bubble appears & notification is in shade
Bug: 133444037
Change-Id: If2b15ce9c322877e56a60e1b4c5fdf625b48c030
parent d71f26f3

packages/SystemUI/src/com/android/systemui/SystemUIFactory.java

+3 −2
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ import com.android.systemui.statusbar.NotificationLockscreenUserManagerImpl; 
import com.android.systemui.statusbar.NotificationMediaManager;

import com.android.systemui.statusbar.ScrimView;

import com.android.systemui.statusbar.notification.NotificationEntryManager;

import com.android.systemui.statusbar.notification.NotificationFilter;

import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider;

import com.android.systemui.statusbar.notification.collection.NotificationData;

import com.android.systemui.statusbar.phone.DozeParameters;
@@ -216,8 +217,8 @@ public class SystemUIFactory { 
    @Singleton

    @Provides

    public NotificationInterruptionStateProvider provideNotificationInterruptionStateProvider(

            Context context) {

        return new NotificationInterruptionStateProvider(context);

            Context context, NotificationFilter filter, StatusBarStateController controller) {

        return new NotificationInterruptionStateProvider(context, filter, controller);

    }



    @Singleton

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

+94 −71
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import android.app.NotificationManager; 
import android.content.Context;

import android.database.ContentObserver;

import android.hardware.display.AmbientDisplayConfiguration;

import android.os.Bundle;

import android.os.PowerManager;

import android.os.RemoteException;

import android.os.ServiceManager;
@@ -32,7 +31,6 @@ import android.provider.Settings; 
import android.service.dreams.DreamService;

import android.service.dreams.IDreamManager;

import android.service.notification.StatusBarNotification;

import android.text.TextUtils;

import android.util.Log;



import com.android.internal.annotations.VisibleForTesting;
@@ -41,9 +39,10 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController; 
import com.android.systemui.statusbar.NotificationPresenter;

import com.android.systemui.statusbar.StatusBarState;

import com.android.systemui.statusbar.notification.collection.NotificationEntry;

import com.android.systemui.statusbar.phone.ShadeController;

import com.android.systemui.statusbar.policy.HeadsUpManager;



import javax.inject.Inject;



/**

 * Provides heads-up and pulsing state for notification entries.

 */
@@ -54,9 +53,8 @@ public class NotificationInterruptionStateProvider { 
    private static final boolean ENABLE_HEADS_UP = true;

    private static final String SETTING_HEADS_UP_TICKER = "ticker_gets_heads_up";



    private final StatusBarStateController mStatusBarStateController =

            Dependency.get(StatusBarStateController.class);

    private final NotificationFilter mNotificationFilter = Dependency.get(NotificationFilter.class);

    private final StatusBarStateController mStatusBarStateController;

    private final NotificationFilter mNotificationFilter;

    private final AmbientDisplayConfiguration mAmbientDisplayConfiguration;



    private final Context mContext;
@@ -64,7 +62,6 @@ public class NotificationInterruptionStateProvider { 
    private final IDreamManager mDreamManager;



    private NotificationPresenter mPresenter;

    private ShadeController mShadeController;

    private HeadsUpManager mHeadsUpManager;

    private HeadsUpSuppressor mHeadsUpSuppressor;
@@ -73,12 +70,16 @@ public class NotificationInterruptionStateProvider { 
    protected boolean mUseHeadsUp = false;

    private boolean mDisableNotificationAlerts;



    public NotificationInterruptionStateProvider(Context context) {

    @Inject

    public NotificationInterruptionStateProvider(Context context, NotificationFilter filter,

            StatusBarStateController stateController) {

        this(context,

                (PowerManager) context.getSystemService(Context.POWER_SERVICE),

                IDreamManager.Stub.asInterface(

                        ServiceManager.checkService(DreamService.DREAM_SERVICE)),

                new AmbientDisplayConfiguration(context));

                new AmbientDisplayConfiguration(context),

                filter,

                stateController);

    }



    @VisibleForTesting
@@ -86,11 +87,15 @@ public class NotificationInterruptionStateProvider { 
            Context context,

            PowerManager powerManager,

            IDreamManager dreamManager,

            AmbientDisplayConfiguration ambientDisplayConfiguration) {

            AmbientDisplayConfiguration ambientDisplayConfiguration,

            NotificationFilter notificationFilter,

            StatusBarStateController statusBarStateController) {

        mContext = context;

        mPowerManager = powerManager;

        mDreamManager = dreamManager;

        mAmbientDisplayConfiguration = ambientDisplayConfiguration;

        mNotificationFilter = notificationFilter;

        mStatusBarStateController = statusBarStateController;

    }



    /** Sets up late-binding dependencies for this component. */
@@ -98,11 +103,8 @@ public class NotificationInterruptionStateProvider { 
            NotificationPresenter notificationPresenter,

            HeadsUpManager headsUpManager,

            HeadsUpSuppressor headsUpSuppressor) {

        mPresenter = notificationPresenter;

        mHeadsUpManager = headsUpManager;

        mHeadsUpSuppressor = headsUpSuppressor;



        mHeadsUpObserver = new ContentObserver(Dependency.get(Dependency.MAIN_HANDLER)) {

        setUpWithPresenter(notificationPresenter, headsUpManager, headsUpSuppressor,

                new ContentObserver(Dependency.get(Dependency.MAIN_HANDLER)) {

                    @Override

                    public void onChange(boolean selfChange) {

                        boolean wasUsing = mUseHeadsUp;
@@ -115,12 +117,25 @@ public class NotificationInterruptionStateProvider { 
                        if (wasUsing != mUseHeadsUp) {

                            if (!mUseHeadsUp) {

                                Log.d(TAG,

                                "dismissing any existing heads up notification on disable event");

                                        "dismissing any existing heads up notification on disable"

                                                + " event");

                                mHeadsUpManager.releaseAllImmediately();

                            }

                        }

                    }

        };

                });

    }



    /** Sets up late-binding dependencies for this component. */

    public void setUpWithPresenter(

            NotificationPresenter notificationPresenter,

            HeadsUpManager headsUpManager,

            HeadsUpSuppressor headsUpSuppressor,

            ContentObserver observer) {

        mPresenter = notificationPresenter;

        mHeadsUpManager = headsUpManager;

        mHeadsUpSuppressor = headsUpSuppressor;

        mHeadsUpObserver = observer;



        if (ENABLE_HEADS_UP) {

            mContext.getContentResolver().registerContentObserver(
@@ -134,13 +149,6 @@ public class NotificationInterruptionStateProvider { 
        mHeadsUpObserver.onChange(true); // set up

    }



    private ShadeController getShadeController() {

        if (mShadeController == null) {

            mShadeController = Dependency.get(ShadeController.class);

        }

        return mShadeController;

    }



    /**

     * Whether the notification should appear as a bubble with a fly-out on top of the screen.

     *
@@ -149,6 +157,15 @@ public class NotificationInterruptionStateProvider { 
     */

    public boolean shouldBubbleUp(NotificationEntry entry) {

        final StatusBarNotification sbn = entry.notification;



        if (!canAlertCommon(entry)) {

            return false;

        }



        if (!canAlertAwakeCommon(entry)) {

            return false;

        }



        if (!entry.canBubble) {

            if (DEBUG) {

                Log.d(TAG, "No bubble up: not allowed to bubble: " + sbn.getKey());
@@ -173,10 +190,6 @@ public class NotificationInterruptionStateProvider { 
            return false;

        }



        if (!canHeadsUpCommon(entry)) {

            return false;

        }



        return true;

    }
@@ -197,23 +210,34 @@ public class NotificationInterruptionStateProvider { 
    private boolean shouldHeadsUpWhenAwake(NotificationEntry entry) {

        StatusBarNotification sbn = entry.notification;



        boolean inShade = mStatusBarStateController.getState() == SHADE;

        if (entry.isBubble() && inShade) {

        if (!mUseHeadsUp) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: in unlocked shade where notification is shown as a "

                        + "bubble: " + sbn.getKey());

                Log.d(TAG, "No heads up: no huns");

            }

            return false;

        }



        if (!canAlertCommon(entry)) {

            return false;

        }



        if (!canAlertAwakeCommon(entry)) {

            return false;

        }



        boolean inShade = mStatusBarStateController.getState() == SHADE;

        if (entry.isBubble() && inShade) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: notification shouldn't alert: " + sbn.getKey());

                Log.d(TAG, "No heads up: in unlocked shade where notification is shown as a "

                        + "bubble: " + sbn.getKey());

            }

            return false;

        }



        if (!canHeadsUpCommon(entry)) {

        if (entry.shouldSuppressPeek()) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: suppressed by DND: " + sbn.getKey());

            }

            return false;

        }
@@ -287,16 +311,13 @@ public class NotificationInterruptionStateProvider { 
    }



    /**

     * Common checks between regular heads up and when pulsing.  See

     * {@link #shouldHeadsUp(NotificationEntry)} and

     * {@link #shouldHeadsUpWhenDozing(NotificationEntry)}.  Notifications that fail any of these

     * checks

     * should not alert at all.

     * Common checks between regular & AOD heads up and bubbles.

     *

     * @param entry the entry to check

     * @return true if these checks pass, false if the notification should not alert

     */

    protected boolean canAlertCommon(NotificationEntry entry) {

    @VisibleForTesting

    public boolean canAlertCommon(NotificationEntry entry) {

        StatusBarNotification sbn = entry.notification;



        if (mNotificationFilter.shouldFilterOut(entry)) {
@@ -313,46 +334,36 @@ public class NotificationInterruptionStateProvider { 
            }

            return false;

        }



        return true;

    }



    /**

     * Common checks between heads up alerting and bubble fly out alerting. See

     * {@link #shouldHeadsUp(NotificationEntry)} and

     * {@link #shouldBubbleUp(NotificationEntry)}. Notifications that fail any of these

     * checks should not interrupt the user on screen.

     * Common checks between alerts that occur while the device is awake (heads up & bubbles).

     *

     * @param entry the entry to check

     * @return true if these checks pass, false if the notification should not interrupt on screen

     * @return true if these checks pass, false if the notification should not alert

     */

    public boolean canHeadsUpCommon(NotificationEntry entry) {

    @VisibleForTesting

    public boolean canAlertAwakeCommon(NotificationEntry entry) {

        StatusBarNotification sbn = entry.notification;



        if (!mUseHeadsUp || mPresenter.isDeviceInVrMode()) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: no huns or vr mode");

            }

            return false;

        }



        if (entry.shouldSuppressPeek()) {

        if (mPresenter.isDeviceInVrMode()) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: suppressed by DND: " + sbn.getKey());

                Log.d(TAG, "No alerting: no huns or vr mode");

            }

            return false;

        }



        if (isSnoozedPackage(sbn)) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: snoozed package: " + sbn.getKey());

                Log.d(TAG, "No alerting: snoozed package: " + sbn.getKey());

            }

            return false;

        }



        if (entry.hasJustLaunchedFullScreenIntent()) {

            if (DEBUG) {

                Log.d(TAG, "No heads up: recent fullscreen: " + sbn.getKey());

                Log.d(TAG, "No alerting: recent fullscreen: " + sbn.getKey());

            }

            return false;

        }
@@ -370,6 +381,18 @@ public class NotificationInterruptionStateProvider { 
        mHeadsUpObserver.onChange(true);

    }



    /** Whether all alerts are disabled. */

    @VisibleForTesting

    public boolean areNotificationAlertsDisabled() {

        return mDisableNotificationAlerts;

    }



    /** Whether HUNs should be used. */

    @VisibleForTesting

    public boolean getUseHeadsUp() {

        return mUseHeadsUp;

    }



    protected NotificationPresenter getPresenter() {

        return mPresenter;

    }

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java

+1 −1
Original line number Diff line number Diff line
@@ -161,7 +161,7 @@ public final class NotificationEntry { 
     * <p>When a notification is a bubble we don't show it in the shade once the bubble has been

     * expanded</p>

     */

    private boolean mShowInShadeWhenBubble;

    private boolean mShowInShadeWhenBubble = true;



    /**

     * Whether the user has dismissed this notification when it was in bubble form.

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java

+0 −7
Original line number Diff line number Diff line
@@ -2310,9 +2310,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView 


    @Override

    public int getIntrinsicHeight() {

        if (isShownAsBubble()) {

            return getMaxExpandHeight();

        }

        if (isUserLocked()) {

            return getActualHeight();

        }
@@ -2358,10 +2355,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView 
        return mStatusbarStateController != null && mStatusbarStateController.isDozing();

    }



    private boolean isShownAsBubble() {

        return mEntry.isBubble() && !mEntry.showInShadeWhenBubble() && !mEntry.isBubbleDismissed();

    }



    @Override

    public boolean isGroupExpanded() {

        return mGroupManager.isGroupExpanded(mStatusBarNotification);

packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java

+8 −3
Original line number Diff line number Diff line
@@ -57,11 +57,13 @@ import androidx.test.filters.SmallTest; 


import com.android.systemui.R;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.plugins.statusbar.StatusBarStateController;

import com.android.systemui.statusbar.NotificationPresenter;

import com.android.systemui.statusbar.NotificationRemoveInterceptor;

import com.android.systemui.statusbar.NotificationTestHelper;

import com.android.systemui.statusbar.notification.NotificationEntryListener;

import com.android.systemui.statusbar.notification.NotificationEntryManager;

import com.android.systemui.statusbar.notification.NotificationFilter;

import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider;

import com.android.systemui.statusbar.notification.collection.NotificationData;

import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
@@ -171,7 +173,9 @@ public class BubbleControllerTest extends SysuiTestCase { 
        when(mZenModeController.getConfig()).thenReturn(mZenModeConfig);



        TestableNotificationInterruptionStateProvider interruptionStateProvider =

                new TestableNotificationInterruptionStateProvider(mContext);

                new TestableNotificationInterruptionStateProvider(mContext,

                        mock(NotificationFilter.class),

                        mock(StatusBarStateController.class));

        interruptionStateProvider.setUpWithPresenter(

                mock(NotificationPresenter.class),

                mock(HeadsUpManager.class),
@@ -647,8 +651,9 @@ public class BubbleControllerTest extends SysuiTestCase { 
    public static class TestableNotificationInterruptionStateProvider extends

            NotificationInterruptionStateProvider {



        public TestableNotificationInterruptionStateProvider(Context context) {

            super(context);

        public TestableNotificationInterruptionStateProvider(Context context,

                NotificationFilter filter, StatusBarStateController controller) {

            super(context, filter, controller);

            mUseHeadsUp = true;

        }

    }