Merge "make IStatusBarService:onNotificationClear calls async" into tm-qpr-dev am: c9358c61f0

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19338168

Change-Id: I1048d0f51f0aeb2435378caaaeb3e69e60790f9b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

packages/SystemUI/src/com/android/systemui/people/widget/LaunchConversationActivity.java

@@ -21,6 +21,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.pm.LauncherApps;
 import android.os.Bundle;
+import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.UserHandle;
 import android.os.UserManager;
@@ -34,6 +35,7 @@ import com.android.internal.logging.UiEventLogger;
 import com.android.internal.logging.UiEventLoggerImpl;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.statusbar.NotificationVisibility;
+import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.people.PeopleSpaceUtils;
 import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -43,6 +45,7 @@ import com.android.systemui.wmshell.BubblesManager;
 import com.android.wm.shell.bubbles.Bubble;
 
 import java.util.Optional;
+import java.util.concurrent.Executor;
 
 import javax.inject.Inject;
 
@@ -58,6 +61,7 @@ public class LaunchConversationActivity extends Activity {
     private boolean mIsForTesting;
     private IStatusBarService mIStatusBarService;
     private CommandQueue mCommandQueue;
+    private Executor mBgExecutor;
     private Bubble mBubble;
     private NotificationEntry mEntryToBubble;
 
@@ -67,7 +71,8 @@ public class LaunchConversationActivity extends Activity {
             CommonNotifCollection commonNotifCollection,
             Optional<BubblesManager> bubblesManagerOptional,
             UserManager userManager,
-            CommandQueue commandQueue
+            CommandQueue commandQueue,
+            @Background Executor bgExecutor
     ) {
         super();
         mVisibilityProvider = visibilityProvider;
@@ -91,6 +96,7 @@ public class LaunchConversationActivity extends Activity {
                 mCommandQueue.removeCallback(this);
             }
         });
+        mBgExecutor = bgExecutor;
     }
 
     @Override
@@ -172,34 +178,36 @@ public class LaunchConversationActivity extends Activity {
             return;
         }
 
-        try {
+        if (mIStatusBarService == null || mCommonNotifCollection == null) {
-            if (mIStatusBarService == null || mCommonNotifCollection == null) {
+            if (DEBUG) {
-                if (DEBUG) {
+                Log.d(TAG, "Skipping clear notification: null services, key: " + notifKey);
-                    Log.d(TAG, "Skipping clear notification: null services, key: " + notifKey);
-                }
-                return;
             }
-
+            return;
-            NotificationEntry entry = mCommonNotifCollection.getEntry(notifKey);
-            if (entry == null || entry.getRanking() == null) {
-                if (DEBUG) {
-                    Log.d(TAG, "Skipping clear notification: NotificationEntry or its Ranking"
-                            + " is null, key: " + notifKey);
-                }
-                return;
-            }
-
-            NotificationVisibility notifVisibility = mVisibilityProvider.obtain(entry, true);
-            int rank = notifVisibility.rank;
-
-            if (DEBUG) Log.d(TAG, "Clearing notification, key: " + notifKey + ", rank: " + rank);
-            mIStatusBarService.onNotificationClear(
-                    packageName, userHandle.getIdentifier(), notifKey,
-                    NotificationStats.DISMISSAL_OTHER,
-                    NotificationStats.DISMISS_SENTIMENT_POSITIVE, notifVisibility);
-        } catch (Exception e) {
-            Log.e(TAG, "Exception cancelling notification:" + e);
         }
+
+        NotificationEntry entry = mCommonNotifCollection.getEntry(notifKey);
+        if (entry == null || entry.getRanking() == null) {
+            if (DEBUG) {
+                Log.d(TAG, "Skipping clear notification: NotificationEntry or its Ranking"
+                        + " is null, key: " + notifKey);
+            }
+            return;
+        }
+
+        NotificationVisibility notifVisibility = mVisibilityProvider.obtain(entry, true);
+        int rank = notifVisibility.rank;
+
+        if (DEBUG) Log.d(TAG, "Clearing notification, key: " + notifKey + ", rank: " + rank);
+        mBgExecutor.execute(() -> {
+            try {
+                mIStatusBarService.onNotificationClear(
+                        packageName, userHandle.getIdentifier(), notifKey,
+                        NotificationStats.DISMISSAL_OTHER,
+                        NotificationStats.DISMISS_SENTIMENT_POSITIVE, notifVisibility);
+            } catch (RemoteException e) {
+                Log.e(TAG, "Exception cancelling notification:" + e);
+            }
+        });
     }
 
     @VisibleForTesting

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

@@ -41,6 +41,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.statusbar.NotificationVisibility;
 import com.android.systemui.Dumpable;
+import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.dump.DumpManager;
 import com.android.systemui.statusbar.NotificationLifetimeExtender;
 import com.android.systemui.statusbar.NotificationListener;
@@ -73,6 +74,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Executor;
 
 import dagger.Lazy;
 
@@ -113,6 +115,7 @@ public class NotificationEntryManager implements
     private final IStatusBarService mStatusBarService;
     private final NotifLiveDataStoreImpl mNotifLiveDataStore;
     private final DumpManager mDumpManager;
+    private final Executor mBgExecutor;
 
     private final Set<NotificationEntry> mAllNotifications = new ArraySet<>();
     private final Set<NotificationEntry> mReadOnlyAllNotifications =
@@ -159,7 +162,8 @@ public class NotificationEntryManager implements
             LeakDetector leakDetector,
             IStatusBarService statusBarService,
             NotifLiveDataStoreImpl notifLiveDataStore,
-            DumpManager dumpManager
+            DumpManager dumpManager,
+            @Background Executor bgExecutor
     ) {
         mLogger = logger;
         mGroupManager = groupManager;
@@ -170,6 +174,7 @@ public class NotificationEntryManager implements
         mStatusBarService = statusBarService;
         mNotifLiveDataStore = notifLiveDataStore;
         mDumpManager = dumpManager;
+        mBgExecutor = bgExecutor;
     }
 
     /** Once called, the NEM will start processing notification events from system server. */
