Merge "Don't throw exceptions in NotifCollection when initializing" into rvc-dev

This commit is contained in:
TreeHugger Robot
2020-06-12 16:04:58 +00:00
committed by Android (Google) Code Review
6 changed files with 110 additions and 8 deletions

View File

@@ -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();
}
}

View File

@@ -385,6 +385,10 @@ public class NotificationEntryManager implements
public void onNotificationRankingUpdate(RankingMap rankingMap) {
updateNotificationRanking(rankingMap);
}
@Override
public void onNotificationsInitialized() {
}
};
/**

View File

@@ -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);
}

View File

@@ -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) {

View File

@@ -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"

View File

@@ -80,6 +80,7 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionLogger;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifDismissInterceptor;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
import com.android.systemui.util.time.FakeSystemClock;
import org.junit.Before;
import org.junit.Test;
@@ -131,6 +132,7 @@ public class NotifCollectionTest extends SysuiTestCase {
private InOrder mListenerInOrder;
private NoManSimulator mNoMan;
private FakeSystemClock mClock = new FakeSystemClock();
@Before
public void setUp() {
@@ -146,10 +148,11 @@ public class NotifCollectionTest extends SysuiTestCase {
mCollection = new NotifCollection(
mStatusBarService,
mock(DumpManager.class),
mClock,
mFeatureFlags,
mLogger,
mEulogizer);
mEulogizer,
mock(DumpManager.class));
mCollection.attach(mGroupCoalescer);
mCollection.addCollectionListener(mCollectionListener);
mCollection.setBuildListener(mBuildListener);
@@ -161,6 +164,8 @@ public class NotifCollectionTest extends SysuiTestCase {
mNoMan = new NoManSimulator();
mNoMan.addListener(mNotifHandler);
mNotifHandler.onNotificationsInitialized();
}
@Test
@@ -1268,6 +1273,42 @@ public class NotifCollectionTest extends SysuiTestCase {
verify(mInterceptor3, never()).shouldInterceptDismissal(clearable);
}
@Test(expected = IllegalStateException.class)
public void testClearNotificationThrowsIfMissing() {
// GIVEN that enough time has passed that we're beyond the forgiveness window
mClock.advanceTime(5001);
// WHEN we get a remove event for a notification we don't know about
final NotificationEntry container = new NotificationEntryBuilder()
.setPkg(TEST_PACKAGE)
.setId(47)
.build();
mNotifHandler.onNotificationRemoved(
container.getSbn(),
new RankingMap(new Ranking[]{ container.getRanking() }));
// THEN an exception is thrown
}
@Test
public void testClearNotificationDoesntThrowIfInForgivenessWindow() {
// GIVEN that some time has passed but we're still within the initialization forgiveness
// window
mClock.advanceTime(4999);
// WHEN we get a remove event for a notification we don't know about
final NotificationEntry container = new NotificationEntryBuilder()
.setPkg(TEST_PACKAGE)
.setId(47)
.build();
mNotifHandler.onNotificationRemoved(
container.getSbn(),
new RankingMap(new Ranking[]{ container.getRanking() }));
// THEN no exception is thrown, but no event is fired
verify(mCollectionListener, never()).onEntryRemoved(any(NotificationEntry.class), anyInt());
}
private static NotificationEntryBuilder buildNotif(String pkg, int id, String tag) {
return new NotificationEntryBuilder()
.setPkg(pkg)