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

Commit fb2287a1 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[Screen Record] Assign fixed IDs to screen record notification groups. 

Previously, the screen recording group notifications were using the ID
of one of the *individual* notifications. This meant that if you swiped
the group away, it actually dismissed just an individual notification,
*not* the whole group, because the group ID was equal to that individual
notification.

This CL instead uses fixed IDs for the group notifications so that
dismissing the group does indeed dismiss the *whole* group, not just a
single notification within the group.

Fixes: 368837832
Flag: EXEMPT BUGFIX

Tests to verify b/368837832 is fixed:
Test: Start and finish multiple successful recordings, then swipe away
the whole "Recording saved" group -> verify whole group is dismissed
and individual notifications aren't re-shown
Test: Start and finish multiple failed recordings (e.g. choose to record
a single app but close that app during the countdown and never reopen
it), then swipe away the whole "Recording failed" group -> verify whole
group is dismissed and individual notifications aren't re-shown

Tests to make sure Change-Id I18922eb9f12d8ed92592c777fe8102325a312a37
didn't regress:
Test: Start and finish multiple successful recordings, and also start
and finish multiple failed recordings (e.g. choose to record a single
app, but close that app during the countdown and never reopen it) ->
Verify all "recording saved" notifs are grouped together, and all "error
saving" notifs are in a separate group. Verify the active "recording"
notif is always on its own.
Test: Have a failed recording, then switch users -> verify notif doesn't
appear
Test: atest RecordingServiceTest

Change-Id: I32450067933eb1250f8d149a6445a3d0d8257cea
parent bb1810c4

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

+15 −3
Original line number Diff line number Diff line
@@ -18,9 +18,12 @@ package com.android.systemui.screenrecord; 


import static com.android.systemui.screenrecord.RecordingService.GROUP_KEY_ERROR_SAVING;

import static com.android.systemui.screenrecord.RecordingService.GROUP_KEY_SAVED;

import static com.android.systemui.screenrecord.RecordingService.NOTIF_GROUP_ID_ERROR_SAVING;

import static com.android.systemui.screenrecord.RecordingService.NOTIF_GROUP_ID_SAVED;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotEquals;

import static org.junit.Assert.assertThrows;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;
@@ -235,7 +238,9 @@ public class RecordingServiceTest extends SysuiTestCase { 


        // Processing notification

        ArgumentCaptor<Notification> notifCaptor = ArgumentCaptor.forClass(Notification.class);

        verify(mNotificationManager).notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());

        ArgumentCaptor<Integer> notifIdCaptor = ArgumentCaptor.forClass(Integer.class);

        verify(mNotificationManager)

                .notifyAsUser(any(), notifIdCaptor.capture(), notifCaptor.capture(), any());

        assertEquals(GROUP_KEY_SAVED, notifCaptor.getValue().getGroup());



        reset(mNotificationManager);
@@ -243,7 +248,7 @@ public class RecordingServiceTest extends SysuiTestCase { 
        mRunnableCaptor.getValue().run();



        verify(mNotificationManager, times(2))

                .notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());

                .notifyAsUser(any(), notifIdCaptor.capture(), notifCaptor.capture(), any());

        // Saved notification

        Notification saveNotification = notifCaptor.getAllValues().get(0);

        assertFalse(saveNotification.isGroupSummary());
@@ -252,6 +257,10 @@ public class RecordingServiceTest extends SysuiTestCase { 
        Notification groupSummaryNotification = notifCaptor.getAllValues().get(1);

        assertTrue(groupSummaryNotification.isGroupSummary());

        assertEquals(GROUP_KEY_SAVED, groupSummaryNotification.getGroup());



        // Verify the group notification ID and the individual notification ID are different

        assertNotEquals(NOTIF_GROUP_ID_SAVED, (int) notifIdCaptor.getAllValues().get(0));

        assertEquals(NOTIF_GROUP_ID_SAVED, (int) notifIdCaptor.getAllValues().get(1));

    }



    @Test
@@ -264,9 +273,12 @@ public class RecordingServiceTest extends SysuiTestCase { 


        verify(mRecordingService).createErrorSavingNotification(any());

        ArgumentCaptor<Notification> notifCaptor = ArgumentCaptor.forClass(Notification.class);

        verify(mNotificationManager).notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());

        ArgumentCaptor<Integer> notifIdCaptor = ArgumentCaptor.forClass(Integer.class);

        verify(mNotificationManager)

                .notifyAsUser(any(), notifIdCaptor.capture(), notifCaptor.capture(), any());

        assertTrue(notifCaptor.getValue().isGroupSummary());

        assertEquals(GROUP_KEY_ERROR_SAVING, notifCaptor.getValue().getGroup());

        assertEquals(NOTIF_GROUP_ID_ERROR_SAVING, (int) notifIdCaptor.getValue());

    }



    @Test

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