@@ -566,17 +571,19 @@ public class NotificationEntryManager implements
     private void sendNotificationRemovalToServer(
             StatusBarNotification notification,
             DismissedByUserStats dismissedByUserStats) {
-        try {
+        mBgExecutor.execute(() -> {
-            mStatusBarService.onNotificationClear(
+            try {
-                    notification.getPackageName(),
+                mStatusBarService.onNotificationClear(
-                    notification.getUser().getIdentifier(),
+                        notification.getPackageName(),
-                    notification.getKey(),
+                        notification.getUser().getIdentifier(),
-                    dismissedByUserStats.dismissalSurface,
+                        notification.getKey(),
-                    dismissedByUserStats.dismissalSentiment,
+                        dismissedByUserStats.dismissalSurface,
-                    dismissedByUserStats.notificationVisibility);
+                        dismissedByUserStats.dismissalSentiment,
-        } catch (RemoteException ex) {
+                        dismissedByUserStats.notificationVisibility);
-            // system process is dead if we're here.
+            } catch (RemoteException ex) {
-        }
+                // system process is dead if we're here.
+            }
+        });
     }
 
     /**

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java

@@ -70,6 +70,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.systemui.Dumpable;
 import com.android.systemui.dagger.SysUISingleton;
+import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.dump.DumpManager;
 import com.android.systemui.dump.LogBufferEulogizer;
@@ -112,6 +113,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Queue;
 import java.util.Set;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
@@ -146,6 +148,7 @@ public class NotifCollection implements Dumpable {
     private final NotifPipelineFlags mNotifPipelineFlags;
     private final NotifCollectionLogger mLogger;
     private final Handler mMainHandler;
+    private final Executor mBgExecutor;
     private final LogBufferEulogizer mEulogizer;
     private final DumpManager mDumpManager;
 
@@ -174,6 +177,7 @@ public class NotifCollection implements Dumpable {
             NotifPipelineFlags notifPipelineFlags,
             NotifCollectionLogger logger,
             @Main Handler mainHandler,
+            @Background Executor bgExecutor,
             LogBufferEulogizer logBufferEulogizer,
             DumpManager dumpManager) {
         mStatusBarService = statusBarService;
@@ -181,6 +185,7 @@ public class NotifCollection implements Dumpable {
         mNotifPipelineFlags = notifPipelineFlags;
         mLogger = logger;
         mMainHandler = mainHandler;
+        mBgExecutor = bgExecutor;
         mEulogizer = logBufferEulogizer;
         mDumpManager = dumpManager;
     }
@@ -294,18 +299,20 @@ public class NotifCollection implements Dumpable {
             entriesToLocallyDismiss.add(entry);
             if (!isCanceled(entry)) {
                 // send message to system server if this notification hasn't already been cancelled
-                try {
+                mBgExecutor.execute(() -> {
-                    mStatusBarService.onNotificationClear(
+                    try {
-                            entry.getSbn().getPackageName(),
+                        mStatusBarService.onNotificationClear(
-                            entry.getSbn().getUser().getIdentifier(),
+                                entry.getSbn().getPackageName(),
-                            entry.getSbn().getKey(),
+                                entry.getSbn().getUser().getIdentifier(),
-                            stats.dismissalSurface,
+                                entry.getSbn().getKey(),
-                            stats.dismissalSentiment,
+                                stats.dismissalSurface,
-                            stats.notificationVisibility);
+                                stats.dismissalSentiment,
-                } catch (RemoteException e) {
+                                stats.notificationVisibility);
-                    // system process is dead if we're here.
+                    } catch (RemoteException e) {
-                    mLogger.logRemoteExceptionOnNotificationClear(entry, e);
+                        // system process is dead if we're here.
-                }
+                        mLogger.logRemoteExceptionOnNotificationClear(entry, e);
+                    }
+                });
             }
         }
 

packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java

@@ -128,7 +128,8 @@ public interface NotificationsModule {
             LeakDetector leakDetector,
             IStatusBarService statusBarService,
             NotifLiveDataStoreImpl notifLiveDataStore,
-            DumpManager dumpManager) {
+            DumpManager dumpManager,
+            @Background Executor bgExecutor) {
         return new NotificationEntryManager(
                 logger,
                 groupManager,
@@ -138,7 +139,8 @@ public interface NotificationsModule {
                 leakDetector,
                 statusBarService,
                 notifLiveDataStore,
-                dumpManager);
+                dumpManager,
+                bgExecutor);
     }
 
     /** Provides an instance of {@link NotificationGutsManager} */

