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

Commit 9512e0c7 authored by Ned Burns's avatar Ned Burns
Browse files

Allow notification reordering after user adjusts importance 

After the user adjusts the priority of a notification, we need to
temporarily lift the VisualStabilityManager's ban on reordering
to allow the priority adjustment to take effect.

There's no clear signal as to when this reprieve should end --
eventually, we'll get a ranking update, but there's no way to tell
that it was the update that our change triggered. So instead we just
wait for 1000ms and then drop the adjustment ban again. If a few
extra reorders slip in during that time, so be it.

Test: atest, manual
Bug: 129772718
Change-Id: Iefb6917bb41935ed6bfc720b51738c66ffa83457
parent 5c9415dd

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

+57 −5
Original line number Diff line number Diff line
@@ -16,18 +16,26 @@ 


package com.android.systemui.statusbar.notification;



import static com.android.systemui.Dependency.MAIN_HANDLER_NAME;



import android.os.Handler;

import android.os.SystemClock;

import android.view.View;



import androidx.collection.ArraySet;



import com.android.systemui.Dumpable;

import com.android.systemui.statusbar.NotificationPresenter;

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

import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;

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



import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.util.ArrayList;



import javax.inject.Inject;

import javax.inject.Named;

import javax.inject.Singleton;



/**
@@ -35,14 +43,19 @@ import javax.inject.Singleton; 
 * and reorder at the right time when they are out of view.

 */

@Singleton

public class VisualStabilityManager implements OnHeadsUpChangedListener {

public class VisualStabilityManager implements OnHeadsUpChangedListener, Dumpable {



    private static final long TEMPORARY_REORDERING_ALLOWED_DURATION = 1000;



    private final ArrayList<Callback> mCallbacks =  new ArrayList<>();

    private final Handler mHandler;



    private NotificationPresenter mPresenter;

    private boolean mPanelExpanded;

    private boolean mScreenOn;

    private boolean mReorderingAllowed;

    private boolean mIsTemporaryReorderingAllowed;

    private long mTemporaryReorderingStart;

    private VisibilityLocationProvider mVisibilityLocationProvider;

    private ArraySet<View> mAllowedReorderViews = new ArraySet<>();

