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

Commit c277a319 authored by Ned Burns's avatar Ned Burns
Browse files

Fix race condition in NotificationListener

When our NotificationListener first connects to noman, it queries the
list of current notifications and then posts all of them to the
NEM/NotifCollection. However, this is a two-stage process: first it
queries the list of notifications and then it queries the current
ranking map (technically it's actually the reverse because the map is
cached, but the same principle still applies). Between these two calls,
it's possible for notifs to get added or removed.

As a result, it's possible for the ranking map that we receive to be
missing entries for some of the currently-active notifications. This
causes the NEM/NotifCollection to crash downstream since ranking objects
are required for all new notifs. To band-aid over this issue until noman
can give us an atomic query for both values, we just fill in the ranking
map with missing entries.

Fixes: 145236001
Test: atest SystemUITests:NotifCollectionTest
Test: manual
Change-Id: Id9a2df79e7c782c3e090c53f838d5869852e32d9
parent 0001dbbc
Loading
Loading
Loading
Loading
+43 −2
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import com.android.systemui.statusbar.phone.NotificationGroupManager;
import com.android.systemui.statusbar.phone.NotificationListenerWithPlugins;

import java.util.ArrayList;
import java.util.List;

import javax.inject.Inject;
import javax.inject.Singleton;
@@ -89,11 +90,22 @@ public class NotificationListener extends NotificationListenerWithPlugins {
        }
        final RankingMap currentRanking = getCurrentRanking();
        mMainHandler.post(() -> {
            // There's currently a race condition between the calls to getActiveNotifications() and
            // getCurrentRanking(). It's possible for the ranking that we store here to not contain
            // entries for every notification in getActiveNotifications(). To prevent downstream
            // crashes, we temporarily fill in these missing rankings with stubs.
            // See b/146011844 for long-term fix
            final List<Ranking> newRankings = new ArrayList<>();
            for (StatusBarNotification sbn : notifications) {
                newRankings.add(getRankingOrTemporaryStandIn(currentRanking, sbn.getKey()));
            }
            final RankingMap completeMap = new RankingMap(newRankings.toArray(new Ranking[0]));

            for (StatusBarNotification sbn : notifications) {
                if (mDownstreamListener != null) {
                    mDownstreamListener.onNotificationPosted(sbn, currentRanking);
                    mDownstreamListener.onNotificationPosted(sbn, completeMap);
                }
                mEntryManager.addNotification(sbn, currentRanking);
                mEntryManager.addNotification(sbn, completeMap);
            }
        });
        NotificationManager noMan = mContext.getSystemService(NotificationManager.class);
@@ -192,6 +204,35 @@ public class NotificationListener extends NotificationListenerWithPlugins {
        }
    }

    private static Ranking getRankingOrTemporaryStandIn(RankingMap rankingMap, String key) {
        Ranking ranking = new Ranking();
        if (!rankingMap.getRanking(key, ranking)) {
            ranking.populate(
                    key,
                    0,
                    false,
                    0,
                    0,
                    0,
                    null,
                    null,
                    null,
                    new ArrayList<>(),
                    new ArrayList<>(),
                    false,
                    0,
                    false,
                    0,
                    false,
                    new ArrayList<>(),
                    new ArrayList<>(),
                    false,
                    false
            );
        }
        return ranking;
    }

    public interface NotificationSettingsListener {

        default void onStatusBarIconsBehaviorChanged(boolean hideSilentStatusIcons) { }