packages/SystemUI/tests/src/com/android/systemui/people/widget/LaunchConversationActivityTest.java

@@ -48,6 +48,8 @@ import com.android.systemui.statusbar.CommandQueue;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection;
 import com.android.systemui.statusbar.notification.collection.render.NotificationVisibilityProvider;
+import com.android.systemui.util.concurrency.FakeExecutor;
+import com.android.systemui.util.time.FakeSystemClock;
 import com.android.systemui.wmshell.BubblesManager;
 import com.android.wm.shell.bubbles.Bubble;
 
@@ -106,6 +108,9 @@ public class LaunchConversationActivityTest extends SysuiTestCase {
 
     private Intent mIntent;
 
+    private FakeSystemClock mFakeSystemClock = new FakeSystemClock();
+    private FakeExecutor mBgExecutor = new FakeExecutor(mFakeSystemClock);
+
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -114,7 +119,8 @@ public class LaunchConversationActivityTest extends SysuiTestCase {
                 mNotifCollection,
                 Optional.of(mBubblesManager),
                 mUserManager,
-                mCommandQueue
+                mCommandQueue,
+                mBgExecutor
         );
         verify(mCommandQueue, times(1)).addCallback(mCallbacksCaptor.capture());
 
@@ -193,6 +199,7 @@ public class LaunchConversationActivityTest extends SysuiTestCase {
         // Ensure callback removed
         verify(mCommandQueue).removeCallback(any());
         // Clear the notification for bubbles.
+        FakeExecutor.exhaustExecutors(mBgExecutor);
         verify(mIStatusBarService, times(1)).onNotificationClear(any(),
                 anyInt(), any(), anyInt(), anyInt(), mNotificationVisibilityCaptor.capture());
         // Do not select the bubble.

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

@@ -19,6 +19,8 @@ package com.android.systemui.statusbar.notification;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
 import static android.service.notification.NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED;
 import static android.service.notification.NotificationListenerService.REASON_CANCEL;
+import static android.service.notification.NotificationStats.DISMISSAL_SHADE;
+import static android.service.notification.NotificationStats.DISMISS_SENTIMENT_NEUTRAL;
 
 import static com.android.systemui.statusbar.notification.NotificationEntryManager.UNDEFINED_DISMISS_REASON;
 
@@ -41,6 +43,7 @@ import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import android.app.ActivityManager;
@@ -50,6 +53,7 @@ import android.app.PendingIntent;
 import android.content.Intent;
 import android.graphics.drawable.Icon;
 import android.os.Handler;
+import android.os.RemoteException;
 import android.os.UserHandle;
 import android.service.notification.NotificationListenerService.Ranking;
 import android.service.notification.NotificationListenerService.RankingMap;
@@ -91,7 +95,9 @@ import com.android.systemui.statusbar.notification.row.NotificationEntryManagerI
 import com.android.systemui.statusbar.notification.row.RowInflaterTask;
 import com.android.systemui.statusbar.policy.DeviceProvisionedController;
 import com.android.systemui.statusbar.policy.HeadsUpManager;
+import com.android.systemui.util.concurrency.FakeExecutor;
 import com.android.systemui.util.leak.LeakDetector;
+import com.android.systemui.util.time.FakeSystemClock;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -138,9 +144,14 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
     @Mock private NotificationMediaManager mNotificationMediaManager;
     @Mock private NotificationRowBinder mNotificationRowBinder;
     @Mock private NotificationListener mNotificationListener;
+    @Mock private IStatusBarService mStatusBarService;
+
+    private FakeSystemClock mFakeSystemClock = new FakeSystemClock();
+    private FakeExecutor mBgExecutor = new FakeExecutor(mFakeSystemClock);
 
     private int mId;
     private NotificationEntry mEntry;
+    private DismissedByUserStats mStats;
     private StatusBarNotification mSbn;
     private NotificationEntryManager mEntryManager;
 
@@ -191,6 +202,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                 Handler.createAsync(TestableLooper.get(this).getLooper()));
 
         mEntry = createNotification();
+        mStats = defaultStats(mEntry);
         mSbn = mEntry.getSbn();
 
         when(mNotifPipelineFlags.isNewPipelineEnabled()).thenReturn(false);
@@ -201,9 +213,10 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                 () -> mNotificationRowBinder,
                 () -> mRemoteInputManager,
                 mLeakDetector,
-                mock(IStatusBarService.class),
+                mStatusBarService,
                 NotifLiveDataStoreMocksKt.createNotifLiveDataStoreImplMock(),
-                mock(DumpManager.class)
+                mock(DumpManager.class),
+                mBgExecutor
         );
         mEntryManager.initialize(
                 mNotificationListener,
@@ -315,6 +328,31 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
         assertFalse(entriesContainKey(mEntryManager.getAllNotifs(), mSbn.getKey()));
     }
 
