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

Commit d08d5683 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

Ensure visible notifications do not reorder when they HUN.

Fixes: 203826051
Test: atest VisualStabilityCoordinatorTest
Change-Id: I1f903d5e56910130b2311d457b9ecd4ff8766985
parent b11a6728
Loading
Loading
Loading
Loading
+13 −4
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import com.android.systemui.dump.DumpManager;
import com.android.systemui.keyguard.WakefulnessLifecycle;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.shade.ShadeStateEvents;
import com.android.systemui.statusbar.notification.VisibilityLocationProvider;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -62,6 +63,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
    private final HeadsUpManager mHeadsUpManager;
    private final ShadeStateEvents mShadeStateEvents;
    private final StatusBarStateController mStatusBarStateController;
    private final VisibilityLocationProvider mVisibilityLocationProvider;
    private final VisualStabilityProvider mVisualStabilityProvider;
    private final WakefulnessLifecycle mWakefulnessLifecycle;

@@ -94,9 +96,11 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
            HeadsUpManager headsUpManager,
            ShadeStateEvents shadeStateEvents,
            StatusBarStateController statusBarStateController,
            VisibilityLocationProvider visibilityLocationProvider,
            VisualStabilityProvider visualStabilityProvider,
            WakefulnessLifecycle wakefulnessLifecycle) {
        mHeadsUpManager = headsUpManager;
        mVisibilityLocationProvider = visibilityLocationProvider;
        mVisualStabilityProvider = visualStabilityProvider;
        mWakefulnessLifecycle = wakefulnessLifecycle;
        mStatusBarStateController = statusBarStateController;
@@ -123,6 +127,11 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
    //  HUNs to the top of the shade
    private final NotifStabilityManager mNotifStabilityManager =
            new NotifStabilityManager("VisualStabilityCoordinator") {
                private boolean canMoveForHeadsUp(NotificationEntry entry) {
                    return entry != null && mHeadsUpManager.isAlerting(entry.getKey())
                            && !mVisibilityLocationProvider.isInVisibleLocation(entry);
                }

                @Override
                public void onBeginRun() {
                    mIsSuppressingPipelineRun = false;
@@ -140,7 +149,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
                @Override
                public boolean isGroupChangeAllowed(@NonNull NotificationEntry entry) {
                    final boolean isGroupChangeAllowedForEntry =
                            mReorderingAllowed || mHeadsUpManager.isAlerting(entry.getKey());
                            mReorderingAllowed || canMoveForHeadsUp(entry);
                    mIsSuppressingGroupChange |= !isGroupChangeAllowedForEntry;
                    return isGroupChangeAllowedForEntry;
                }
@@ -156,7 +165,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
                public boolean isSectionChangeAllowed(@NonNull NotificationEntry entry) {
                    final boolean isSectionChangeAllowedForEntry =
                            mReorderingAllowed
                                    || mHeadsUpManager.isAlerting(entry.getKey())
                                    || canMoveForHeadsUp(entry)
                                    || mEntriesThatCanChangeSection.containsKey(entry.getKey());
                    if (!isSectionChangeAllowedForEntry) {
                        mEntriesWithSuppressedSectionChange.add(entry.getKey());
@@ -165,8 +174,8 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable,
                }

                @Override
                public boolean isEntryReorderingAllowed(@NonNull ListEntry section) {
                    return mReorderingAllowed;
                public boolean isEntryReorderingAllowed(@NonNull ListEntry entry) {
                    return mReorderingAllowed || canMoveForHeadsUp(entry.getRepresentativeEntry());
                }

                @Override
+38 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.statusbar.notification.collection.provider

import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.statusbar.notification.VisibilityLocationProvider
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import javax.inject.Inject

/**
 * An injectable component which delegates the visibility location computation to a delegate which
 * can be initialized after the initial injection, generally because it's provided by a view.
 */
@SysUISingleton
class VisibilityLocationProviderDelegator @Inject constructor() : VisibilityLocationProvider {
    private var delegate: VisibilityLocationProvider? = null

    fun setDelegate(provider: VisibilityLocationProvider) {
        delegate = provider
    }

    override fun isInVisibleLocation(entry: NotificationEntry): Boolean =
        requireNotNull(this.delegate) { "delegate not initialized" }.isInVisibleLocation(entry)
}
+7 −0
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import com.android.systemui.shade.ShadeEventsModule;
import com.android.systemui.shade.ShadeExpansionStateManager;
import com.android.systemui.statusbar.NotificationListener;
import com.android.systemui.statusbar.notification.AssistantFeedbackController;
import com.android.systemui.statusbar.notification.VisibilityLocationProvider;
import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl;
import com.android.systemui.statusbar.notification.collection.NotifLiveDataStore;
import com.android.systemui.statusbar.notification.collection.NotifLiveDataStoreImpl;
@@ -51,6 +52,7 @@ import com.android.systemui.statusbar.notification.collection.inflation.OnUserIn
import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection;
import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
import com.android.systemui.statusbar.notification.collection.provider.NotificationVisibilityProviderImpl;
import com.android.systemui.statusbar.notification.collection.provider.VisibilityLocationProviderDelegator;
import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager;
import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManagerImpl;
import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager;
@@ -151,6 +153,11 @@ public interface NotificationsModule {
    @Binds
    NotifGutsViewManager bindNotifGutsViewManager(NotificationGutsManager notificationGutsManager);

    /** Provides an instance of {@link VisibilityLocationProvider} */
    @Binds
    VisibilityLocationProvider bindVisibilityLocationProvider(
            VisibilityLocationProviderDelegator visibilityLocationProviderDelegator);

    /** Provides an instance of {@link NotificationLogger} */
    @SysUISingleton
    @Provides
+6 −0
Original line number Diff line number Diff line
@@ -89,6 +89,7 @@ import com.android.systemui.statusbar.notification.collection.PipelineDumpable;
import com.android.systemui.statusbar.notification.collection.PipelineDumper;
import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
import com.android.systemui.statusbar.notification.collection.provider.VisibilityLocationProviderDelegator;
import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager;
import com.android.systemui.statusbar.notification.collection.render.NotifStackController;
import com.android.systemui.statusbar.notification.collection.render.NotifStats;
@@ -157,6 +158,7 @@ public class NotificationStackScrollLayoutController {
    private final NotifCollection mNotifCollection;
    private final UiEventLogger mUiEventLogger;
    private final NotificationRemoteInputManager mRemoteInputManager;
    private final VisibilityLocationProviderDelegator mVisibilityLocationProviderDelegator;
    private final ShadeController mShadeController;
    private final KeyguardMediaController mKeyguardMediaController;
    private final SysuiStatusBarStateController mStatusBarStateController;
@@ -638,6 +640,7 @@ public class NotificationStackScrollLayoutController {
            ShadeTransitionController shadeTransitionController,
            UiEventLogger uiEventLogger,
            NotificationRemoteInputManager remoteInputManager,
            VisibilityLocationProviderDelegator visibilityLocationProviderDelegator,
            ShadeController shadeController,
            InteractionJankMonitor jankMonitor,
            StackStateLogger stackLogger,
@@ -679,6 +682,7 @@ public class NotificationStackScrollLayoutController {
        mNotifCollection = notifCollection;
        mUiEventLogger = uiEventLogger;
        mRemoteInputManager = remoteInputManager;
        mVisibilityLocationProviderDelegator = visibilityLocationProviderDelegator;
        mShadeController = shadeController;
        mFeatureFlags = featureFlags;
        mNotificationTargetsHelper = notificationTargetsHelper;
@@ -750,6 +754,8 @@ public class NotificationStackScrollLayoutController {
        mNotificationRoundnessManager.setOnRoundingChangedCallback(mView::invalidate);
        mView.addOnExpandedHeightChangedListener(mNotificationRoundnessManager::setExpanded);

        mVisibilityLocationProviderDelegator.setDelegate(this::isInVisibleLocation);

        mTunerService.addTunable(
                (key, newValue) -> {
                    switch (key) {
+37 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package com.android.systemui.statusbar.notification.collection.coordinator;

import static com.google.common.truth.Truth.assertThat;

import static junit.framework.Assert.assertFalse;

import static org.junit.Assert.assertTrue;
@@ -38,6 +40,7 @@ import com.android.systemui.keyguard.WakefulnessLifecycle;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.shade.ShadeStateEvents;
import com.android.systemui.shade.ShadeStateEvents.ShadeStateEventsListener;
import com.android.systemui.statusbar.notification.VisibilityLocationProvider;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
import com.android.systemui.statusbar.notification.collection.GroupEntryBuilder;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -73,6 +76,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
    @Mock private Pluggable.PluggableListener<NotifStabilityManager> mInvalidateListener;
    @Mock private HeadsUpManager mHeadsUpManager;
    @Mock private ShadeStateEvents mShadeStateEvents;
    @Mock private VisibilityLocationProvider mVisibilityLocationProvider;
    @Mock private VisualStabilityProvider mVisualStabilityProvider;

    @Captor private ArgumentCaptor<WakefulnessLifecycle.Observer> mWakefulnessObserverCaptor;
@@ -100,6 +104,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
                mHeadsUpManager,
                mShadeStateEvents,
                mStatusBarStateController,
                mVisibilityLocationProvider,
                mVisualStabilityProvider,
                mWakefulnessLifecycle);

@@ -354,6 +359,38 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        verifyStabilityManagerWasInvalidated(times(1));
    }

    @Test
    public void testMovingVisibleHeadsUpNotAllowed() {
        // GIVEN stability enforcing conditions
        setPanelExpanded(true);
        setSleepy(false);

        // WHEN a notification is alerting and visible
        when(mHeadsUpManager.isAlerting(mEntry.getKey())).thenReturn(true);
        when(mVisibilityLocationProvider.isInVisibleLocation(any(NotificationEntry.class)))
                .thenReturn(true);

        // VERIFY the notification cannot be reordered
        assertThat(mNotifStabilityManager.isEntryReorderingAllowed(mEntry)).isFalse();
        assertThat(mNotifStabilityManager.isSectionChangeAllowed(mEntry)).isFalse();
    }

    @Test
    public void testMovingInvisibleHeadsUpAllowed() {
        // GIVEN stability enforcing conditions
        setPanelExpanded(true);
        setSleepy(false);

        // WHEN a notification is alerting but not visible
        when(mHeadsUpManager.isAlerting(mEntry.getKey())).thenReturn(true);
        when(mVisibilityLocationProvider.isInVisibleLocation(any(NotificationEntry.class)))
                .thenReturn(false);

        // VERIFY the notification can be reordered
        assertThat(mNotifStabilityManager.isEntryReorderingAllowed(mEntry)).isTrue();
        assertThat(mNotifStabilityManager.isSectionChangeAllowed(mEntry)).isTrue();
    }

    @Test
    public void testNeverSuppressedChanges_noInvalidationCalled() {
        // GIVEN no notifications are currently being suppressed from grouping nor being sorted
Loading