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

Commit 01ac8cd9 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

Group screen record "error starting" notifs and "error saving" notifs.

This ensures that the notification title will always say "Screen
Recorder", and never "System UI".

Fixes: 356586297
Flag: EXEMPT bugfix

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: I18922eb9f12d8ed92592c777fe8102325a312a37
parent ece03333
Loading
Loading
Loading
Loading
+32 −17
Original line number Diff line number Diff line
@@ -63,7 +63,9 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
    protected 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";
    @VisibleForTesting static final String GROUP_KEY_SAVED = "screen_record_saved";
    private static final String GROUP_KEY_ERROR_STARTING = "screen_record_error_starting";
    @VisibleForTesting static final String GROUP_KEY_ERROR_SAVING = "screen_record_error_saving";
    private static final String EXTRA_RESULT_CODE = "extra_resultCode";
    protected static final String EXTRA_PATH = "extra_path";
    private static final String EXTRA_AUDIO_SOURCE = "extra_useAudio";
@@ -181,7 +183,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                    mUiEventLogger.log(Events.ScreenRecordEvent.SCREEN_RECORD_START);
                } else {
                    updateState(false);
                    createErrorStartingNotification();
                    createErrorStartingNotification(currentUser);
                    stopForeground(STOP_FOREGROUND_DETACH);
                    stopSelf();
                    return Service.START_NOT_STICKY;
@@ -276,8 +278,8 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     * errors.
     */
    @VisibleForTesting
    protected void createErrorStartingNotification() {
        createErrorNotification(strings().getStartError());
    protected void createErrorStartingNotification(UserHandle currentUser) {
        createErrorNotification(currentUser, strings().getStartError(), GROUP_KEY_ERROR_STARTING);
    }

    /**
@@ -285,17 +287,22 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
     * errors.
     */
    @VisibleForTesting
    protected void createErrorSavingNotification() {
        createErrorNotification(strings().getSaveError());
    protected void createErrorSavingNotification(UserHandle currentUser) {
        createErrorNotification(currentUser, strings().getSaveError(), GROUP_KEY_ERROR_SAVING);
    }

    private void createErrorNotification(String notificationContentTitle) {
    private void createErrorNotification(
            UserHandle currentUser, String notificationContentTitle, String groupKey) {
        // Make sure error notifications get their own group.
        postGroupSummaryNotification(currentUser, notificationContentTitle, groupKey);

        Bundle extras = new Bundle();
        extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, strings().getTitle());

        Notification.Builder builder = new Notification.Builder(this, getChannelId())
                .setSmallIcon(R.drawable.ic_screenrecord)
                .setContentTitle(notificationContentTitle)
                .setGroup(groupKey)
                .addExtras(extras);
        startForeground(mNotificationId, builder.build());
    }
@@ -350,7 +357,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                .setContentText(
                        strings().getBackgroundProcessingLabel())
                .setSmallIcon(R.drawable.ic_screenrecord)
                .setGroup(GROUP_KEY)
                .setGroup(GROUP_KEY_SAVED)
                .addExtras(extras);
        return builder.build();
    }
@@ -387,7 +394,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                        PendingIntent.FLAG_IMMUTABLE))
                .addAction(shareAction)
                .setAutoCancel(true)
                .setGroup(GROUP_KEY)
                .setGroup(GROUP_KEY_SAVED)
                .addExtras(extras);

        // Add thumbnail if available