    private ArraySet<NotificationEntry> mLowPriorityReorderingViews = new ArraySet<>();
@@ -50,7 +63,12 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener { 
    private boolean mPulsing;



    @Inject

    public VisualStabilityManager(NotificationEntryManager notificationEntryManager) {

    public VisualStabilityManager(

            NotificationEntryManager notificationEntryManager,

            @Named(MAIN_HANDLER_NAME) Handler handler) {



        mHandler = handler;



        notificationEntryManager.addNotificationEntryListener(new NotificationEntryListener() {

            @Override

            public void onPreEntryUpdated(NotificationEntry entry) {
@@ -114,10 +132,11 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener { 
    }



    private void updateReorderingAllowed() {

        boolean reorderingAllowed = (!mScreenOn || !mPanelExpanded) && !mPulsing;

        boolean changed = reorderingAllowed && !mReorderingAllowed;

        boolean reorderingAllowed =

                (!mScreenOn || !mPanelExpanded || mIsTemporaryReorderingAllowed) && !mPulsing;

        boolean changedToTrue = reorderingAllowed && !mReorderingAllowed;

        mReorderingAllowed = reorderingAllowed;

        if (changed) {

        if (changedToTrue) {

            notifyCallbacks();

        }

    }
@@ -179,6 +198,25 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener { 
        }

    }



    /**

     * Temporarily allows reordering of the entire shade for a period of 1000ms. Subsequent calls

     * to this method will extend the timer.

     */

    public void temporarilyAllowReordering() {

        mHandler.removeCallbacks(mOnTemporaryReorderingExpired);

        mHandler.postDelayed(mOnTemporaryReorderingExpired, TEMPORARY_REORDERING_ALLOWED_DURATION);

        if (!mIsTemporaryReorderingAllowed) {

            mTemporaryReorderingStart = SystemClock.elapsedRealtime();

        }

        mIsTemporaryReorderingAllowed = true;

        updateReorderingAllowed();

    }



    private final Runnable mOnTemporaryReorderingExpired = () -> {

        mIsTemporaryReorderingAllowed = false;

        updateReorderingAllowed();

    };



    /**

     * Notify the visual stability manager that a new view was added and should be allowed to

     * reorder next time.
@@ -187,6 +225,20 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener { 
        mAddedChildren.add(view);

    }



    @Override

    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {

        pw.println("VisualStabilityManager state:");

        pw.print("  mIsTemporaryReorderingAllowed="); pw.println(mIsTemporaryReorderingAllowed);

        pw.print("  mTemporaryReorderingStart="); pw.println(mTemporaryReorderingStart);



        long now = SystemClock.elapsedRealtime();

        pw.print("    Temporary reordering window has been open for ");

        pw.print(now - (mIsTemporaryReorderingAllowed ? mTemporaryReorderingStart : now));

        pw.println("ms");



        pw.println();

    }



    public interface Callback {

        /**

         * Called when reordering is allowed again.

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

+7 −1
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ import com.android.systemui.statusbar.NotificationLockscreenUserManager; 
import com.android.systemui.statusbar.NotificationPresenter;

import com.android.systemui.statusbar.StatusBarState;

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

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

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

import com.android.systemui.statusbar.notification.row.NotificationInfo.CheckSaveListener;

import com.android.systemui.statusbar.notification.stack.NotificationListContainer;
@@ -73,6 +74,7 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx 


    private final MetricsLogger mMetricsLogger = Dependency.get(MetricsLogger.class);

    private final Context mContext;

    private final VisualStabilityManager mVisualStabilityManager;

    private final AccessibilityManager mAccessibilityManager;



    // Dependencies:
@@ -96,8 +98,11 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx 
    protected String mKeyToRemoveOnGutsClosed;



    @Inject

    public NotificationGutsManager(Context context) {

    public NotificationGutsManager(

            Context context,

            VisualStabilityManager visualStabilityManager) {

        mContext = context;

        mVisualStabilityManager = visualStabilityManager;

        mAccessibilityManager = (AccessibilityManager)

                mContext.getSystemService(Context.ACCESSIBILITY_SERVICE);

    }
@@ -304,6 +309,7 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx 
        notificationInfoView.bindNotification(

                pmUser,

                iNotificationManager,

                mVisualStabilityManager,

                packageName,

                row.getEntry().channel,

                row.getUniqueChannels(),

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

+7 −1
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ import com.android.internal.logging.nano.MetricsProto.MetricsEvent; 
import com.android.systemui.Dependency;

import com.android.systemui.Interpolators;

import com.android.systemui.R;

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

import com.android.systemui.statusbar.notification.logging.NotificationCounters;



import java.lang.annotation.Retention;
@@ -104,6 +105,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
    private INotificationManager mINotificationManager;

    private PackageManager mPm;

    private MetricsLogger mMetricsLogger;

    private VisualStabilityManager mVisualStabilityManager;

    private ChannelEditorDialogController mChannelEditorDialogController;



    private String mPackageName;
@@ -244,6 +246,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
    void bindNotification(

            final PackageManager pm,

            final INotificationManager iNotificationManager,

            final VisualStabilityManager visualStabilityManager,

            final String pkg,

            final NotificationChannel notificationChannel,

            final Set<NotificationChannel> uniqueChannelsInRow,
@@ -256,7 +259,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
            int importance,

            boolean wasShownHighPriority)

            throws RemoteException {

        bindNotification(pm, iNotificationManager, pkg, notificationChannel,

        bindNotification(pm, iNotificationManager, visualStabilityManager, pkg, notificationChannel,

                uniqueChannelsInRow, sbn, checkSaveListener, onSettingsClick,

                onAppSettingsClick, isDeviceProvisioned, isNonblockable,

                false /* isBlockingHelper */,
@@ -266,6 +269,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
    public void bindNotification(

            PackageManager pm,

            INotificationManager iNotificationManager,

            VisualStabilityManager visualStabilityManager,

            String pkg,

            NotificationChannel notificationChannel,

            Set<NotificationChannel> uniqueChannelsInRow,
@@ -281,6 +285,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
            throws RemoteException {

        mINotificationManager = iNotificationManager;

        mMetricsLogger = Dependency.get(MetricsLogger.class);

        mVisualStabilityManager = visualStabilityManager;

        mChannelEditorDialogController = Dependency.get(ChannelEditorDialogController.class);

        mPackageName = pkg;

        mUniqueChannelsInRow = uniqueChannelsInRow;
@@ -537,6 +542,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G 
                    new UpdateImportanceRunnable(mINotificationManager, mPackageName, mAppUid,

                            mNumUniqueChannelsInRow == 1 ? mSingleNotificationChannel : null,

                            mStartingChannelImportance, newImportance));

            mVisualStabilityManager.temporarilyAllowReordering();

        }

    }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/VisualStabilityManagerTest.java

+84 −27
Original line number Diff line number Diff line
@@ -16,17 +16,20 @@ 


package com.android.systemui.statusbar.notification;



import static junit.framework.Assert.assertEquals;



import static org.mockito.Matchers.anyObject;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.os.Handler;

import android.service.notification.StatusBarNotification;

import android.test.suitebuilder.annotation.SmallTest;

import android.testing.AndroidTestingRunner;

import android.testing.TestableLooper;



import androidx.test.runner.AndroidJUnit4;

import androidx.test.filters.SmallTest;



import com.android.systemui.SysuiTestCase;

import com.android.systemui.statusbar.NotificationPresenter;
@@ -38,11 +41,13 @@ import org.junit.Test; 
import org.junit.runner.RunWith;



@SmallTest

@RunWith(AndroidJUnit4.class)

@RunWith(AndroidTestingRunner.class)

@TestableLooper.RunWithLooper()

public class VisualStabilityManagerTest extends SysuiTestCase {



    private VisualStabilityManager mVisualStabilityManager = new VisualStabilityManager(

            mock(NotificationEntryManager.class));

    private TestableLooper mTestableLooper;



    private VisualStabilityManager mVisualStabilityManager;

    private VisualStabilityManager.Callback mCallback = mock(VisualStabilityManager.Callback.class);

    private VisibilityLocationProvider mLocationProvider = mock(VisibilityLocationProvider.class);

    private ExpandableNotificationRow mRow = mock(ExpandableNotificationRow.class);
@@ -50,46 +55,53 @@ public class VisualStabilityManagerTest extends SysuiTestCase { 


    @Before

    public void setUp() {

        mTestableLooper = TestableLooper.get(this);

        mVisualStabilityManager = new VisualStabilityManager(

                mock(NotificationEntryManager.class),

                new Handler(mTestableLooper.getLooper()));



        mVisualStabilityManager.setUpWithPresenter(mock(NotificationPresenter.class));

        mVisualStabilityManager.setVisibilityLocationProvider(mLocationProvider);

        mEntry = new NotificationEntry(mock(StatusBarNotification.class));

        mEntry.setRow(mRow);



        when(mRow.getEntry()).thenReturn(mEntry);

    }



    @Test

    public void testPanelExpansion() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), false);

        assertFalse(mVisualStabilityManager.canReorderNotification(mRow));

        mVisualStabilityManager.setPanelExpanded(false);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), true);

        assertTrue(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testScreenOn() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), false);

        assertFalse(mVisualStabilityManager.canReorderNotification(mRow));

        mVisualStabilityManager.setScreenOn(false);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), true);

