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

Commit cf74abb5 authored by Michael Mikhail's avatar Michael Mikhail
Browse files

Fix race condition using notification pipeline

It turns out that NotifPipeline returns a pointer to allNotifs field in
NotifCollection when getAllNotifs() is called. This list can be modified
by main thread which updates the list being sent to background. This
leads to a race condition.
This CL creates a list of StatusbarNotification being added from
notification entry returned from getAllNotifs() in main thread. So we
make sure the list of notifications being sent to background thread
should not get modified by main thread.
When notification entry is updated, sbn is being reset. Storing all
statusbar notifications ensures they don't get updated by main thread
when they are being accessed by background thread.

Flag: com.android.systemui.notification_media_manager_background_execution
Bug: 336612071
Test: atest NotificationMediaManagerTest

Change-Id: I84a7270b8f8367267ea0e870c0f615dfdffcf345
parent f7caa4df
Loading
Loading
Loading
Loading
+53 −3
Original line number Diff line number Diff line
@@ -303,7 +303,12 @@ public class NotificationMediaManager implements Dumpable {
        // TODO(b/169655907): get the semi-filtered notifications for current user
        Collection<NotificationEntry> allNotifications = mNotifPipeline.getAllNotifs();
        if (notificationMediaManagerBackgroundExecution()) {
            mBackgroundExecutor.execute(() -> findPlayingMediaNotification(allNotifications));
            // Create new sbn list to be accessed in background thread.
            List<StatusBarNotification> statusBarNotifications = new ArrayList<>();
            for (NotificationEntry entry: allNotifications) {
                statusBarNotifications.add(entry.getSbn());
            }
            mBackgroundExecutor.execute(() -> findPlayingMediaNotification(statusBarNotifications));
        } else {
            findPlayingMediaNotification(allNotifications);
        }
@@ -341,6 +346,51 @@ public class NotificationMediaManager implements Dumpable {
            }
        }

        StatusBarNotification statusBarNotification = null;
        if (mediaNotification != null) {
            statusBarNotification = mediaNotification.getSbn();
        }
        setUpControllerAndKey(controller, statusBarNotification);
    }

    /**
     * Find a notification and media controller associated with the playing media session, and
     * update this manager's internal state.
     * This method must be called in background.
     * TODO(b/273443374) check this method
     */
    void findPlayingMediaNotification(@NonNull List<StatusBarNotification> allNotifications) {
        // Promote the media notification with a controller in 'playing' state, if any.
        StatusBarNotification statusBarNotification = null;
        MediaController controller = null;
        for (StatusBarNotification sbn : allNotifications) {
            Notification notif = sbn.getNotification();
            if (notif.isMediaNotification()) {
                final MediaSession.Token token =
                        sbn.getNotification().extras.getParcelable(
                                Notification.EXTRA_MEDIA_SESSION, MediaSession.Token.class);
                if (token != null) {
                    MediaController aController = new MediaController(mContext, token);
                    if (PlaybackState.STATE_PLAYING
                            == getMediaControllerPlaybackState(aController)) {
                        if (DEBUG_MEDIA) {
                            Log.v(TAG, "DEBUG_MEDIA: found mediastyle controller matching "
                                    + sbn.getKey());
                        }
                        statusBarNotification = sbn;
                        controller = aController;
                        break;
                    }
                }
            }
        }

        setUpControllerAndKey(controller, statusBarNotification);
    }

    private void setUpControllerAndKey(
            MediaController controller,
            StatusBarNotification mediaNotification) {
        if (controller != null && !sameSessions(mMediaController, controller)) {
            // We have a new media session
            clearCurrentMediaNotificationSession();
@@ -354,8 +404,8 @@ public class NotificationMediaManager implements Dumpable {
        }

        if (mediaNotification != null
                && !mediaNotification.getSbn().getKey().equals(mMediaNotificationKey)) {
            mMediaNotificationKey = mediaNotification.getSbn().getKey();
                && !mediaNotification.getKey().equals(mMediaNotificationKey)) {
            mMediaNotificationKey = mediaNotification.getKey();
            if (DEBUG_MEDIA) {
                Log.v(TAG, "DEBUG_MEDIA: Found new media notification: key="
                        + mMediaNotificationKey);