Merge "Don't throw exceptions in NotifCollection when initializing" into rvc-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
8cf7a51d3d
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -385,6 +385,10 @@ public class NotificationEntryManager implements
|
||||
public void onNotificationRankingUpdate(RankingMap rankingMap) {
|
||||
updateNotificationRanking(rankingMap);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onNotificationsInitialized() {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user