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

Commit 5042913a authored by András Kurucz's avatar András Kurucz
Browse files

[Flexiglass] Don't allow notification reordering while interacting with a HUN

When the user drags on a HUN to expand the shade, but within the same
gesture changes the swipe directon to snooze the HUN instead, we can get
into a temporarily state, where the HUN is already scheduled for removal
from the HeadsUpManager, but it still needs to be kept around.
The legacy stack solves this by NPVC reading HUM#isTrackingHeadsUp(),
and artifically keeping the shade expanded, until the user finishes the
swipe. This logic doesn't exist in Flexi, so this CL adds a new flag to
VisualStabilityCoordinator, which keeps isReorderingAllowed() false until
we have a tracked HUN.

Fixes: 391204020
Test: change swipe direction while snoozing a HUN
	- start swiping DOWN on a HUN (start opening the shade)
	- change the swipe direction to UP (decide to snooze instead)
        - Check: the HUN should be still displayed
Flag: com.android.systemui.scene_container
Change-Id: I5b138cd9c53ab86bc8a8f50d31c9b8331bd98546
parent 5611f3b3
Loading
Loading
Loading
Loading
+22 −21
Original line number Diff line number Diff line
@@ -22,9 +22,10 @@ import static com.google.common.truth.Truth.assertThat;

import static junit.framework.Assert.assertFalse;

import static kotlinx.coroutines.flow.StateFlowKt.MutableStateFlow;

import static org.junit.Assume.assumeFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@@ -36,8 +37,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import static kotlinx.coroutines.flow.StateFlowKt.MutableStateFlow;

import android.platform.test.annotations.EnableFlags;
import android.platform.test.flag.junit.FlagsParameterization;
import android.testing.TestableLooper;
@@ -74,13 +73,16 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntryB
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable;
import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider;
import com.android.systemui.statusbar.notification.data.repository.HeadsUpRepository;
import com.android.systemui.statusbar.notification.domain.interactor.SeenNotificationsInteractor;
import com.android.systemui.statusbar.notification.headsup.HeadsUpManager;
import com.android.systemui.statusbar.policy.KeyguardStateController;
import com.android.systemui.util.concurrency.FakeExecutor;
import com.android.systemui.util.kotlin.JavaAdapter;
import com.android.systemui.util.time.FakeSystemClock;

import kotlinx.coroutines.flow.MutableStateFlow;
import kotlinx.coroutines.test.TestScope;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -90,15 +92,12 @@ import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.verification.VerificationMode;

import java.util.List;
import java.util.Set;

import kotlinx.coroutines.flow.MutableStateFlow;
import kotlinx.coroutines.test.TestScope;

import platform.test.runner.parameterized.ParameterizedAndroidJunit4;
import platform.test.runner.parameterized.Parameters;

import java.util.List;
import java.util.Set;

@SmallTest
@RunWith(ParameterizedAndroidJunit4.class)
@TestableLooper.RunWithLooper
@@ -118,7 +117,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
    @Mock private StatusBarStateController mStatusBarStateController;
    @Mock private Pluggable.PluggableListener<NotifStabilityManager> mInvalidateListener;
    @Mock private SeenNotificationsInteractor mSeenNotificationsInteractor;
    @Mock private HeadsUpManager mHeadsUpManager;
    @Mock private HeadsUpRepository mHeadsUpRepository;
    @Mock private VisibilityLocationProvider mVisibilityLocationProvider;
    @Mock private VisualStabilityProvider mVisualStabilityProvider;
    @Mock private VisualStabilityCoordinatorLogger mLogger;
@@ -159,7 +158,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
                mFakeBackgroundExecutor,
                mFakeMainExecutor,
                mDumpManager,
                mHeadsUpManager,
                mHeadsUpRepository,
                mShadeAnimationInteractor,
                mJavaAdapter,
                mSeenNotificationsInteractor,
@@ -172,6 +171,8 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
                mKosmos.getKeyguardTransitionInteractor(),
                mKeyguardStateController,
                mLogger);

        when(mHeadsUpRepository.isTrackingHeadsUp()).thenReturn(MutableStateFlow(false));
        mCoordinator.attach(mNotifPipeline);
        mTestScope.getTestScheduler().runCurrent();

