DO NOT MERGE Filter out suppressed notifications in entry manager

`NotificationEntryManager#hasActiveNotifications` used to be in 1:1
correspondence with the visible notifications in the shade, and as such
was suitable for code that needs to do things like calculate the height
of the shade. However if there is a bubble, it's still backed by a
notification but is suppressed from the shade.

This CL is unfortunately hacky because the API for
NotificationEntryManager doesn't have the concept of "a notification
which exists but is not in the shade" and therefore the correct fix
actually would involve creating the correct api and updating all call
sites with the correct semantics.

Test: manual
Bug: 161461739
Change-Id: I9b0fc0f48609b64371d3cb7677228b4bb1fa6aef
Merged-In: I9b0fc0f48609b64371d3cb7677228b4bb1fa6aef
This commit is contained in:
Evan Laird
2020-07-29 17:56:58 -04:00
parent 243cd3ec1a
commit b88b1b68ad
10 changed files with 50 additions and 15 deletions

View File

@@ -36,6 +36,7 @@ import android.util.Log;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.statusbar.NotificationVisibility;
import com.android.systemui.Dumpable;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.statusbar.FeatureFlags;
import com.android.systemui.statusbar.NotificationLifetimeExtender;
import com.android.systemui.statusbar.NotificationListener;
@@ -189,6 +190,8 @@ public class NotificationEntryManager implements
}
}
private final Lazy<BubbleController> mBubbleControllerLazy;
/**
* Injected constructor. See {@link NotificationsModule}.
*/
@@ -201,6 +204,7 @@ public class NotificationEntryManager implements
Lazy<NotificationRowBinder> notificationRowBinderLazy,
Lazy<NotificationRemoteInputManager> notificationRemoteInputManagerLazy,
LeakDetector leakDetector,
Lazy<BubbleController> bubbleController,
ForegroundServiceDismissalFeatureController fgsFeatureController) {
mLogger = logger;
mGroupManager = groupManager;
@@ -211,6 +215,7 @@ public class NotificationEntryManager implements
mRemoteInputManagerLazy = notificationRemoteInputManagerLazy;
mLeakDetector = leakDetector;
mFgsFeatureController = fgsFeatureController;
mBubbleControllerLazy = bubbleController;
}
/** Once called, the NEM will start processing notification events from system server. */
@@ -920,8 +925,20 @@ public class NotificationEntryManager implements
/**
* @return {@code true} if there is at least one notification that should be visible right now
*/
public boolean hasActiveNotifications() {
return mReadOnlyNotifications.size() != 0;
public boolean hasVisibleNotifications() {
if (mReadOnlyNotifications.size() == 0) {
return false;
}
// Filter out suppressed notifications, which are active notifications backing a bubble
// but are not present in the shade
for (NotificationEntry e : mSortedAndFiltered) {
if (!mBubbleControllerLazy.get().isBubbleNotificationSuppressedFromShade(e)) {
return true;
}
}
return false;
}
@Override

View File

@@ -87,6 +87,7 @@ public interface NotificationsModule {
Lazy<NotificationRowBinder> notificationRowBinderLazy,
Lazy<NotificationRemoteInputManager> notificationRemoteInputManagerLazy,
LeakDetector leakDetector,
Lazy<BubbleController> bubbleController,
ForegroundServiceDismissalFeatureController fgsFeatureController) {
return new NotificationEntryManager(
logger,
@@ -97,6 +98,7 @@ public interface NotificationsModule {
notificationRowBinderLazy,
notificationRemoteInputManagerLazy,
leakDetector,
bubbleController,
fgsFeatureController);
}

View File

@@ -6580,7 +6580,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd
if (mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
return !mNotifPipeline.getShadeList().isEmpty();
} else {
return mEntryManager.hasActiveNotifications();
return mEntryManager.hasVisibleNotifications();
}
}

View File

@@ -97,7 +97,7 @@ public class LightsOutNotifController {
}
private boolean hasActiveNotifications() {
return mEntryManager.hasActiveNotifications();
return mEntryManager.hasVisibleNotifications();
}
@VisibleForTesting

View File

@@ -3003,7 +3003,7 @@ public class NotificationPanelViewController extends PanelViewController {
private void updateShowEmptyShadeView() {
boolean
showEmptyShadeView =
mBarState != StatusBarState.KEYGUARD && !mEntryManager.hasActiveNotifications();
mBarState != StatusBarState.KEYGUARD && !mEntryManager.hasVisibleNotifications();
showEmptyShadeView(showEmptyShadeView);
}

View File

@@ -330,7 +330,7 @@ public class StatusBarNotificationPresenter implements NotificationPresenter,
}
public boolean hasActiveNotifications() {
return mEntryManager.hasActiveNotifications();
return mEntryManager.hasVisibleNotifications();
}
@Override

View File

@@ -61,6 +61,7 @@ import com.android.internal.statusbar.NotificationVisibility;
import com.android.systemui.Dependency;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.statusbar.FeatureFlags;
import com.android.systemui.statusbar.NotificationLifetimeExtender;
import com.android.systemui.statusbar.NotificationMediaManager;
@@ -98,6 +99,8 @@ import java.util.Collection;
import java.util.List;
import java.util.Set;
import dagger.Lazy;
/**
* Unit tests for {@link NotificationEntryManager}. This test will not test any interactions with
* inflation. Instead, for functional inflation tests, see
@@ -126,6 +129,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
@Mock private LeakDetector mLeakDetector;
@Mock private NotificationMediaManager mNotificationMediaManager;
@Mock private NotificationRowBinder mNotificationRowBinder;
@Mock private Lazy<BubbleController> mBubbleControllerLazy;
private int mId;
private NotificationEntry mEntry;
@@ -200,6 +204,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
() -> mNotificationRowBinder,
() -> mRemoteInputManager,
mLeakDetector,
mBubbleControllerLazy,
mock(ForegroundServiceDismissalFeatureController.class)
);
mEntryManager.setUpWithPresenter(mPresenter);

View File

@@ -43,6 +43,7 @@ import androidx.test.filters.SmallTest;
import com.android.internal.util.NotificationMessagingUtil;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.plugins.FalsingManager;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.shared.plugins.PluginManager;
@@ -94,6 +95,8 @@ import org.mockito.stubbing.Answer;
import java.util.concurrent.CountDownLatch;
import dagger.Lazy;
/**
* Functional tests for notification inflation from {@link NotificationEntryManager}.
*/
@@ -135,6 +138,8 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
@Mock private NotificationRowComponent.Builder mNotificationRowComponentBuilder;
@Mock private PeopleNotificationIdentifier mPeopleNotificationIdentifier;
@Mock private Lazy<BubbleController> mBubbleControllerLazy;
private StatusBarNotification mSbn;
private NotificationListenerService.RankingMap mRankingMap;
private NotificationEntryManager mEntryManager;
@@ -182,6 +187,7 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
() -> mRowBinder,
() -> mRemoteInputManager,
mLeakDetector,
mBubbleControllerLazy,
mock(ForegroundServiceDismissalFeatureController.class)
);