        assertTrue(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testReorderingAllowedChangesScreenOn() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), false);

        assertFalse(mVisualStabilityManager.isReorderingAllowed());

        mVisualStabilityManager.setScreenOn(false);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), true);

        assertTrue(mVisualStabilityManager.isReorderingAllowed());

    }



    @Test

    public void testReorderingAllowedChangesPanel() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), false);

        assertFalse(mVisualStabilityManager.isReorderingAllowed());

        mVisualStabilityManager.setPanelExpanded(false);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), true);

        assertTrue(mVisualStabilityManager.isReorderingAllowed());

    }



    @Test
@@ -126,51 +138,51 @@ public class VisualStabilityManagerTest extends SysuiTestCase { 
        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        mVisualStabilityManager.notifyViewAddition(mRow);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), true);

        assertTrue(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testReorderingVisibleHeadsUpNotAllowed() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        when(mLocationProvider.isInVisibleLocation(anyObject())).thenReturn(true);

        when(mLocationProvider.isInVisibleLocation(any(NotificationEntry.class))).thenReturn(true);

        mVisualStabilityManager.onHeadsUpStateChanged(mEntry, true);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), false);

        assertFalse(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testReorderingVisibleHeadsUpAllowed() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        when(mLocationProvider.isInVisibleLocation(anyObject())).thenReturn(false);

        when(mLocationProvider.isInVisibleLocation(any(NotificationEntry.class))).thenReturn(false);

        mVisualStabilityManager.onHeadsUpStateChanged(mEntry, true);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), true);

        assertTrue(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testReorderingVisibleHeadsUpAllowedOnce() {

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.setScreenOn(true);

        when(mLocationProvider.isInVisibleLocation(anyObject())).thenReturn(false);

        when(mLocationProvider.isInVisibleLocation(any(NotificationEntry.class))).thenReturn(false);

        mVisualStabilityManager.onHeadsUpStateChanged(mEntry, true);

        mVisualStabilityManager.onReorderingFinished();

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), false);

        assertFalse(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testPulsing() {

        mVisualStabilityManager.setPulsing(true);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), false);

        assertFalse(mVisualStabilityManager.canReorderNotification(mRow));

        mVisualStabilityManager.setPulsing(false);

        assertEquals(mVisualStabilityManager.canReorderNotification(mRow), true);

        assertTrue(mVisualStabilityManager.canReorderNotification(mRow));

    }



    @Test

    public void testReorderingAllowedChanges_Pulsing() {

        mVisualStabilityManager.setPulsing(true);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), false);

        assertFalse(mVisualStabilityManager.isReorderingAllowed());

        mVisualStabilityManager.setPulsing(false);

        assertEquals(mVisualStabilityManager.isReorderingAllowed(), true);

        assertTrue(mVisualStabilityManager.isReorderingAllowed());

    }



    @Test
