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

Commit 203f80a0 authored by Ned Burns's avatar Ned Burns
Browse files

Don't throw exceptions in NotifCollection when initializing 

When systemUI is in the process of connecting to system server to
receive notifications, there's a short window in which we might receive
events out of order. These events will violate our otherwise strict
requirements. To solve this, we really need a more robust way to connect
to system server, but in the meantime, we create a "forgiveness window"
that lasts five seconds. During this time, things that would normally
crash the system only log an error.

Bug: 151317347
Test: atest, manual
Change-Id: I9aad4ec94826189efc13433b4cd38f443113fd7e
parent b20914d9

packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java

+8 −0
Original line number Diff line number Diff line
@@ -104,6 +104,9 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
                    listener.onNotificationPosted(sbn, completeMap);

                }

            }

            for (NotificationHandler listener : mNotificationHandlers) {

                listener.onNotificationsInitialized();

            }

        });

        onSilentStatusBarIconsVisibilityChanged(

                mNotificationManager.shouldHideSilentStatusBarIcons());
@@ -224,5 +227,10 @@ public class NotificationListener extends NotificationListenerWithPlugins { 
        void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap);

        void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap, int reason);

        void onNotificationRankingUpdate(RankingMap rankingMap);



        /**

         * Called after the listener has connected to NoMan and posted any current notifications.

         */

        void onNotificationsInitialized();

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java

+4 −0
Original line number Diff line number Diff line
@@ -385,6 +385,10 @@ public class NotificationEntryManager implements 
        public void onNotificationRankingUpdate(RankingMap rankingMap) {

            updateNotificationRanking(rankingMap);

        }



        @Override

        public void onNotificationsInitialized() {

        }

    };



    /**

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java

+41 −6
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ import android.annotation.Nullable; 
import android.annotation.UserIdInt;

import android.app.Notification;

import android.os.RemoteException;

import android.os.SystemClock;

import android.os.UserHandle;

import android.service.notification.NotificationListenerService;

import android.service.notification.NotificationListenerService.Ranking;
@@ -82,6 +81,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No 
import com.android.systemui.statusbar.notification.collection.notifcollection.RankingAppliedEvent;

import com.android.systemui.statusbar.notification.collection.notifcollection.RankingUpdatedEvent;

import com.android.systemui.util.Assert;

import com.android.systemui.util.time.SystemClock;



import java.io.FileDescriptor;

import java.io.PrintWriter;
@@ -95,6 +95,7 @@ import java.util.List; 
import java.util.Map;

import java.util.Objects;

import java.util.Queue;

import java.util.concurrent.TimeUnit;



import javax.inject.Inject;

import javax.inject.Singleton;
@@ -125,6 +126,7 @@ import javax.inject.Singleton; 
@Singleton

public class NotifCollection implements Dumpable {

    private final IStatusBarService mStatusBarService;

    private final SystemClock mClock;

    private final FeatureFlags mFeatureFlags;

    private final NotifCollectionLogger mLogger;

    private final LogBufferEulogizer mEulogizer;
@@ -142,20 +144,24 @@ public class NotifCollection implements Dumpable { 


    private boolean mAttached = false;

    private boolean mAmDispatchingToOtherCode;

    private long mInitializedTimestamp = 0;



    @Inject

    public NotifCollection(

            IStatusBarService statusBarService,

            DumpManager dumpManager,

            SystemClock clock,

            FeatureFlags featureFlags,

            NotifCollectionLogger logger,

            LogBufferEulogizer logBufferEulogizer) {

            LogBufferEulogizer logBufferEulogizer,

            DumpManager dumpManager) {

        Assert.isMainThread();

        mStatusBarService = statusBarService;

        mClock = clock;

        mFeatureFlags = featureFlags;

        mLogger = logger;

        mEulogizer = logBufferEulogizer;



        dumpManager.registerDumpable(TAG, this);

        mFeatureFlags = featureFlags;

    }



    /** Initializes the NotifCollection and registers it to receive notification events. */