+    @Test
+    public void testPerformRemoveNotification_sendRemovalToServer() throws RemoteException {
+        // GIVEN an entry manager with a notification
+        mEntryManager.addActiveNotificationForTest(mEntry);
+
+        // GIVEN interceptor that doesn't intercept
+        when(mRemoveInterceptor.onNotificationRemoveRequested(
+                eq(mEntry.getKey()), argThat(matchEntryOnKey()), anyInt()))
+                .thenReturn(false);
+
+        // WHEN the notification entry is removed
+        mEntryManager.performRemoveNotification(mSbn, mStats, REASON_CANCEL);
+
+        // THEN notification removal is sent to the server
+        FakeExecutor.exhaustExecutors(mBgExecutor);
+        verify(mStatusBarService).onNotificationClear(
+                mSbn.getPackageName(),
+                mSbn.getUser().getIdentifier(),
+                mSbn.getKey(),
+                mStats.dismissalSurface,
+                mStats.dismissalSentiment,
+                mStats.notificationVisibility);
+        verifyNoMoreInteractions(mStatusBarService);
+    }
+
     @Test
     public void testRemoveNotification_onEntryRemoveNotFiredIfEntryDoesntExist() {
 
@@ -573,23 +611,6 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                 any(NotificationVisibility.class), anyBoolean(), eq(UNDEFINED_DISMISS_REASON));
     }
 
-    private NotificationEntry createNotification() {
-        Notification.Builder n = new Notification.Builder(mContext, "id")
-                .setSmallIcon(R.drawable.ic_person)
-                .setContentTitle("Title")
-                .setContentText("Text");
-
-        return new NotificationEntryBuilder()
-                .setPkg(TEST_PACKAGE_NAME)
-                .setOpPkg(TEST_PACKAGE_NAME)
-                .setUid(TEST_UID)
-                .setId(mId++)
-                .setNotification(n.build())
-                .setChannel(new NotificationChannel("id", "", IMPORTANCE_DEFAULT))
-                .setUser(new UserHandle(ActivityManager.getCurrentUser()))
-                .build();
-    }
-
     /* Tests annexed from NotificationDataTest go here */
 
     @Test