@@ -194,7 +195,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
                .setSummary(mEntry)
                .build();

        when(mHeadsUpManager.isHeadsUpEntry(mEntry.getKey())).thenReturn(false);
        when(mHeadsUpRepository.isHeadsUpEntry(mEntry.getKey())).thenReturn(false);

        // Whenever we invalidate, the pipeline runs again, so we invalidate the state
        doAnswer(i -> {
@@ -461,7 +462,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        setSleepy(false);

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

@@ -477,7 +478,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        setSleepy(false);

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

@@ -701,7 +702,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        assertFalse(mNotifStabilityManager.isSectionChangeAllowed(mEntry));

        // GIVEN mEntry is a HUN
        when(mHeadsUpManager.isHeadsUpEntry(mEntry.getKey())).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(mEntry.getKey())).thenReturn(true);

        // THEN group + section changes are allowed
        assertTrue(mNotifStabilityManager.isGroupChangeAllowed(mEntry));
@@ -768,7 +769,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        //  GIVEN - there is a group heads-up.
        String headsUpGroupKey = "heads_up_group_key";
        mCoordinator.setHeadsUpGroupKeys(Set.of(headsUpGroupKey));
        when(mHeadsUpManager.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);

        // GIVEN - HUN Group Summary
        final NotificationEntry nonHeadsUpGroupSummary = mock(NotificationEntry.class);
@@ -793,7 +794,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        //  GIVEN - there is a group heads-up.
        final String headsUpGroupKey = "heads_up_group_key";
        mCoordinator.setHeadsUpGroupKeys(Set.of(headsUpGroupKey));
        when(mHeadsUpManager.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);

        // GIVEN - HUN Group
        final NotificationEntry headsUpGroupSummary = mock(NotificationEntry.class);
@@ -825,7 +826,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        //  GIVEN - there is a group heads-up.
        final String headsUpGroupKey = "heads_up_group_key";
        mCoordinator.setHeadsUpGroupKeys(Set.of(headsUpGroupKey));
        when(mHeadsUpManager.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);

        // GIVEN - HUN Group
        final NotificationEntry headsUpGroupSummary = mock(NotificationEntry.class);
@@ -858,7 +859,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        //  GIVEN - there is a group heads-up.
        String headsUpGroupKey = "heads_up_group_key";
        mCoordinator.setHeadsUpGroupKeys(Set.of(headsUpGroupKey));
        when(mHeadsUpManager.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);

        // GIVEN - non HUN parent Group Summary
        final NotificationEntry groupSummary = mock(NotificationEntry.class);
@@ -891,7 +892,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase {
        // GIVEN - there is a group heads-up.
        final String headsUpGroupKey = "heads_up_group_key";
        mCoordinator.setHeadsUpGroupKeys(Set.of(headsUpGroupKey));
        when(mHeadsUpManager.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);
        when(mHeadsUpRepository.isHeadsUpEntry(headsUpGroupKey)).thenReturn(true);

        // GIVEN - HUN Group Summary
        final NotificationEntry headsUpGroupSummary = mock(NotificationEntry.class);
+18 −1
Original line number Diff line number Diff line
@@ -101,6 +101,7 @@ import com.android.systemui.wmshell.BubblesTestActivity;

import kotlin.coroutines.CoroutineContext;

import kotlinx.coroutines.flow.StateFlowKt;
import kotlinx.coroutines.test.TestScope;

import org.mockito.ArgumentCaptor;
@@ -166,11 +167,21 @@ public class NotificationTestHelper {
        this(context, dependency, testLooper, new FakeFeatureFlagsClassic());
    }


    public NotificationTestHelper(
            Context context,
            TestableDependency dependency,
            @Nullable TestableLooper testLooper,
            @NonNull FakeFeatureFlagsClassic featureFlags) {
        this(context, dependency, testLooper, featureFlags, mockHeadsUpManager());
    }

    public NotificationTestHelper(
            Context context,
            TestableDependency dependency,
            @Nullable TestableLooper testLooper,
            @NonNull FakeFeatureFlagsClassic featureFlags,
            @NonNull HeadsUpManager headsUpManager) {
        mContext = context;
        mFeatureFlags = Objects.requireNonNull(featureFlags);
        dependency.injectTestDependency(FeatureFlagsClassic.class, mFeatureFlags);
@@ -182,7 +193,7 @@ public class NotificationTestHelper {
        mKeyguardBypassController = mock(KeyguardBypassController.class);
        mGroupMembershipManager = mock(GroupMembershipManager.class);
        mGroupExpansionManager = mock(GroupExpansionManager.class);
        mHeadsUpManager = mock(HeadsUpManager.class);
        mHeadsUpManager = headsUpManager;
        mIconManager = new IconManager(
                mock(CommonNotifCollection.class),
                mock(LauncherApps.class),
@@ -689,6 +700,12 @@ public class NotificationTestHelper {
                .build();
    }

    private static HeadsUpManager mockHeadsUpManager() {
        HeadsUpManager mock = mock(HeadsUpManager.class);
        when(mock.isTrackingHeadsUp()).thenReturn(StateFlowKt.MutableStateFlow(false));
        return mock;
    }

    private static class MockSmartReplyInflater implements SmartReplyStateInflater {
        @Override
        public InflatedSmartReplyState inflateSmartReplyState(NotificationEntry entry) {
+3 −2
Original line number Diff line number Diff line
@@ -1785,7 +1785,8 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump
        // height - which means user is swiping down. Otherwise shade QS will either not show at all
        // with HUN movement or it will blink when touching HUN initially
        boolean qsShouldExpandWithHeadsUp = !mSplitShadeEnabled
                || (!mHeadsUpManager.isTrackingHeadsUp() || expandedHeight > mHeadsUpStartHeight);
                || (!mHeadsUpManager.isTrackingHeadsUp().getValue()
                || expandedHeight > mHeadsUpStartHeight);
        if (goingBetweenClosedShadeAndExpandedQs && qsShouldExpandWithHeadsUp) {
            float qsExpansionFraction;
            if (mSplitShadeEnabled) {
@@ -2045,7 +2046,7 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump
        // motion has the expected speed. We also only want this on non-lockscreen for now.
        if (mSplitShadeEnabled && mBarState == SHADE) {
            boolean transitionFromHeadsUp = (mHeadsUpManager != null
                    && mHeadsUpManager.isTrackingHeadsUp()) || mExpandingFromHeadsUp;
                    && mHeadsUpManager.isTrackingHeadsUp().getValue()) || mExpandingFromHeadsUp;
            // heads-up starting height is too close to mSplitShadeFullTransitionDistance and
            // when dragging HUN transition is already 90% complete. It makes shade become
            // immediately visible when starting to drag. We want to set distance so that
+25 −9
Original line number Diff line number Diff line
@@ -45,8 +45,8 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager;
import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider;
import com.android.systemui.statusbar.notification.data.repository.HeadsUpRepository;
import com.android.systemui.statusbar.notification.domain.interactor.SeenNotificationsInteractor;
import com.android.systemui.statusbar.notification.headsup.HeadsUpManager;
import com.android.systemui.statusbar.notification.shared.NotificationMinimalism;
import com.android.systemui.statusbar.policy.KeyguardStateController;
import com.android.systemui.util.concurrency.DelayableExecutor;
@@ -72,7 +72,7 @@ import javax.inject.Inject;
public class VisualStabilityCoordinator implements Coordinator, Dumpable {
    private final DelayableExecutor mDelayableExecutor;
    private final DelayableExecutor mMainExecutor;
    private final HeadsUpManager mHeadsUpManager;
    private final HeadsUpRepository mHeadsUpRepository;
    private final SeenNotificationsInteractor mSeenNotificationsInteractor;
    private final ShadeAnimationInteractor mShadeAnimationInteractor;
    private final StatusBarStateController mStatusBarStateController;
@@ -94,6 +94,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
    private boolean mNotifPanelLaunchingActivity;
    private boolean mCommunalShowing = false;
    private boolean mLockscreenShowing = false;
    private boolean mTrackingHeadsUp = false;
    private boolean mLockscreenInGoneTransition = false;
    private Set<String> mHeadsUpGroupKeys = new HashSet<>();

@@ -117,7 +118,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
            @Background DelayableExecutor delayableExecutor,
            @Main DelayableExecutor mainExecutor,
            DumpManager dumpManager,
            HeadsUpManager headsUpManager,
            HeadsUpRepository headsUpRepository,
            ShadeAnimationInteractor shadeAnimationInteractor,
            JavaAdapter javaAdapter,
            SeenNotificationsInteractor seenNotificationsInteractor,
@@ -130,7 +131,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
            KeyguardTransitionInteractor keyguardTransitionInteractor,
            KeyguardStateController keyguardStateController,
            VisualStabilityCoordinatorLogger logger) {
        mHeadsUpManager = headsUpManager;
        mHeadsUpRepository = headsUpRepository;
        mShadeAnimationInteractor = shadeAnimationInteractor;
        mJavaAdapter = javaAdapter;
        mSeenNotificationsInteractor = seenNotificationsInteractor;
@@ -177,6 +178,8 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
            mJavaAdapter.alwaysCollectFlow(mKeyguardTransitionInteractor.transitionValue(
                            KeyguardState.LOCKSCREEN),
                    this::onLockscreenKeyguardStateTransitionValueChanged);
            mJavaAdapter.alwaysCollectFlow(mHeadsUpRepository.isTrackingHeadsUp(),
                    this::onTrackingHeadsUpModeChanged);
        }

        if (Flags.checkLockscreenGoneTransition()) {
@@ -239,7 +242,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
                    boolean isTopUnseen = NotificationMinimalism.isEnabled()
                            && (mSeenNotificationsInteractor.isTopUnseenNotification(entry)
                                || mSeenNotificationsInteractor.isTopOngoingNotification(entry));
                    if (isTopUnseen || mHeadsUpManager.isHeadsUpEntry(entry.getKey())) {
                    if (isTopUnseen || mHeadsUpRepository.isHeadsUpEntry(entry.getKey())) {
                        return !mVisibilityLocationProvider.isInVisibleLocation(entry);
                    }
                    return false;
@@ -276,7 +279,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
                        return false;
                    }

                    return mHeadsUpManager.isHeadsUpEntry(summary.getKey());
                    return mHeadsUpRepository.isHeadsUpEntry(summary.getKey());
                }
                /**
                 * When reordering is enabled, non-heads-up groups can be pruned.
@@ -416,7 +419,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
                            if (summary == null) continue;

                            final String summaryKey = summary.getKey();
                            if (mHeadsUpManager.isHeadsUpEntry(summaryKey)) {
                            if (mHeadsUpRepository.isHeadsUpEntry(summaryKey)) {
                                currentHeadsUpKeys.add(summaryKey);
                            }
                        }
@@ -476,11 +479,19 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {

    private boolean isReorderingAllowed() {
        final boolean sleepyAndDozed = mFullyDozed && mSleepy;
        final boolean stackShowing = mPanelExpanded
                || (SceneContainerFlag.isEnabled() && mLockscreenShowing);
        final boolean stackShowing = isStackShowing();
        return (sleepyAndDozed || !stackShowing || mCommunalShowing) && !mPulsing;
    }

    /** @return true when the notification stack is visible to the user */
    private boolean isStackShowing() {
        if (SceneContainerFlag.isEnabled()) {
            return mPanelExpanded || mLockscreenShowing || mTrackingHeadsUp;
        } else {
            return mPanelExpanded;
        }
    }

    /**
     * Allows this notification entry to be re-ordered in the notification list temporarily until
     * the timeout has passed.
@@ -611,6 +622,11 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
        updateAllowedStates("lockscreenShowing", isShowing);
    }

    private void onTrackingHeadsUpModeChanged(boolean isTrackingHeadsUp) {
        mTrackingHeadsUp = isTrackingHeadsUp;
        updateAllowedStates("trackingHeadsUp", isTrackingHeadsUp);
    }

    private void onLockscreenInGoneTransitionChanged(boolean inGoneTransition) {
        if (!Flags.checkLockscreenGoneTransition()) {
            return;
+10 −0
Original line number Diff line number Diff line
@@ -40,6 +40,16 @@ interface HeadsUpRepository {
    /** Set of currently active top-level heads up rows to be displayed. */
    val activeHeadsUpRows: Flow<Set<HeadsUpRowRepository>>

    /** Whether the user is swiping on a heads up row */
    val isTrackingHeadsUp: Flow<Boolean>

    /** @return true if the actively managed heads up notifications contain an entry for this key */
    fun isHeadsUpEntry(key: String): Boolean

    /**
     * set whether a HUN is currently animation out, to keep its view container visible during the
     * animation
     */
    fun setHeadsUpAnimatingAway(animatingAway: Boolean)

    /** Snooze the currently pinned HUN. */
Loading