Merge "New Pipeline: begrudgingly tolerate a few reentrant runs" into tm-qpr-dev
This commit is contained in:
TreeHugger Robot
committed by
Android (Google) Code Review
Android (Google) Code Review
commit
5bb3f6f78c
@@ -38,6 +38,7 @@ import android.os.Trace;
|
||||
import android.service.notification.StatusBarNotification;
|
||||
import android.util.ArrayMap;
|
||||
import android.util.ArraySet;
|
||||
import android.util.Log;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
import androidx.annotation.VisibleForTesting;
|
||||
@@ -126,6 +127,9 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
|
||||
private List<ListEntry> mReadOnlyNewNotifList = Collections.unmodifiableList(mNewNotifList);
|
||||
private final NotifPipelineChoreographer mChoreographer;
|
||||
|
||||
private int mConsecutiveReentrantRebuilds = 0;
|
||||
@VisibleForTesting public static final int MAX_CONSECUTIVE_REENTRANT_REBUILDS = 3;
|
||||
|
||||
@Inject
|
||||
public ShadeListBuilder(
|
||||
DumpManager dumpManager,
|
||||
@@ -310,7 +314,7 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
|
||||
|
||||
mLogger.logOnBuildList(reason);
|
||||
mAllEntries = entries;
|
||||
mChoreographer.schedule();
|
||||
scheduleRebuild(/* reentrant = */ false);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1332,11 +1336,64 @@ public class ShadeListBuilder implements Dumpable, PipelineDumpable {
|
||||
throw new RuntimeException("Missing default sectioner!");
|
||||
}
|
||||
|
||||
private void rebuildListIfBefore(@PipelineState.StateName int state) {
|
||||
mPipelineState.requireIsBefore(state);
|
||||
if (mPipelineState.is(STATE_IDLE)) {
|
||||
mChoreographer.schedule();
|
||||
private void rebuildListIfBefore(@PipelineState.StateName int rebuildState) {
|
||||
final @PipelineState.StateName int currentState = mPipelineState.getState();
|
||||
|
||||
// If the pipeline is idle, requesting an invalidation is always okay, and starts a new run.
|
||||
if (currentState == STATE_IDLE) {
|
||||
scheduleRebuild(/* reentrant = */ false, rebuildState);
|
||||
return;
|
||||
}
|
||||
|
||||
// If the pipeline is running, it is okay to request an invalidation of a *later* stage.
|
||||
// Since the current pipeline run hasn't run it yet, no new pipeline run is needed.
|
||||
if (rebuildState > currentState) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the pipeline is running, it is bad to request an invalidation of *earlier* stages or
|
||||
// the *current* stage; this will run the pipeline more often than needed, and may even
|
||||
// cause an infinite loop of pipeline runs.
|
||||
//
|
||||
// Unfortunately, there are some unfixed bugs that cause reentrant pipeline runs, so we keep
|
||||
// a counter and allow a few reentrant runs in a row between any two non-reentrant runs.
|
||||
//
|
||||
// It is technically possible for a *pair* of invalidations, one reentrant and one not, to
|
||||
// trigger *each other*, alternating responsibility for pipeline runs in an infinite loop
|
||||
// but constantly resetting the reentrant run counter. Hopefully that doesn't happen.
|
||||
scheduleRebuild(/* reentrant = */ true, rebuildState);
|
||||
}
|
||||
|
||||
private void scheduleRebuild(boolean reentrant) {
|
||||
scheduleRebuild(reentrant, STATE_IDLE);
|
||||
}
|
||||
|
||||
private void scheduleRebuild(boolean reentrant, @PipelineState.StateName int rebuildState) {
|
||||
if (!reentrant) {
|
||||
mConsecutiveReentrantRebuilds = 0;
|
||||
mChoreographer.schedule();
|
||||
return;
|
||||
}
|
||||
|
||||
final @PipelineState.StateName int currentState = mPipelineState.getState();
|
||||
|
||||
final String rebuildStateName = PipelineState.getStateName(rebuildState);
|
||||
final String currentStateName = PipelineState.getStateName(currentState);
|
||||
final IllegalStateException exception = new IllegalStateException(
|
||||
"Reentrant notification pipeline rebuild of state " + rebuildStateName
|
||||
+ " while pipeline in state " + currentStateName + ".");
|
||||
|
||||
mConsecutiveReentrantRebuilds++;
|
||||
|
||||
if (mConsecutiveReentrantRebuilds > MAX_CONSECUTIVE_REENTRANT_REBUILDS) {
|
||||
Log.e(TAG, "Crashing after more than " + MAX_CONSECUTIVE_REENTRANT_REBUILDS
|
||||
+ " consecutive reentrant notification pipeline rebuilds.", exception);
|
||||
throw exception;
|
||||
}
|
||||
|
||||
Log.e(TAG, "Allowing " + mConsecutiveReentrantRebuilds
|
||||
+ " consecutive reentrant notification pipeline rebuild(s).", exception);
|
||||
mChoreographer.schedule();
|
||||
}
|
||||
|
||||
private static int countChildren(List<ListEntry> entries) {
|
||||
|
||||
@@ -68,6 +68,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga
|
||||
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter;
|
||||
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner;
|
||||
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifStabilityManager;
|
||||
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable;
|
||||
import com.android.systemui.statusbar.notification.collection.notifcollection.CollectionReadyForBuildListener;
|
||||
import com.android.systemui.util.time.FakeSystemClock;
|
||||
|
||||
@@ -1715,66 +1716,201 @@ public class ShadeListBuilderTest extends SysuiTestCase {
|
||||
assertEquals(GroupEntry.ROOT_ENTRY, group.getPreviousParent());
|
||||
}
|
||||
|
||||
@Test(expected = IllegalStateException.class)
|
||||
public void testOutOfOrderPreGroupFilterInvalidationThrows() {
|
||||
// GIVEN a PreGroupNotifFilter that gets invalidated during the grouping stage
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_5);
|
||||
OnBeforeTransformGroupsListener listener = (list) -> filter.invalidateList(null);
|
||||
static class CountingInvalidator {
|
||||
CountingInvalidator(Pluggable pluggableToInvalidate) {
|
||||
mPluggableToInvalidate = pluggableToInvalidate;
|
||||
mInvalidationCount = 0;
|
||||
}
|
||||
|
||||
public void setInvalidationCount(int invalidationCount) {
|
||||
mInvalidationCount = invalidationCount;
|
||||
}
|
||||
|
||||
public void maybeInvalidate() {
|
||||
if (mInvalidationCount > 0) {
|
||||
mPluggableToInvalidate.invalidateList("test invalidation");
|
||||
mInvalidationCount--;
|
||||
}
|
||||
}
|
||||
|
||||
private Pluggable mPluggableToInvalidate;
|
||||
private int mInvalidationCount;
|
||||
|
||||
private static final String TAG = "ShadeListBuilderTestCountingInvalidator";
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOutOfOrderPreGroupFilterInvalidationDoesNotThrowBeforeTooManyRuns() {
|
||||
// GIVEN a PreGroupNotifFilter that gets invalidated during the grouping stage,
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(filter);
|
||||
OnBeforeTransformGroupsListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addPreGroupFilter(filter);
|
||||
mListBuilder.addOnBeforeTransformGroupsListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the filter is invalidated
|
||||
addNotif(0, PACKAGE_1);
|
||||
// WHEN we try to run the pipeline and the filter is invalidated exactly
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception is thrown
|
||||
// THEN an exception is NOT thrown.
|
||||
}
|
||||
|
||||
@Test(expected = IllegalStateException.class)
|
||||
public void testOutOfOrderPrompterInvalidationThrows() {
|
||||
// GIVEN a NotifPromoter that gets invalidated during the sorting stage
|
||||
public void testOutOfOrderPreGroupFilterInvalidationThrowsAfterTooManyRuns() {
|
||||
// GIVEN a PreGroupNotifFilter that gets invalidated during the grouping stage,
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(filter);
|
||||
OnBeforeTransformGroupsListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addPreGroupFilter(filter);
|
||||
mListBuilder.addOnBeforeTransformGroupsListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the filter is invalidated more than
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 1);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception IS thrown.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNonConsecutiveOutOfOrderInvalidationDontThrowAfterTooManyRuns() {
|
||||
// GIVEN a PreGroupNotifFilter that gets invalidated during the grouping stage,
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(filter);
|
||||
OnBeforeTransformGroupsListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addPreGroupFilter(filter);
|
||||
mListBuilder.addOnBeforeTransformGroupsListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the filter is invalidated at least
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception is NOT thrown.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOutOfOrderPrompterInvalidationDoesNotThrowBeforeTooManyRuns() {
|
||||
// GIVEN a NotifPromoter that gets invalidated during the sorting stage,
|
||||
NotifPromoter promoter = new IdPromoter(47);
|
||||
OnBeforeSortListener listener =
|
||||
(list) -> promoter.invalidateList(null);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(promoter);
|
||||
OnBeforeSortListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addPromoter(promoter);
|
||||
mListBuilder.addOnBeforeSortListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the promoter is invalidated
|
||||
// WHEN we try to run the pipeline and the promoter is invalidated exactly
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_1);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception is thrown
|
||||
// THEN an exception is NOT thrown.
|
||||
}
|
||||
|
||||
@Test(expected = IllegalStateException.class)
|
||||
public void testOutOfOrderComparatorInvalidationThrows() {
|
||||
// GIVEN a NotifComparator that gets invalidated during the finalizing stage
|
||||
NotifComparator comparator = new HypeComparator(PACKAGE_5);
|
||||
OnBeforeRenderListListener listener =
|
||||
(list) -> comparator.invalidateList(null);
|
||||
public void testOutOfOrderPrompterInvalidationThrowsAfterTooManyRuns() {
|
||||
// GIVEN a NotifPromoter that gets invalidated during the sorting stage,
|
||||
NotifPromoter promoter = new IdPromoter(47);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(promoter);
|
||||
OnBeforeSortListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addPromoter(promoter);
|
||||
mListBuilder.addOnBeforeSortListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the promoter is invalidated more than
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_1);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 1);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception IS thrown.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOutOfOrderComparatorInvalidationDoesNotThrowBeforeTooManyRuns() {
|
||||
// GIVEN a NotifComparator that gets invalidated during the finalizing stage,
|
||||
NotifComparator comparator = new HypeComparator(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(comparator);
|
||||
OnBeforeRenderListListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.setComparators(singletonList(comparator));
|
||||
mListBuilder.addOnBeforeRenderListListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the comparator is invalidated
|
||||
addNotif(0, PACKAGE_1);
|
||||
// WHEN we try to run the pipeline and the comparator is invalidated exactly
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception is thrown
|
||||
// THEN an exception is NOT thrown.
|
||||
}
|
||||
|
||||
@Test(expected = IllegalStateException.class)
|
||||
public void testOutOfOrderPreRenderFilterInvalidationThrows() {
|
||||
// GIVEN a PreRenderNotifFilter that gets invalidated during the finalizing stage
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_5);
|
||||
OnBeforeRenderListListener listener = (list) -> filter.invalidateList(null);
|
||||
public void testOutOfOrderComparatorInvalidationThrowsAfterTooManyRuns() {
|
||||
// GIVEN a NotifComparator that gets invalidated during the finalizing stage,
|
||||
NotifComparator comparator = new HypeComparator(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(comparator);
|
||||
OnBeforeRenderListListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.setComparators(singletonList(comparator));
|
||||
mListBuilder.addOnBeforeRenderListListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the comparator is invalidated more than
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 1);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception IS thrown.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOutOfOrderPreRenderFilterInvalidationDoesNotThrowBeforeTooManyRuns() {
|
||||
// GIVEN a PreRenderNotifFilter that gets invalidated during the finalizing stage,
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(filter);
|
||||
OnBeforeRenderListListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addFinalizeFilter(filter);
|
||||
mListBuilder.addOnBeforeRenderListListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the PreRenderFilter is invalidated
|
||||
addNotif(0, PACKAGE_1);
|
||||
// WHEN we try to run the pipeline and the PreRenderFilter is invalidated exactly
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception is thrown
|
||||
// THEN an exception is NOT thrown.
|
||||
}
|
||||
|
||||
@Test(expected = IllegalStateException.class)
|
||||
public void testOutOfOrderPreRenderFilterInvalidationThrowsAfterTooManyRuns() {
|
||||
// GIVEN a PreRenderNotifFilter that gets invalidated during the finalizing stage,
|
||||
NotifFilter filter = new PackageFilter(PACKAGE_1);
|
||||
CountingInvalidator invalidator = new CountingInvalidator(filter);
|
||||
OnBeforeRenderListListener listener = (list) -> invalidator.maybeInvalidate();
|
||||
mListBuilder.addFinalizeFilter(filter);
|
||||
mListBuilder.addOnBeforeRenderListListener(listener);
|
||||
|
||||
// WHEN we try to run the pipeline and the PreRenderFilter is invalidated more than
|
||||
// MAX_CONSECUTIVE_REENTRANT_REBUILDS times,
|
||||
addNotif(0, PACKAGE_2);
|
||||
invalidator.setInvalidationCount(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 1);
|
||||
dispatchBuild();
|
||||
runWhileScheduledUpTo(ShadeListBuilder.MAX_CONSECUTIVE_REENTRANT_REBUILDS + 2);
|
||||
|
||||
// THEN an exception IS thrown.
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -2096,6 +2232,18 @@ public class ShadeListBuilderTest extends SysuiTestCase {
|
||||
mPipelineChoreographer.runIfScheduled();
|
||||
}
|
||||
|
||||
private void runWhileScheduledUpTo(int maxRuns) {
|
||||
int runs = 0;
|
||||
while (mPipelineChoreographer.isScheduled()) {
|
||||
if (runs > maxRuns) {
|
||||
throw new IndexOutOfBoundsException(
|
||||
"Pipeline scheduled itself more than " + maxRuns + "times");
|
||||
}
|
||||
runs++;
|
||||
mPipelineChoreographer.runIfScheduled();
|
||||
}
|
||||
}
|
||||
|
||||
private void verifyBuiltList(ExpectedEntry ...expectedEntries) {
|
||||
try {
|
||||
assertEquals(
|
||||
|
||||
Reference in New Issue
Block a user