@@ -180,4 +192,49 @@ public class VisualStabilityManagerTest extends SysuiTestCase { 
        mVisualStabilityManager.setPulsing(false);

        verify(mCallback).onReorderingAllowed();

    }



    @Test

    public void testTemporarilyAllowReorderingNotifiesCallbacks() {

        // GIVEN having the panel open (which would block reordering)

        mVisualStabilityManager.setScreenOn(true);

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.addReorderingAllowedCallback(mCallback);



        // WHEN we temprarily allow reordering

        mVisualStabilityManager.temporarilyAllowReordering();



        // THEN callbacks are notified that reordering is allowed

        verify(mCallback).onReorderingAllowed();

        assertTrue(mVisualStabilityManager.isReorderingAllowed());

    }



    @Test

    public void testTemporarilyAllowReorderingDoesntOverridePulsing() {

        // GIVEN we are in a pulsing state

        mVisualStabilityManager.setPulsing(true);

        mVisualStabilityManager.addReorderingAllowedCallback(mCallback);



        // WHEN we temprarily allow reordering

        mVisualStabilityManager.temporarilyAllowReordering();



        // THEN reordering is still not allowed

        verify(mCallback, never()).onReorderingAllowed();

        assertFalse(mVisualStabilityManager.isReorderingAllowed());

    }



    @Test

    public void testTemporarilyAllowReorderingExpires() {

        // GIVEN having the panel open (which would block reordering)

        mVisualStabilityManager.setScreenOn(true);

        mVisualStabilityManager.setPanelExpanded(true);

        mVisualStabilityManager.addReorderingAllowedCallback(mCallback);



        // WHEN we temprarily allow reordering and then wait until the window expires

        mVisualStabilityManager.temporarilyAllowReordering();

        assertTrue(mVisualStabilityManager.isReorderingAllowed());

        mTestableLooper.processMessages(1);



        // THEN reordering is no longer allowed

        assertFalse(mVisualStabilityManager.isReorderingAllowed());

    }

}

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