@@ -402,21 +409,28 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
    }

    /**
     * Adds a group notification so that save notifications from multiple recordings are
     * grouped together, and the foreground service recording notification is not
     * 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.
     */
    private void postGroupNotification(UserHandle currentUser) {
    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) {
        Bundle extras = new Bundle();
        extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME,
                strings().getTitle());
        Notification groupNotif = new Notification.Builder(this, getChannelId())
                .setSmallIcon(R.drawable.ic_screenrecord)
                .setContentTitle(strings().getSaveTitle())
                .setGroup(GROUP_KEY)
                .setContentTitle(notificationContentTitle)
                .setGroup(groupKey)
                .setGroupSummary(true)
                .setExtras(extras)
                .build();
        mNotificationManager.notifyAsUser(getTag(), NOTIF_BASE_ID, groupNotif, currentUser);
        mNotificationManager.notifyAsUser(getTag(), mNotificationId, groupNotif, currentUser);
    }

    private void stopService() {
@@ -427,6 +441,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
        if (userId == USER_ID_NOT_SPECIFIED) {
            userId = mUserContextTracker.getUserContext().getUserId();
        }
        UserHandle currentUser = new UserHandle(userId);
        Log.d(getTag(), "notifying for user " + userId);
        setTapsVisible(mOriginalShowTaps);
        try {
@@ -444,7 +459,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
            Log.e(getTag(), "stopRecording called, but there was an error when ending"
                    + "recording");
            exception.printStackTrace();
            createErrorSavingNotification();
            createErrorSavingNotification(currentUser);
        } catch (Throwable throwable) {
            if (getRecorder() != null) {
                // Something unexpected happen, SystemUI will crash but let's delete
@@ -468,7 +483,7 @@ public class RecordingService extends Service implements ScreenMediaRecorderList
                Log.d(getTag(), "saving recording");
                Notification notification = createSaveNotification(
                        getRecorder() != null ? getRecorder().save() : null);
                postGroupNotification(currentUser);
                postGroupSummaryNotificationForSaves(currentUser);
                mNotificationManager.notifyAsUser(null, mNotificationId,  notification,
                        currentUser);
            } catch (IOException | IllegalStateException e) {
+38 −7
Original line number Diff line number Diff line
@@ -16,8 +16,13 @@

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 org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
@@ -25,6 +30,7 @@ import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

@@ -73,8 +79,6 @@ public class RecordingServiceTest extends SysuiTestCase {
    @Mock
    private ScreenMediaRecorder mScreenMediaRecorder;
    @Mock
    private Notification mNotification;
    @Mock
    private Executor mExecutor;
    @Mock
    private Handler mHandler;
@@ -124,10 +128,6 @@ public class RecordingServiceTest extends SysuiTestCase {

        // Mock notifications
        doNothing().when(mRecordingService).createRecordingNotification();
        doReturn(mNotification).when(mRecordingService).createProcessingNotification();
        doReturn(mNotification).when(mRecordingService).createSaveNotification(any());
        doNothing().when(mRecordingService).createErrorStartingNotification();
        doNothing().when(mRecordingService).createErrorSavingNotification();
        doNothing().when(mRecordingService).showErrorToast(anyInt());
        doNothing().when(mRecordingService).stopForeground(anyInt());

@@ -227,6 +227,33 @@ public class RecordingServiceTest extends SysuiTestCase {
        verify(mScreenMediaRecorder).release();
    }

    @Test
    public void testOnSystemRequestedStop_whenRecordingInProgress_showsNotifications() {
        doReturn(true).when(mController).isRecording();

        mRecordingService.onStopped();

        // Processing notification
        ArgumentCaptor<Notification> notifCaptor = ArgumentCaptor.forClass(Notification.class);
        verify(mNotificationManager).notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());
        assertEquals(GROUP_KEY_SAVED, notifCaptor.getValue().getGroup());

        reset(mNotificationManager);
        verify(mExecutor).execute(mRunnableCaptor.capture());
        mRunnableCaptor.getValue().run();

        verify(mNotificationManager, times(2))
                .notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());
        // Saved notification
        Notification saveNotification = notifCaptor.getAllValues().get(0);
        assertFalse(saveNotification.isGroupSummary());
        assertEquals(GROUP_KEY_SAVED, saveNotification.getGroup());
        // Group summary notification
        Notification groupSummaryNotification = notifCaptor.getAllValues().get(1);
        assertTrue(groupSummaryNotification.isGroupSummary());
        assertEquals(GROUP_KEY_SAVED, groupSummaryNotification.getGroup());
    }

    @Test
    public void testOnSystemRequestedStop_recorderEndThrowsRuntimeException_showsErrorNotification()
            throws IOException {
@@ -235,7 +262,11 @@ public class RecordingServiceTest extends SysuiTestCase {

        mRecordingService.onStopped();

        verify(mRecordingService).createErrorSavingNotification();
        verify(mRecordingService).createErrorSavingNotification(any());
        ArgumentCaptor<Notification> notifCaptor = ArgumentCaptor.forClass(Notification.class);
        verify(mNotificationManager).notifyAsUser(any(), anyInt(), notifCaptor.capture(), any());
        assertTrue(notifCaptor.getValue().isGroupSummary());
        assertEquals(GROUP_KEY_ERROR_SAVING, notifCaptor.getValue().getGroup());
    }

    @Test