View File

@@ -52,6 +52,7 @@ import com.android.internal.logging.testing.UiEventLoggerFake;
import com.android.systemui.ExpandHelper;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.classifier.FalsingManagerFake;
import com.android.systemui.media.KeyguardMediaController;
import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin;
@@ -109,6 +110,8 @@ import org.mockito.junit.MockitoRule;
import java.util.ArrayList;
import java.util.List;
import dagger.Lazy;
/**
* Tests for {@link NotificationStackScrollLayout}.
*/
@@ -140,6 +143,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
@Mock private NotificationSection mNotificationSection;
@Mock private NotificationLockscreenUserManager mLockscreenUserManager;
@Mock private FeatureFlags mFeatureFlags;
@Mock private Lazy<BubbleController> mBubbleControllerLazy;
private UserChangedListener mUserChangedListener;
private NotificationEntryManager mEntryManager;
private int mOriginalInterruptionModelSetting;
@@ -190,6 +194,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
() -> mock(NotificationRowBinder.class),
() -> mRemoteInputManager,
mock(LeakDetector.class),
mBubbleControllerLazy,
mock(ForegroundServiceDismissalFeatureController.class)
);
mEntryManager.setUpWithPresenter(mock(NotificationPresenter.class));

View File

@@ -130,7 +130,7 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testLightsOut_withNotifs_onSystemBarAppearanceChanged() {
// GIVEN active visible notifications
when(mEntryManager.hasActiveNotifications()).thenReturn(true);
when(mEntryManager.hasVisibleNotifications()).thenReturn(true);
// WHEN lights out
mCallbacks.onSystemBarAppearanceChanged(
@@ -147,7 +147,7 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testLightsOut_withoutNotifs_onSystemBarAppearanceChanged() {
// GIVEN no active visible notifications
when(mEntryManager.hasActiveNotifications()).thenReturn(false);
when(mEntryManager.hasVisibleNotifications()).thenReturn(false);
// WHEN lights out
mCallbacks.onSystemBarAppearanceChanged(
@@ -164,7 +164,7 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testLightsOn_afterLightsOut_onSystemBarAppearanceChanged() {
// GIVEN active visible notifications
when(mEntryManager.hasActiveNotifications()).thenReturn(true);
when(mEntryManager.hasVisibleNotifications()).thenReturn(true);
// WHEN lights on
mCallbacks.onSystemBarAppearanceChanged(
@@ -181,13 +181,13 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testEntryAdded() {
// GIVEN no visible notifications and lights out
when(mEntryManager.hasActiveNotifications()).thenReturn(false);
when(mEntryManager.hasVisibleNotifications()).thenReturn(false);
mLightsOutNotifController.mAppearance = LIGHTS_OUT;
mLightsOutNotifController.updateLightsOutView();
assertIsShowingDot(false);
// WHEN an active notification is added
when(mEntryManager.hasActiveNotifications()).thenReturn(true);
when(mEntryManager.hasVisibleNotifications()).thenReturn(true);
assertTrue(mLightsOutNotifController.shouldShowDot());
mEntryListener.onNotificationAdded(mock(NotificationEntry.class));
@@ -198,13 +198,13 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testEntryRemoved() {
// GIVEN a visible notification and lights out
when(mEntryManager.hasActiveNotifications()).thenReturn(true);
when(mEntryManager.hasVisibleNotifications()).thenReturn(true);
mLightsOutNotifController.mAppearance = LIGHTS_OUT;
mLightsOutNotifController.updateLightsOutView();
assertIsShowingDot(true);
// WHEN all active notifications are removed
when(mEntryManager.hasActiveNotifications()).thenReturn(false);
when(mEntryManager.hasVisibleNotifications()).thenReturn(false);
assertFalse(mLightsOutNotifController.shouldShowDot());
mEntryListener.onEntryRemoved(
mock(NotificationEntry.class), null, false, REASON_CANCEL_ALL);
@@ -216,13 +216,13 @@ public class LightsOutNotifControllerTest extends SysuiTestCase {
@Test
public void testEntryUpdated() {
// GIVEN no visible notifications and lights out
when(mEntryManager.hasActiveNotifications()).thenReturn(false);
when(mEntryManager.hasVisibleNotifications()).thenReturn(false);
mLightsOutNotifController.mAppearance = LIGHTS_OUT;
mLightsOutNotifController.updateLightsOutView();
assertIsShowingDot(false);
// WHEN an active notification is added
when(mEntryManager.hasActiveNotifications()).thenReturn(true);
when(mEntryManager.hasVisibleNotifications()).thenReturn(true);
assertTrue(mLightsOutNotifController.shouldShowDot());
mEntryListener.onPostEntryUpdated(mock(NotificationEntry.class));