+38 −15
Original line number Diff line number Diff line
@@ -61,6 +61,9 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 


    private static final int USER_ID_NOT_SPECIFIED = -1;

    protected static final int NOTIF_BASE_ID = 4273;

    protected static final int NOTIF_GROUP_ID_SAVED = NOTIF_BASE_ID + 1;

    protected static final int NOTIF_GROUP_ID_ERROR_SAVING = NOTIF_BASE_ID + 2;

    protected static final int NOTIF_GROUP_ID_ERROR_STARTING = NOTIF_BASE_ID + 3;

    private static final String TAG = "RecordingService";

    private static final String CHANNEL_ID = "screen_record";

    @VisibleForTesting static final String GROUP_KEY_SAVED = "screen_record_saved";
@@ -280,7 +283,11 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 
     */

    @VisibleForTesting

    protected void createErrorStartingNotification(UserHandle currentUser) {

        createErrorNotification(currentUser, strings().getStartError(), GROUP_KEY_ERROR_STARTING);

        createErrorNotification(

                currentUser,

                strings().getStartError(),

                GROUP_KEY_ERROR_STARTING,

                NOTIF_GROUP_ID_ERROR_STARTING);

    }



    /**
@@ -289,13 +296,21 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 
     */

    @VisibleForTesting

    protected void createErrorSavingNotification(UserHandle currentUser) {

        createErrorNotification(currentUser, strings().getSaveError(), GROUP_KEY_ERROR_SAVING);

        createErrorNotification(

                currentUser,

                strings().getSaveError(),

                GROUP_KEY_ERROR_SAVING,

                NOTIF_GROUP_ID_ERROR_SAVING);

    }



    private void createErrorNotification(

            UserHandle currentUser, String notificationContentTitle, String groupKey) {

            UserHandle currentUser,

            String notificationContentTitle,

            String groupKey,

            int notificationIdForGroup) {

        // Make sure error notifications get their own group.

        postGroupSummaryNotification(currentUser, notificationContentTitle, groupKey);

        postGroupSummaryNotification(

                currentUser, notificationContentTitle, groupKey, notificationIdForGroup);



        Bundle extras = new Bundle();

        extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, strings().getTitle());
@@ -410,17 +425,20 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 
    }



    /**

     * Adds a group summary notification for save notifications so that save notifications from

     * multiple recordings are grouped together, and the foreground service recording notification

     * is not.

     * Posts a group summary notification for the given group.

     *

     * Notifications that should be grouped:

     *  - Save notifications

     *  - Error saving notifications

     *  - Error starting notifications

     *

     * The foreground service recording notification should never be grouped.

     */

    private void postGroupSummaryNotificationForSaves(UserHandle currentUser) {

        postGroupSummaryNotification(currentUser, strings().getSaveTitle(), GROUP_KEY_SAVED);

    }



    /** Posts a group summary notification for the given group. */

    private void postGroupSummaryNotification(

            UserHandle currentUser, String notificationContentTitle, String groupKey) {

            UserHandle currentUser,

            String notificationContentTitle,

            String groupKey,

            int notificationIdForGroup) {

        Bundle extras = new Bundle();

        extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,

                strings().getTitle());
@@ -431,7 +449,8 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 
                .setGroupSummary(true)

                .setExtras(extras)

                .build();

        mNotificationManager.notifyAsUser(getTag(), mNotificationId, groupNotif, currentUser);

        mNotificationManager.notifyAsUser(

                getTag(), notificationIdForGroup, groupNotif, currentUser);

    }



    private void stopService() {
@@ -484,7 +503,11 @@ public class RecordingService extends Service implements ScreenMediaRecorderList 
                Log.d(getTag(), "saving recording");

                Notification notification = createSaveNotification(

                        getRecorder() != null ? getRecorder().save() : null);

                postGroupSummaryNotificationForSaves(currentUser);

                postGroupSummaryNotification(

                        currentUser,

                        strings().getSaveTitle(),

                        GROUP_KEY_SAVED,

                        NOTIF_GROUP_ID_SAVED);

                mNotificationManager.notifyAsUser(null, mNotificationId,  notification,

                        currentUser);

            } catch (IOException | IllegalStateException e) {