@@ -713,4 +734,28 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
             return mManagedNotifs.contains(notificationKey);
         }
     }
+
+    private NotificationEntry createNotification() {
+        Notification.Builder n = new Notification.Builder(mContext, "id")
+                .setSmallIcon(R.drawable.ic_person)
+                .setContentTitle("Title")
+                .setContentText("Text");
+
+        return new NotificationEntryBuilder()
+                .setPkg(TEST_PACKAGE_NAME)
+                .setOpPkg(TEST_PACKAGE_NAME)
+                .setUid(TEST_UID)
+                .setId(mId++)
+                .setNotification(n.build())
+                .setChannel(new NotificationChannel("id", "", IMPORTANCE_DEFAULT))
+                .setUser(new UserHandle(ActivityManager.getCurrentUser()))
+                .build();
+    }
+
+    private static DismissedByUserStats defaultStats(NotificationEntry entry) {
+        return new DismissedByUserStats(
+                DISMISSAL_SHADE,
+                DISMISS_SENTIMENT_NEUTRAL,
+                NotificationVisibility.obtain(entry.getKey(), 7, 2, true));
+    }
 }

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

@@ -92,6 +92,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No
 import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger;
 import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor;
 import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
+import com.android.systemui.util.concurrency.FakeExecutor;
 import com.android.systemui.util.time.FakeSystemClock;
 
 import org.junit.Before;
@@ -146,6 +147,7 @@ public class NotifCollectionTest extends SysuiTestCase {
 
     private NoManSimulator mNoMan;
     private FakeSystemClock mClock = new FakeSystemClock();
+    private FakeExecutor mBgExecutor = new FakeExecutor(mClock);
 
     @Before
     public void setUp() {
@@ -162,6 +164,7 @@ public class NotifCollectionTest extends SysuiTestCase {
                 mNotifPipelineFlags,
                 mLogger,
                 mMainHandler,
+                mBgExecutor,
                 mEulogizer,
                 mock(DumpManager.class));
         mCollection.attach(mGroupCoalescer);
@@ -461,6 +464,8 @@ public class NotifCollectionTest extends SysuiTestCase {
         DismissedByUserStats stats = defaultStats(entry2);
         mCollection.dismissNotification(entry2, defaultStats(entry2));
 
+        FakeExecutor.exhaustExecutors(mBgExecutor);
+
         // THEN we send the dismissal to system server
         verify(mStatusBarService).onNotificationClear(
                 notif2.sbn.getPackageName(),
@@ -674,6 +679,8 @@ public class NotifCollectionTest extends SysuiTestCase {
         mInterceptor1.onEndInterceptionCallback.onEndDismissInterception(mInterceptor1, entry,
                 stats);
 
+        FakeExecutor.exhaustExecutors(mBgExecutor);
+
         // THEN we send the dismissal to system server
         verify(mStatusBarService).onNotificationClear(
                 eq(notif.sbn.getPackageName()),
@@ -1211,6 +1218,7 @@ public class NotifCollectionTest extends SysuiTestCase {
                         new Pair<>(entry2, defaultStats(entry2))));
 
         // THEN we send the dismissals to system server
+        FakeExecutor.exhaustExecutors(mBgExecutor);
         verify(mStatusBarService).onNotificationClear(
                 notif1.sbn.getPackageName(),
                 notif1.sbn.getUser().getIdentifier(),
@@ -1577,6 +1585,7 @@ public class NotifCollectionTest extends SysuiTestCase {
 
         // WHEN finally dismissing
         onDismiss.run();
+        FakeExecutor.exhaustExecutors(mBgExecutor);
         verify(mStatusBarService).onNotificationClear(any(), anyInt(), eq(notifEvent.key),
                 anyInt(), anyInt(), any());
         verifyNoMoreInteractions(mStatusBarService);

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

@@ -194,7 +194,8 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
                 mLeakDetector,
                 mock(IStatusBarService.class),
                 NotifLiveDataStoreMocksKt.createNotifLiveDataStoreImplMock(),
-                mock(DumpManager.class)
+                mock(DumpManager.class),
+                mBgExecutor
         );
         mEntryManager.initialize(
                 mNotificationListener,