+9 −1
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; 
import com.android.systemui.statusbar.NotificationPresenter;

import com.android.systemui.statusbar.NotificationTestHelper;

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

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

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

import com.android.systemui.statusbar.notification.row.NotificationGutsManager.OnSettingsClickListener;

import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout;
@@ -97,6 +98,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 


    @Rule public MockitoRule mockito = MockitoJUnit.rule();

    @Mock private MetricsLogger mMetricsLogger;

    @Mock private VisualStabilityManager mVisualStabilityManager;

    @Mock private NotificationPresenter mPresenter;

    @Mock private NotificationActivityStarter mNotificationActivityStarter;

    @Mock private NotificationStackScrollLayout mStackScroller;
@@ -111,11 +113,12 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        mDependency.injectTestDependency(DeviceProvisionedController.class,

                mDeviceProvisionedController);

        mDependency.injectTestDependency(MetricsLogger.class, mMetricsLogger);

        mDependency.injectTestDependency(VisualStabilityManager.class, mVisualStabilityManager);

        mHandler = Handler.createAsync(mTestableLooper.getLooper());



        mHelper = new NotificationTestHelper(mContext);



        mGutsManager = new NotificationGutsManager(mContext);

        mGutsManager = new NotificationGutsManager(mContext, mVisualStabilityManager);

        mGutsManager.setUpWithPresenter(mPresenter, mStackScroller,

                mCheckSaveListener, mOnSettingsClickListener);

        mGutsManager.setNotificationActivityStarter(mNotificationActivityStarter);
@@ -316,6 +319,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        verify(notificationInfoView).bindNotification(

                any(PackageManager.class),

                any(INotificationManager.class),

                eq(mVisualStabilityManager),

                eq(statusBarNotification.getPackageName()),

                any(NotificationChannel.class),

                anySet(),
@@ -344,6 +348,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        verify(notificationInfoView).bindNotification(

                any(PackageManager.class),

                any(INotificationManager.class),

                eq(mVisualStabilityManager),

                eq(statusBarNotification.getPackageName()),

                any(NotificationChannel.class),

                anySet(),
@@ -374,6 +379,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        verify(notificationInfoView).bindNotification(

                any(PackageManager.class),

                any(INotificationManager.class),

                eq(mVisualStabilityManager),

                eq(statusBarNotification.getPackageName()),

                any(NotificationChannel.class),

                anySet(),
@@ -403,6 +409,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        verify(notificationInfoView).bindNotification(

                any(PackageManager.class),

                any(INotificationManager.class),

                eq(mVisualStabilityManager),

                eq(statusBarNotification.getPackageName()),

                any(NotificationChannel.class),

                anySet(),
@@ -431,6 +438,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { 
        verify(notificationInfoView).bindNotification(

                any(PackageManager.class),

                any(INotificationManager.class),

                eq(mVisualStabilityManager),

                eq(statusBarNotification.getPackageName()),

                any(NotificationChannel.class),

                anySet(),