Use a unique notification per screen recording

Instead of static notification IDs for recording / processing / saved, use a
new notification for each screen recording. Also adds a group key to the
saved notifications so that they are grouped together separately from
the FGS recording notification.

Fixes: 268404371
Fixes: 278954828
Test: atest RecordingServiceTest
Test: manual
Change-Id: I8e45fc57bc7b20a0bed3360390e9e8c6e98ab887

packages/SystemUI/src/com/android/systemui/screenrecord/RecordingService.java

@@ -33,6 +33,7 @@ import android.os.Bundle;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.RemoteException;
+import android.os.SystemClock;
 import android.os.UserHandle;
 import android.provider.Settings;
 import android.util.Log;
@@ -60,11 +61,10 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     public static final int REQUEST_CODE = 2;
 
     private static final int USER_ID_NOT_SPECIFIED = -1;
-    private static final int NOTIFICATION_RECORDING_ID = 4274;
-    private static final int NOTIFICATION_PROCESSING_ID = 4275;
-    private static final int NOTIFICATION_VIEW_ID = 4273;
+    private static final int NOTIF_BASE_ID = 4273;
     private static final String TAG = "RecordingService";
     private static final String CHANNEL_ID = "screen_record";
+    private static final String GROUP_KEY = "screen_record_saved";
     private static final String EXTRA_RESULT_CODE = "extra_resultCode";
     private static final String EXTRA_PATH = "extra_path";
     private static final String EXTRA_AUDIO_SOURCE = "extra_useAudio";
@@ -89,6 +89,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     private final UiEventLogger mUiEventLogger;
     private final NotificationManager mNotificationManager;
     private final UserContextProvider mUserContextTracker;
+    private int mNotificationId = NOTIF_BASE_ID;
 
     @Inject
     public RecordingService(RecordingController controller, @LongRunning Executor executor,
@@ -134,14 +135,23 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
         }
         String action = intent.getAction();
         Log.d(TAG, "onStartCommand " + action);
+        NotificationChannel channel = new NotificationChannel(
+                CHANNEL_ID,
+                getString(R.string.screenrecord_title),
+                NotificationManager.IMPORTANCE_DEFAULT);
+        channel.setDescription(getString(R.string.screenrecord_channel_description));
+        channel.enableVibration(true);
+        mNotificationManager.createNotificationChannel(channel);
 
         int currentUserId = mUserContextTracker.getUserContext().getUserId();
         UserHandle currentUser = new UserHandle(currentUserId);
         switch (action) {
             case ACTION_START:
+                // Get a unique ID for this recording's notifications
+                mNotificationId = NOTIF_BASE_ID + (int) SystemClock.uptimeMillis();
                 mAudioSource = ScreenRecordingAudioSource
                         .values()[intent.getIntExtra(EXTRA_AUDIO_SOURCE, 0)];
-                Log.d(TAG, "recording with audio source" + mAudioSource);
+                Log.d(TAG, "recording with audio source " + mAudioSource);
                 mShowTaps = intent.getBooleanExtra(EXTRA_SHOW_TAPS, false);
                 MediaProjectionCaptureTarget captureTarget =
                         intent.getParcelableExtra(EXTRA_CAPTURE_TARGET,
@@ -169,7 +179,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                 } else {
                     updateState(false);
                     createErrorNotification();
-                    stopForeground(true);
+                    stopForeground(STOP_FOREGROUND_DETACH);
                     stopSelf();
                     return Service.START_NOT_STICKY;
                 }
@@ -200,7 +210,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                     startActivity(Intent.createChooser(shareIntent, shareLabel)
                             .setFlags(Intent.FLAG_ACTIVITY_NEW_TASK));
                     // Remove notification
-                    mNotificationManager.cancelAsUser(null, NOTIFICATION_VIEW_ID, currentUser);
+                    mNotificationManager.cancelAsUser(null, mNotificationId, currentUser);
                     return false;
                 }, false, false);
 
@@ -260,14 +270,6 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     @VisibleForTesting
     protected void createErrorNotification() {
         Resources res = getResources();
-        NotificationChannel channel = new NotificationChannel(
-                CHANNEL_ID,
-                getString(R.string.screenrecord_title),
-                NotificationManager.IMPORTANCE_DEFAULT);
-        channel.setDescription(getString(R.string.screenrecord_channel_description));
-        channel.enableVibration(true);
-        mNotificationManager.createNotificationChannel(channel);
-
         Bundle extras = new Bundle();
         extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,
                 res.getString(R.string.screenrecord_title));
@@ -277,7 +279,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                 .setSmallIcon(R.drawable.ic_screenrecord)
                 .setContentTitle(notificationTitle)
                 .addExtras(extras);
-        startForeground(NOTIFICATION_RECORDING_ID, builder.build());
+        startForeground(mNotificationId, builder.build());
     }
 
     @VisibleForTesting
@@ -288,14 +290,6 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     @VisibleForTesting
     protected void createRecordingNotification() {
         Resources res = getResources();
-        NotificationChannel channel = new NotificationChannel(
-                CHANNEL_ID,
-                getString(R.string.screenrecord_title),
-                NotificationManager.IMPORTANCE_DEFAULT);
-        channel.setDescription(getString(R.string.screenrecord_channel_description));
-        channel.enableVibration(true);
-        mNotificationManager.createNotificationChannel(channel);
-
         Bundle extras = new Bundle();
         extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,
                 res.getString(R.string.screenrecord_title));