@@ -376,9 +382,10 @@ public class NotifCollection implements Dumpable { 


        final NotificationEntry entry = mNotificationSet.get(sbn.getKey());

        if (entry == null) {

            throw mEulogizer.record(

            crashIfNotInitializing(

                    new IllegalStateException("No notification to remove with key "

                            + sbn.getKey()));

            return;

        }



        entry.mCancellationReason = reason;
@@ -394,6 +401,10 @@ public class NotifCollection implements Dumpable { 
        dispatchEventsAndRebuildList();

    }



    private void onNotificationsInitialized() {

        mInitializedTimestamp = mClock.uptimeMillis();

    }



    private void postNotification(

            StatusBarNotification sbn,

            Ranking ranking) {
@@ -401,7 +412,7 @@ public class NotifCollection implements Dumpable { 


        if (entry == null) {

            // A new notification!

            entry = new NotificationEntry(sbn, ranking, SystemClock.uptimeMillis());

            entry = new NotificationEntry(sbn, ranking, mClock.uptimeMillis());

            mEventQueue.add(new InitEntryEvent(entry));

            mEventQueue.add(new BindEntryEvent(entry, sbn));

            mNotificationSet.put(sbn.getKey(), entry);
@@ -628,6 +639,23 @@ public class NotifCollection implements Dumpable { 
        }

    }



    // While the NotificationListener is connecting to NotificationManager, there is a short period

    // during which it's possible for us to receive events about notifications we don't yet know

    // about (or that otherwise don't make sense). Until that race condition is fixed, we create a

    // "forgiveness window" of five seconds during which we won't crash if we receive nonsensical

    // messages from system server.

    private void crashIfNotInitializing(RuntimeException exception) {

        final boolean isRecentlyInitialized = mInitializedTimestamp == 0

                || mClock.uptimeMillis() - mInitializedTimestamp

                        < INITIALIZATION_FORGIVENESS_WINDOW;



        if (isRecentlyInitialized) {

            mLogger.logIgnoredError(exception.getMessage());

        } else {

            throw mEulogizer.record(exception);

        }

    }



    private static Ranking requireRanking(RankingMap rankingMap, String key) {

        // TODO: Modify RankingMap so that we don't have to make a copy here

        Ranking ranking = new Ranking();
@@ -742,6 +770,11 @@ public class NotifCollection implements Dumpable { 
        public void onNotificationRankingUpdate(RankingMap rankingMap) {

            NotifCollection.this.onNotificationRankingUpdate(rankingMap);

        }



        @Override

        public void onNotificationsInitialized() {

            NotifCollection.this.onNotificationsInitialized();

        }

    };



    private static final String TAG = "NotifCollection";
@@ -773,4 +806,6 @@ public class NotifCollection implements Dumpable { 


    static final int REASON_NOT_CANCELED = -1;

    public static final int REASON_UNKNOWN = 0;



    private static final long INITIALIZATION_FORGIVENESS_WINDOW = TimeUnit.SECONDS.toMillis(5);

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coalescer/GroupCoalescer.java

+5 −0
Original line number Diff line number Diff line
@@ -153,6 +153,11 @@ public class GroupCoalescer implements Dumpable { 
            applyRanking(rankingMap);

            mHandler.onNotificationRankingUpdate(rankingMap);

        }



        @Override

        public void onNotificationsInitialized() {

            mHandler.onNotificationsInitialized();

        }

    };



    private void maybeEmitBatch(StatusBarNotification sbn) {

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt

+9 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.os.RemoteException 
import android.service.notification.NotificationListenerService.RankingMap

import com.android.systemui.log.LogBuffer

import com.android.systemui.log.LogLevel.DEBUG

import com.android.systemui.log.LogLevel.ERROR

import com.android.systemui.log.LogLevel.INFO

import com.android.systemui.log.LogLevel.WARNING

import com.android.systemui.log.LogLevel.WTF
@@ -167,6 +168,14 @@ class NotifCollectionLogger @Inject constructor( 
            "LIFETIME EXTENSION ENDED for $str1 by '$str2'; $int1 remaining extensions"

        })

    }



    fun logIgnoredError(message: String?) {

        buffer.log(TAG, ERROR, {

            str1 = message

        }, {

            "ERROR suppressed due to initialization forgiveness: $str1"

        })

    }

}



private const val TAG = "NotifCollection"