Move preparation coordinator to after grouping

Move preparation coordinator to after grouping since some inflation
decisions (e.g. whether we inflate at all) will be dependent on the
grouping.

Bug: 145748993
Test: atest InflationCoordinatorTest
Change-Id: I1169beb3cf136674b89fd713ff40e9ed3b217f75
This commit is contained in:
Kevin Han
2020-02-19 10:39:04 -08:00
parent 2265301d53
commit 4466434976
2 changed files with 165 additions and 22 deletions

View File

@@ -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<NotificationEntry> mPendingNotifications = new ArrayList<>();
private final Map<NotificationEntry, Integer> 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<ListEntry> 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<NotificationEntry> 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;
}

View File

@@ -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<NotifCollectionListener> mCollectionListenerCaptor;
@Captor private ArgumentCaptor<OnBeforeFinalizeFilterListener> mBeforeFilterListenerCaptor;
@Captor private ArgumentCaptor<NotifInflaterImpl.InflationCallback> 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<NotifFilter> 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));
}
}