Merge "Allow notification reordering after user adjusts importance" into qt-dev

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

@@ -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 reorderingAllowed =
-        boolean changed = reorderingAllowed && !mReorderingAllowed;
+                (!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

@@ -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

@@ -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;
@@ -539,6 +544,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

@@ -16,17 +16,20 @@
 
 package com.android.systemui.statusbar.notification;
 
-import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
-
+import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.anyObject;
+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(
+    private TestableLooper mTestableLooper;
-            mock(NotificationEntryManager.class));
+
+    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

@@ -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(),