@@ -323,7 +317,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                 .setForegroundServiceBehavior(Notification.FOREGROUND_SERVICE_IMMEDIATE)
                 .addAction(stopAction)
                 .addExtras(extras);
-        startForeground(NOTIFICATION_RECORDING_ID, builder.build());
+        startForeground(mNotificationId, builder.build());
     }
 
     @VisibleForTesting
@@ -337,11 +331,12 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
         extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,
                 res.getString(R.string.screenrecord_title));
 
-        Notification.Builder builder = new Notification.Builder(getApplicationContext(), CHANNEL_ID)
+        Notification.Builder builder = new Notification.Builder(this, CHANNEL_ID)
                 .setContentTitle(notificationTitle)
                 .setContentText(
                         getResources().getString(R.string.screenrecord_background_processing_label))
                 .setSmallIcon(R.drawable.ic_screenrecord)
+                .setGroup(GROUP_KEY)
                 .addExtras(extras);
         return builder.build();
     }
@@ -378,6 +373,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                         PendingIntent.FLAG_IMMUTABLE))
                 .addAction(shareAction)
                 .setAutoCancel(true)
+                .setGroup(GROUP_KEY)
                 .addExtras(extras);
 
         // Add thumbnail if available
@@ -391,6 +387,24 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
         return builder.build();
     }
 
+    /**
+     * Adds a group notification so that save notifications from multiple recordings are
+     * grouped together, and the foreground service recording notification is not
+     */
+    private void postGroupNotification(UserHandle currentUser) {
+        Bundle extras = new Bundle();
+        extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,
+                getResources().getString(R.string.screenrecord_title));
+        Notification groupNotif = new Notification.Builder(this, CHANNEL_ID)
+                .setSmallIcon(R.drawable.ic_screenrecord)
+                .setContentTitle(getResources().getString(R.string.screenrecord_save_title))
+                .setGroup(GROUP_KEY)
+                .setGroupSummary(true)
+                .setExtras(extras)
+                .build();
+        mNotificationManager.notifyAsUser(TAG, NOTIF_BASE_ID, groupNotif, currentUser);
+    }
+
     private void stopService() {
         stopService(USER_ID_NOT_SPECIFIED);
     }
@@ -423,27 +437,26 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
             Log.e(TAG, "stopRecording called, but recorder was null");
         }
         updateState(false);
+        stopForeground(STOP_FOREGROUND_DETACH);
         stopSelf();
     }
 
     private void saveRecording(int userId) {
         UserHandle currentUser = new UserHandle(userId);
-        mNotificationManager.notifyAsUser(null, NOTIFICATION_PROCESSING_ID,
+        mNotificationManager.notifyAsUser(null, mNotificationId,
                 createProcessingNotification(), currentUser);
 
         mLongExecutor.execute(() -> {
             try {
                 Log.d(TAG, "saving recording");
                 Notification notification = createSaveNotification(getRecorder().save());
-                if (!mController.isRecording()) {
-                    mNotificationManager.notifyAsUser(null, NOTIFICATION_VIEW_ID, notification,
-                            currentUser);
-                }
+                postGroupNotification(currentUser);
+                mNotificationManager.notifyAsUser(null, mNotificationId,  notification,
+                        currentUser);
             } catch (IOException e) {
                 Log.e(TAG, "Error saving screen recording: " + e.getMessage());
                 showErrorToast(R.string.screenrecord_delete_error);
-            } finally {
-                mNotificationManager.cancelAsUser(null, NOTIFICATION_PROCESSING_ID, currentUser);
+                mNotificationManager.cancelAsUser(null, mNotificationId, currentUser);
             }
         });
     }

packages/SystemUI/tests/src/com/android/systemui/screenrecord/RecordingServiceTest.java

@@ -18,7 +18,6 @@ package com.android.systemui.screenrecord;
 
 import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
@@ -94,6 +93,7 @@ public class RecordingServiceTest extends SysuiTestCase {
         doReturn(mContext.getUserId()).when(mRecordingService).getUserId();
         doReturn(mContext.getPackageName()).when(mRecordingService).getPackageName();
         doReturn(mContext.getContentResolver()).when(mRecordingService).getContentResolver();
+        doReturn(mContext.getResources()).when(mRecordingService).getResources();
 
         // Mock notifications
         doNothing().when(mRecordingService).createRecordingNotification();
@@ -101,7 +101,7 @@ public class RecordingServiceTest extends SysuiTestCase {
         doReturn(mNotification).when(mRecordingService).createSaveNotification(any());
         doNothing().when(mRecordingService).createErrorNotification();
         doNothing().when(mRecordingService).showErrorToast(anyInt());
-        doNothing().when(mRecordingService).stopForeground(anyBoolean());
+        doNothing().when(mRecordingService).stopForeground(anyInt());
 
         doNothing().when(mRecordingService).startForeground(anyInt(), any());
         doReturn(mScreenMediaRecorder).when(mRecordingService).getRecorder();