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 2f4b2e535c610..ebecf181b8080 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,31 +16,39 @@ package com.android.systemui.statusbar.notification.collection.coordinator; +import android.annotation.IntDef; import android.os.RemoteException; import android.service.notification.StatusBarNotification; +import android.util.ArrayMap; import com.android.internal.statusbar.IStatusBarService; +import com.android.systemui.statusbar.notification.collection.GroupEntry; +import com.android.systemui.statusbar.notification.collection.ListEntry; 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.OnBeforeFinalizeFilterListener; 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.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.List; +import java.util.Map; +import java.util.Objects; import javax.inject.Inject; import javax.inject.Singleton; /** - * Kicks off notification inflation and view rebinding when a notification is added or updated. + * Kicks off core notification inflation and view rebinding when a notification is added or updated. * Aborts inflation when a notification is removed. * - * If a notification is not done inflating, this coordinator will filter the notification out - * from the {@link ShadeListBuilder}. + * If a notification was uninflated, this coordinator will filter the notification out from the + * {@link ShadeListBuilder} until it is inflated. */ @Singleton public class PreparationCoordinator implements Coordinator { @@ -49,7 +57,7 @@ public class PreparationCoordinator implements Coordinator { private final PreparationCoordinatorLogger mLogger; private final NotifInflater mNotifInflater; private final NotifInflationErrorManager mNotifErrorManager; - private final List mPendingNotifications = new ArrayList<>(); + private final Map mInflationStates = new ArrayMap<>(); private final IStatusBarService mStatusBarService; @Inject @@ -69,27 +77,44 @@ public class PreparationCoordinator implements Coordinator { @Override public void attach(NotifPipeline pipeline) { pipeline.addCollectionListener(mNotifCollectionListener); + // Inflate after grouping/sorting since that affects what views to inflate. + pipeline.addOnBeforeFinalizeFilterListener(mOnBeforeFinalizeFilterListener); pipeline.addFinalizeFilter(mNotifInflationErrorFilter); pipeline.addFinalizeFilter(mNotifInflatingFilter); } private final NotifCollectionListener mNotifCollectionListener = new NotifCollectionListener() { + @Override - public void onEntryAdded(NotificationEntry entry) { - inflateEntry(entry, "entryAdded"); + public void onEntryInit(NotificationEntry entry) { + mInflationStates.put(entry, STATE_UNINFLATED); } @Override public void onEntryUpdated(NotificationEntry entry) { - rebind(entry, "entryUpdated"); + @InflationState int state = getInflationState(entry); + if (state == STATE_INFLATED) { + mInflationStates.put(entry, STATE_INFLATED_INVALID); + } else if (state == STATE_ERROR) { + // Updated so maybe it won't error out now. + mInflationStates.put(entry, STATE_UNINFLATED); + } } @Override public void onEntryRemoved(NotificationEntry entry, int reason) { abortInflation(entry, "entryRemoved reason=" + reason); } + + @Override + public void onEntryCleanUp(NotificationEntry entry) { + mInflationStates.remove(entry); + } }; + private final OnBeforeFinalizeFilterListener mOnBeforeFinalizeFilterListener = + entries -> inflateAllRequiredViews(entries); + private final NotifFilter mNotifInflationErrorFilter = new NotifFilter( TAG + "InflationError") { /** @@ -97,10 +122,7 @@ public class PreparationCoordinator implements Coordinator { */ @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { - if (mNotifErrorManager.hasInflationError(entry)) { - return true; - } - return false; + return getInflationState(entry) == STATE_ERROR; } }; @@ -110,7 +132,8 @@ public class PreparationCoordinator implements Coordinator { */ @Override public boolean shouldFilterOut(NotificationEntry entry, long now) { - return mPendingNotifications.contains(entry); + @InflationState int state = getInflationState(entry); + return (state != STATE_INFLATED) && (state != STATE_INFLATED_INVALID); } }; @@ -119,7 +142,7 @@ public class PreparationCoordinator implements Coordinator { @Override public void onInflationFinished(NotificationEntry entry) { mLogger.logNotifInflated(entry.getKey()); - mPendingNotifications.remove(entry); + mInflationStates.put(entry, STATE_INFLATED); mNotifInflatingFilter.invalidateList(); } }; @@ -128,7 +151,7 @@ public class PreparationCoordinator implements Coordinator { new NotifInflationErrorManager.NotifInflationErrorListener() { @Override public void onNotifInflationError(NotificationEntry entry, Exception e) { - mPendingNotifications.remove(entry); + mInflationStates.put(entry, STATE_ERROR); try { final StatusBarNotification sbn = entry.getSbn(); // report notification inflation errors back up @@ -152,9 +175,41 @@ public class PreparationCoordinator implements Coordinator { } }; + private void inflateAllRequiredViews(List entries) { + for (int i = 0, size = entries.size(); i < size; i++) { + ListEntry entry = entries.get(i); + if (entry instanceof GroupEntry) { + GroupEntry groupEntry = (GroupEntry) entry; + inflateNotifRequiredViews(groupEntry.getSummary()); + List children = groupEntry.getChildren(); + for (int j = 0, groupSize = children.size(); j < groupSize; j++) { + inflateNotifRequiredViews(children.get(j)); + } + } else { + NotificationEntry notifEntry = (NotificationEntry) entry; + inflateNotifRequiredViews(notifEntry); + } + } + } + + private void inflateNotifRequiredViews(NotificationEntry entry) { + @InflationState int state = mInflationStates.get(entry); + switch (state) { + case STATE_UNINFLATED: + inflateEntry(entry, "entryAdded"); + break; + case STATE_INFLATED_INVALID: + rebind(entry, "entryUpdated"); + break; + case STATE_INFLATED: + case STATE_ERROR: + default: + // Nothing to do. + } + } + private void inflateEntry(NotificationEntry entry, String reason) { abortInflation(entry, reason); - mPendingNotifications.add(entry); mNotifInflater.inflateViews(entry); } @@ -165,6 +220,32 @@ public class PreparationCoordinator implements Coordinator { private void abortInflation(NotificationEntry entry, String reason) { mLogger.logInflationAborted(entry.getKey(), reason); entry.abortTask(); - mPendingNotifications.remove(entry); } + + private @InflationState int getInflationState(NotificationEntry entry) { + Integer stateObj = mInflationStates.get(entry); + Objects.requireNonNull(stateObj, + "Asking state of a notification preparation coordinator doesn't know about"); + return stateObj; + } + + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = {"STATE_"}, + value = {STATE_UNINFLATED, STATE_INFLATED_INVALID, STATE_INFLATED, STATE_ERROR}) + @interface InflationState {} + + /** The notification has never been inflated before. */ + private static final int STATE_UNINFLATED = 0; + + /** The notification is inflated. */ + private static final int STATE_INFLATED = 1; + + /** + * The notification is inflated, but its content may be out-of-date since the notification has + * been updated. + */ + private static final int STATE_INFLATED_INVALID = 2; + + /** The notification errored out while inflating */ + private static final int STATE_ERROR = -1; } 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 index 4667e910e85a8..792b4d5c707cc 100644 --- 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 @@ -16,8 +16,8 @@ package com.android.systemui.statusbar.notification.collection.coordinator; -import static junit.framework.Assert.assertTrue; - +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -35,13 +35,16 @@ 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.OnBeforeFinalizeFilterListener; 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 org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -54,14 +57,22 @@ public class PreparationCoordinatorTest extends SysuiTestCase { private static final String TEST_MESSAGE = "TEST_MESSAGE"; private PreparationCoordinator mCoordinator; + private NotifCollectionListener mCollectionListener; + private OnBeforeFinalizeFilterListener mBeforeFilterListener; + private NotifFilter mUninflatedFilter; private NotifFilter mInflationErrorFilter; + private NotifInflaterImpl.InflationCallback mCallback; private NotifInflationErrorManager mErrorManager; private NotificationEntry mEntry; private Exception mInflationError; - @Mock - private NotifPipeline mNotifPipeline; + @Captor private ArgumentCaptor mCollectionListenerCaptor; + @Captor private ArgumentCaptor mBeforeFilterListenerCaptor; + @Captor private ArgumentCaptor mCallbackCaptor; + + @Mock private NotifPipeline mNotifPipeline; @Mock private IStatusBarService mService; + @Mock private NotifInflaterImpl mNotifInflater; @Before public void setUp() { @@ -73,7 +84,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase { mCoordinator = new PreparationCoordinator( mock(PreparationCoordinatorLogger.class), - mock(NotifInflaterImpl.class), + mNotifInflater, mErrorManager, mService); @@ -82,6 +93,19 @@ public class PreparationCoordinatorTest extends SysuiTestCase { verify(mNotifPipeline, times(2)).addFinalizeFilter(filterCaptor.capture()); List filters = filterCaptor.getAllValues(); mInflationErrorFilter = filters.get(0); + mUninflatedFilter = filters.get(1); + + verify(mNotifPipeline).addCollectionListener(mCollectionListenerCaptor.capture()); + mCollectionListener = mCollectionListenerCaptor.getValue(); + + verify(mNotifPipeline).addOnBeforeFinalizeFilterListener( + mBeforeFilterListenerCaptor.capture()); + mBeforeFilterListener = mBeforeFilterListenerCaptor.getValue(); + + verify(mNotifInflater).setInflationCallback(mCallbackCaptor.capture()); + mCallback = mCallbackCaptor.getValue(); + + mCollectionListener.onEntryInit(mEntry); } @Test @@ -108,4 +132,42 @@ public class PreparationCoordinatorTest extends SysuiTestCase { // THEN we filter it from the notification list. assertTrue(mInflationErrorFilter.shouldFilterOut(mEntry, 0)); } + + @Test + public void testInflatesNewNotification() { + // WHEN there is a new notification + mCollectionListener.onEntryAdded(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we inflate it + verify(mNotifInflater).inflateViews(mEntry); + + // THEN we filter it out until it's done inflating. + assertTrue(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testRebindsInflatedNotificationsOnUpdate() { + // GIVEN an inflated notification + mCallback.onInflationFinished(mEntry); + + // WHEN notification is updated + mCollectionListener.onEntryUpdated(mEntry); + mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); + + // THEN we rebind it + verify(mNotifInflater).rebindViews(mEntry); + + // THEN we do not filter it because it's not the first inflation. + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } + + @Test + public void testDoesntFilterInflatedNotifs() { + // WHEN a notification is inflated + mCallback.onInflationFinished(mEntry); + + // THEN it isn't filtered from shade list + assertFalse(mUninflatedFilter.shouldFilterOut(mEntry, 0)); + } }