Add inflation error manager
Move hasInflationError into a state manager class and put relevant
error logging and filtering logic as listeners on the state.
Test: manual
Test: atest SystemUITests
Change-Id: I669eabbb06248c0866aba6323342eb2c4fd73db5
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
index 3fa1954..b5c81b2 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
@@ -151,10 +151,6 @@
.append(" ");
}
- if (notifEntry.hasInflationError()) {
- rksb.append("(!)hasInflationError ");
- }
-
if (notifEntry.getDismissState() != NOT_DISMISSED) {
rksb.append("dismissState=")
.append(notifEntry.getDismissState())
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
index 9272e51b..83f56cc 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
@@ -18,9 +18,7 @@
import static android.service.notification.NotificationStats.DISMISS_SENTIMENT_NEUTRAL;
-import android.os.RemoteException;
import android.service.notification.NotificationStats;
-import android.service.notification.StatusBarNotification;
import com.android.internal.statusbar.IStatusBarService;
import com.android.internal.statusbar.NotificationVisibility;
@@ -29,6 +27,7 @@
import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl;
import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
import com.android.systemui.statusbar.notification.logging.NotificationLogger;
+import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
import com.android.systemui.statusbar.notification.row.NotificationContentInflater;
import javax.inject.Inject;
@@ -44,6 +43,7 @@
private final IStatusBarService mStatusBarService;
private final NotifCollection mNotifCollection;
+ private final NotifInflationErrorManager mNotifErrorManager;
private NotificationRowBinderImpl mNotificationRowBinder;
private InflationCallback mExternalInflationCallback;
@@ -51,9 +51,11 @@
@Inject
public NotifInflaterImpl(
IStatusBarService statusBarService,
- NotifCollection notifCollection) {
+ NotifCollection notifCollection,
+ NotifInflationErrorManager errorManager) {
mStatusBarService = statusBarService;
mNotifCollection = notifCollection;
+ mNotifErrorManager = errorManager;
}
/**
@@ -81,7 +83,6 @@
@Override
public void inflateViews(NotificationEntry entry) {
try {
- entry.setHasInflationError(false);
requireBinder().inflateViews(entry, getDismissCallback(entry));
} catch (InflationException e) {
// logged in mInflationCallback.handleInflationException
@@ -131,25 +132,12 @@
public void handleInflationException(
NotificationEntry entry,
Exception e) {
- entry.setHasInflationError(true);
- try {
- final StatusBarNotification sbn = entry.getSbn();
- // report notification inflation errors back up
- // to notification delegates
- mStatusBarService.onNotificationError(
- sbn.getPackageName(),
- sbn.getTag(),
- sbn.getId(),
- sbn.getUid(),
- sbn.getInitialPid(),
- e.getMessage(),
- sbn.getUserId());
- } catch (RemoteException ex) {
- }
+ mNotifErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
+ mNotifErrorManager.clearInflationError(entry);
if (mExternalInflationCallback != null) {
mExternalInflationCallback.onInflationFinished(entry);
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
index 006d40d..f482d37 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
@@ -122,9 +122,6 @@
*/
@CancellationReason int mCancellationReason = REASON_NOT_CANCELED;
- /** @see #hasInflationError() */
- private boolean mHasInflationError;
-
/** @see #getDismissState() */
@NonNull private DismissState mDismissState = DismissState.NOT_DISMISSED;
@@ -274,23 +271,6 @@
*/
/**
- * Whether this notification had an error when attempting to inflate. This is only used in
- * the NewNotifPipeline
- */
- public boolean hasInflationError() {
- return mHasInflationError;
- }
-
- /**
- * Set whether the notification has an error while inflating.
- *
- * TODO: Move this into an inflation error manager class.
- */
- public void setHasInflationError(boolean hasError) {
- mHasInflationError = hasError;
- }
-
- /**
* Set if the user has dismissed this notif but we haven't yet heard back from system server to
* confirm the dismissal.
*/
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
index 1e5946a..1c8fdac 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
@@ -16,12 +16,18 @@
package com.android.systemui.statusbar.notification.collection.coordinator;
+import android.os.RemoteException;
+import android.service.notification.StatusBarNotification;
+
+import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
+import com.android.systemui.statusbar.notification.collection.ShadeListBuilder;
import com.android.systemui.statusbar.notification.collection.inflation.NotifInflater;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
+import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
import java.util.ArrayList;
import java.util.List;
@@ -34,7 +40,7 @@
* Aborts inflation when a notification is removed.
*
* If a notification is not done inflating, this coordinator will filter the notification out
- * from the NotifListBuilder.
+ * from the {@link ShadeListBuilder}.
*/
@Singleton
public class PreparationCoordinator implements Coordinator {
@@ -42,15 +48,22 @@
private final PreparationCoordinatorLogger mLogger;
private final NotifInflater mNotifInflater;
+ private final NotifInflationErrorManager mNotifErrorManager;
private final List<NotificationEntry> mPendingNotifications = new ArrayList<>();
+ private final IStatusBarService mStatusBarService;
@Inject
public PreparationCoordinator(
PreparationCoordinatorLogger logger,
- NotifInflaterImpl notifInflater) {
+ NotifInflaterImpl notifInflater,
+ NotifInflationErrorManager errorManager,
+ IStatusBarService service) {
mLogger = logger;
mNotifInflater = notifInflater;
mNotifInflater.setInflationCallback(mInflationCallback);
+ mNotifErrorManager = errorManager;
+ mNotifErrorManager.addInflationErrorListener(mInflationErrorListener);
+ mStatusBarService = service;
}
@Override
@@ -84,8 +97,7 @@
*/
@Override
public boolean shouldFilterOut(NotificationEntry entry, long now) {
- if (entry.hasInflationError()) {
- mPendingNotifications.remove(entry);
+ if (mNotifErrorManager.hasInflationError(entry)) {
return true;
}
return false;
@@ -112,6 +124,34 @@
}
};
+ private final NotifInflationErrorManager.NotifInflationErrorListener mInflationErrorListener =
+ new NotifInflationErrorManager.NotifInflationErrorListener() {
+ @Override
+ public void onNotifInflationError(NotificationEntry entry, Exception e) {
+ mPendingNotifications.remove(entry);
+ try {
+ final StatusBarNotification sbn = entry.getSbn();
+ // report notification inflation errors back up
+ // to notification delegates
+ mStatusBarService.onNotificationError(
+ sbn.getPackageName(),
+ sbn.getTag(),
+ sbn.getId(),
+ sbn.getUid(),
+ sbn.getInitialPid(),
+ e.getMessage(),
+ sbn.getUserId());
+ } catch (RemoteException ex) {
+ }
+ mNotifInflationErrorFilter.invalidateList();
+ }
+
+ @Override
+ public void onNotifInflationErrorCleared(NotificationEntry entry) {
+ mNotifInflationErrorFilter.invalidateList();
+ }
+ };
+
private void inflateEntry(NotificationEntry entry, String reason) {
abortInflation(entry, reason);
mPendingNotifications.add(entry);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifInflationErrorManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifInflationErrorManager.java
new file mode 100644
index 0000000..a3ca084
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifInflationErrorManager.java
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.notification.row;
+
+import androidx.annotation.NonNull;
+import androidx.collection.ArraySet;
+
+import com.android.systemui.statusbar.notification.collection.NotificationEntry;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+/**
+ * A manager handling the error state of a notification when it encounters an exception while
+ * inflating. We don't want to show these notifications to the user but may want to keep them
+ * around for logging purposes.
+ */
+@Singleton
+public class NotifInflationErrorManager {
+
+ Set<NotificationEntry> mErroredNotifs = new ArraySet<>();
+ List<NotifInflationErrorListener> mListeners = new ArrayList<>();
+
+ @Inject
+ public NotifInflationErrorManager() { }
+
+ /**
+ * Mark the notification as errored out due to encountering an exception while inflating.
+ *
+ * @param e the exception encountered while inflating
+ */
+ public void setInflationError(NotificationEntry entry, Exception e) {
+ mErroredNotifs.add(entry);
+ for (int i = 0; i < mListeners.size(); i++) {
+ mListeners.get(i).onNotifInflationError(entry, e);
+ }
+ }
+
+ /**
+ * Notification inflated successfully and is no longer errored out.
+ */
+ public void clearInflationError(NotificationEntry entry) {
+ if (mErroredNotifs.contains(entry)) {
+ mErroredNotifs.remove(entry);
+ for (int i = 0; i < mListeners.size(); i++) {
+ mListeners.get(i).onNotifInflationErrorCleared(entry);
+ }
+ }
+ }
+
+ /**
+ * Whether or not the notification encountered an exception while inflating.
+ */
+ public boolean hasInflationError(@NonNull NotificationEntry entry) {
+ return mErroredNotifs.contains(entry);
+ }
+
+ /**
+ * Add listener for changes in inflation error state.
+ */
+ public void addInflationErrorListener(NotifInflationErrorListener listener) {
+ mListeners.add(listener);
+ }
+
+ /**
+ * Listener for changes in notification inflation error state.
+ */
+ public interface NotifInflationErrorListener {
+
+ /**
+ * Called when notification encounters an inflation exception.
+ *
+ * @param e the exception encountered while inflating
+ */
+ void onNotifInflationError(NotificationEntry entry, Exception e);
+
+ /**
+ * Called when notification inflation error is cleared.
+ */
+ default void onNotifInflationErrorCleared(NotificationEntry entry) {}
+ }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindStage.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindStage.java
index f124179..f783245 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindStage.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindStage.java
@@ -18,12 +18,8 @@
import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_ALL;
-import android.os.RemoteException;
-import android.service.notification.StatusBarNotification;
-
import androidx.annotation.NonNull;
-import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.BindParams;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationCallback;
@@ -41,14 +37,14 @@
@Singleton
public class RowContentBindStage extends BindStage<RowContentBindParams> {
private final NotificationRowContentBinder mBinder;
- private final IStatusBarService mStatusBarService;
+ private final NotifInflationErrorManager mNotifInflationErrorManager;
@Inject
RowContentBindStage(
NotificationRowContentBinder binder,
- IStatusBarService statusBarService) {
+ NotifInflationErrorManager errorManager) {
mBinder = binder;
- mStatusBarService = statusBarService;
+ mNotifInflationErrorManager = errorManager;
}
@Override
@@ -78,24 +74,12 @@
InflationCallback inflationCallback = new InflationCallback() {
@Override
public void handleInflationException(NotificationEntry entry, Exception e) {
- entry.setHasInflationError(true);
- try {
- final StatusBarNotification sbn = entry.getSbn();
- mStatusBarService.onNotificationError(
- sbn.getPackageName(),
- sbn.getTag(),
- sbn.getId(),
- sbn.getUid(),
- sbn.getInitialPid(),
- e.getMessage(),
- sbn.getUserId());
- } catch (RemoteException ex) {
- }
+ mNotifInflationErrorManager.setInflationError(entry, e);
}
@Override
public void onAsyncInflationFinished(NotificationEntry entry) {
- entry.setHasInflationError(false);
+ mNotifInflationErrorManager.clearInflationError(entry);
getStageParams(entry).clearDirtyContentViews();
callback.onStageFinished(entry);
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java
new file mode 100644
index 0000000..61a4fbe
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorTest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.notification.collection.coordinator;
+
+import static junit.framework.Assert.assertTrue;
+
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import android.os.RemoteException;
+import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
+
+import androidx.test.filters.SmallTest;
+
+import com.android.internal.statusbar.IStatusBarService;
+import com.android.systemui.SysuiTestCase;
+import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl;
+import com.android.systemui.statusbar.notification.collection.NotifPipeline;
+import com.android.systemui.statusbar.notification.collection.NotificationEntry;
+import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder;
+import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
+import com.android.systemui.statusbar.notification.row.NotifInflationErrorManager;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.util.List;
+
+@SmallTest
+@RunWith(AndroidTestingRunner.class)
+@TestableLooper.RunWithLooper
+public class PreparationCoordinatorTest extends SysuiTestCase {
+ private static final String TEST_MESSAGE = "TEST_MESSAGE";
+
+ private PreparationCoordinator mCoordinator;
+ private NotifFilter mInflationErrorFilter;
+ private NotifInflationErrorManager mErrorManager;
+ private NotificationEntry mEntry;
+ private Exception mInflationError;
+
+ @Mock
+ private NotifPipeline mNotifPipeline;
+ @Mock private IStatusBarService mService;
+
+ @Before
+ public void setUp() {
+ MockitoAnnotations.initMocks(this);
+
+ mEntry = new NotificationEntryBuilder().build();
+ mInflationError = new Exception(TEST_MESSAGE);
+ mErrorManager = new NotifInflationErrorManager();
+
+ mCoordinator = new PreparationCoordinator(
+ mock(PreparationCoordinatorLogger.class),
+ mock(NotifInflaterImpl.class),
+ mErrorManager,
+ mService);
+
+ ArgumentCaptor<NotifFilter> filterCaptor = ArgumentCaptor.forClass(NotifFilter.class);
+ mCoordinator.attach(mNotifPipeline);
+ verify(mNotifPipeline, times(2)).addPreRenderFilter(filterCaptor.capture());
+ List<NotifFilter> filters = filterCaptor.getAllValues();
+ mInflationErrorFilter = filters.get(0);
+ }
+
+ @Test
+ public void testErrorLogsToService() throws RemoteException {
+ // WHEN an entry has an inflation error.
+ mErrorManager.setInflationError(mEntry, mInflationError);
+
+ // THEN we log to status bar service.
+ verify(mService).onNotificationError(
+ eq(mEntry.getSbn().getPackageName()),
+ eq(mEntry.getSbn().getTag()),
+ eq(mEntry.getSbn().getId()),
+ eq(mEntry.getSbn().getUid()),
+ eq(mEntry.getSbn().getInitialPid()),
+ eq(mInflationError.getMessage()),
+ eq(mEntry.getSbn().getUserId()));
+ }
+
+ @Test
+ public void testFiltersOutErroredNotifications() {
+ // WHEN an entry has an inflation error.
+ mErrorManager.setInflationError(mEntry, mInflationError);
+
+ // THEN we filter it from the notification list.
+ assertTrue(mInflationErrorFilter.shouldFilterOut(mEntry, 0));
+ }
+}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java
index 35b5508..2319758 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java
@@ -41,7 +41,6 @@
import android.view.LayoutInflater;
import android.widget.RemoteViews;
-import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.TestableDependency;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.bubbles.BubblesTestActivity;
@@ -112,7 +111,7 @@
mock(NotifRemoteViewCache.class),
mock(NotificationRemoteInputManager.class));
contentBinder.setInflateSynchronously(true);
- mBindStage = new RowContentBindStage(contentBinder, mock(IStatusBarService.class));
+ mBindStage = new RowContentBindStage(contentBinder, mock(NotifInflationErrorManager.class));
NotificationEntryManager entryManager = mock(NotificationEntryManager.class);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
index 775f722..d9fe655 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
@@ -34,7 +34,6 @@
import androidx.test.filters.SmallTest;
-import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.BindParams;
@@ -62,7 +61,7 @@
MockitoAnnotations.initMocks(this);
mRowContentBindStage = new RowContentBindStage(mBinder,
- mock(IStatusBarService.class));
+ mock(NotifInflationErrorManager.class));
mRowContentBindStage.createStageParams(mEntry);
}
