diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographer.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographer.kt new file mode 100644 index 0000000000000..6f8e5da70497e --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographer.kt @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2022 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 + +import android.view.Choreographer +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Main +import com.android.systemui.util.ListenerSet +import com.android.systemui.util.concurrency.DelayableExecutor +import dagger.Module +import dagger.Provides + +/** + * Choreographs evaluation resulting from multiple asynchronous sources. Specifically, it exposes + * [schedule], and [addOnEvalListener]; the former will "schedule" an asynchronous invocation of the + * latter. Multiple invocations of [schedule] before any added listeners are invoked have no effect. + */ +interface NotifPipelineChoreographer { + /** + * Schedules all listeners registered with [addOnEvalListener] to be asynchronously executed at + * some point in the future. The exact timing is up to the implementation. + */ + fun schedule() + + /** Cancels a pending evaluation triggered by any recent calls to [schedule]. */ + fun cancel() + + /** Adds a listener [Runnable] that will be invoked when the scheduled evaluation occurs. */ + fun addOnEvalListener(onEvalListener: Runnable) + + /** Removes a listener previously registered with [addOnEvalListener]. */ + fun removeOnEvalListener(onEvalListener: Runnable) +} + +@Module +object NotifPipelineChoreographerModule { + @Provides + @JvmStatic + @SysUISingleton + fun provideChoreographer( + choreographer: Choreographer, + @Main mainExecutor: DelayableExecutor + ): NotifPipelineChoreographer = NotifPipelineChoreographerImpl(choreographer, mainExecutor) +} + +private const val TIMEOUT_MS: Long = 100 + +private class NotifPipelineChoreographerImpl( + private val viewChoreographer: Choreographer, + private val executor: DelayableExecutor +) : NotifPipelineChoreographer { + + private val listeners = ListenerSet() + private var timeoutSubscription: Runnable? = null + private var isScheduled = false + + private val frameCallback = Choreographer.FrameCallback { + if (isScheduled) { + isScheduled = false + timeoutSubscription?.run() + listeners.forEach { it.run() } + } + } + + override fun schedule() { + if (isScheduled) return + isScheduled = true + viewChoreographer.postFrameCallback(frameCallback) + if (!isScheduled) { + // Guard against synchronous evaluation of the frame callback. + return + } + timeoutSubscription = executor.executeDelayed(::onTimeout, TIMEOUT_MS) + } + + override fun cancel() { + if (!isScheduled) return + timeoutSubscription?.run() + viewChoreographer.removeFrameCallback(frameCallback) + } + + override fun addOnEvalListener(onEvalListener: Runnable) { + listeners.addIfAbsent(onEvalListener) + } + + override fun removeOnEvalListener(onEvalListener: Runnable) { + listeners.remove(onEvalListener) + } + + private fun onTimeout() { + if (isScheduled) { + isScheduled = false + viewChoreographer.removeFrameCallback(frameCallback) + listeners.forEach { it.run() } + } + } +} + +class FakeNotifPipelineChoreographer : NotifPipelineChoreographer { + + var isScheduled = false + val listeners = ListenerSet() + + fun runIfScheduled() { + if (isScheduled) { + isScheduled = false + listeners.forEach { it.run() } + } + } + + override fun schedule() { + isScheduled = true + } + + override fun cancel() { + isScheduled = false + } + + override fun addOnEvalListener(onEvalListener: Runnable) { + listeners.addIfAbsent(onEvalListener) + } + + override fun removeOnEvalListener(onEvalListener: Runnable) { + listeners.remove(onEvalListener) + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index b328ae8cd0bb2..e0c2fd5632262 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -122,20 +122,23 @@ public class ShadeListBuilder implements Dumpable { private List mReadOnlyNotifList = Collections.unmodifiableList(mNotifList); private List mReadOnlyNewNotifList = Collections.unmodifiableList(mNewNotifList); + private final NotifPipelineChoreographer mChoreographer; @Inject public ShadeListBuilder( - SystemClock systemClock, - NotifPipelineFlags flags, - ShadeListBuilderLogger logger, DumpManager dumpManager, - NotificationInteractionTracker interactionTracker + NotifPipelineChoreographer pipelineChoreographer, + NotifPipelineFlags flags, + NotificationInteractionTracker interactionTracker, + ShadeListBuilderLogger logger, + SystemClock systemClock ) { Assert.isMainThread(); mSystemClock = systemClock; mLogger = logger; mAlwaysLogList = flags.isDevLoggingEnabled(); mInteractionTracker = interactionTracker; + mChoreographer = pipelineChoreographer; dumpManager.registerDumpable(TAG, this); setSectioners(Collections.emptyList()); @@ -148,6 +151,7 @@ public class ShadeListBuilder implements Dumpable { public void attach(NotifCollection collection) { Assert.isMainThread(); collection.setBuildListener(mReadyForBuildListener); + mChoreographer.addOnEvalListener(this::buildList); } /** @@ -290,7 +294,7 @@ public class ShadeListBuilder implements Dumpable { mLogger.logOnBuildList(); mAllEntries = entries; - buildList(); + mChoreographer.schedule(); } }; @@ -1281,7 +1285,7 @@ public class ShadeListBuilder implements Dumpable { private void rebuildListIfBefore(@PipelineState.StateName int state) { mPipelineState.requireIsBefore(state); if (mPipelineState.is(STATE_IDLE)) { - buildList(); + mChoreographer.schedule(); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index 45a9092068e43..35d4582d9dee8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java @@ -47,6 +47,7 @@ import com.android.systemui.statusbar.notification.collection.NotifInflaterImpl; import com.android.systemui.statusbar.notification.collection.NotifLiveDataStore; import com.android.systemui.statusbar.notification.collection.NotifLiveDataStoreImpl; import com.android.systemui.statusbar.notification.collection.NotifPipeline; +import com.android.systemui.statusbar.notification.collection.NotifPipelineChoreographerModule; import com.android.systemui.statusbar.notification.collection.coordinator.ShadeEventCoordinator; import com.android.systemui.statusbar.notification.collection.coordinator.VisualStabilityCoordinator; import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorsModule; @@ -103,6 +104,7 @@ import dagger.Provides; */ @Module(includes = { CoordinatorsModule.class, + NotifPipelineChoreographerModule.class, NotifPanelEventSourceModule.class, NotificationSectionHeadersModule.class, }) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographerTest.kt new file mode 100644 index 0000000000000..3820b98ab632b --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifPipelineChoreographerTest.kt @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2022 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 + +import android.testing.AndroidTestingRunner +import android.view.Choreographer +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.mock +import com.android.systemui.util.mockito.withArgCaptor +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.anyLong +import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` as whenever + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class NotifPipelineChoreographerTest : SysuiTestCase() { + + val viewChoreographer: Choreographer = mock() + val timeoueSubscription: Runnable = mock() + val executor: DelayableExecutor = mock().also { + whenever(it.executeDelayed(any(), anyLong())).thenReturn(timeoueSubscription) + } + + val pipelineChoreographer: NotifPipelineChoreographer = NotifPipelineChoreographerModule + .provideChoreographer(viewChoreographer, executor) + + @Test + fun scheduleThenEvalFrameCallback() { + // GIVEN a registered eval listener and scheduled choreographer + var hasEvaluated = false + pipelineChoreographer.addOnEvalListener { + hasEvaluated = true + } + pipelineChoreographer.schedule() + val frameCallback: Choreographer.FrameCallback = withArgCaptor { + verify(viewChoreographer).postFrameCallback(capture()) + } + // WHEN the choreographer would invoke its callback + frameCallback.doFrame(0) + // THEN the choreographer would evaluate, and the timeoutSubscription would have been + // cancelled + assertTrue(hasEvaluated) + verify(timeoueSubscription).run() + } + + @Test + fun scheduleThenEvalTimeoutCallback() { + // GIVEN a registered eval listener and scheduled choreographer + var hasEvaluated = false + pipelineChoreographer.addOnEvalListener { + hasEvaluated = true + } + pipelineChoreographer.schedule() + val frameCallback: Choreographer.FrameCallback = withArgCaptor { + verify(viewChoreographer).postFrameCallback(capture()) + } + val runnable: Runnable = withArgCaptor { + verify(executor).executeDelayed(capture(), anyLong()) + } + // WHEN the executor would invoke its callback (indicating a timeout) + runnable.run() + // THEN the choreographer would evaluate, and the FrameCallback would have been unregistered + assertTrue(hasEvaluated) + verify(viewChoreographer).removeFrameCallback(frameCallback) + } + + @Test + fun scheduleThenCancel() { + // GIVEN a scheduled choreographer + pipelineChoreographer.schedule() + val frameCallback: Choreographer.FrameCallback = withArgCaptor { + verify(viewChoreographer).postFrameCallback(capture()) + } + // WHEN the scheduled run is cancelled + pipelineChoreographer.cancel() + // THEN both the FrameCallback is unregistered and the timeout subscription is cancelled. + verify(viewChoreographer).removeFrameCallback(frameCallback) + verify(timeoueSubscription).run() + } +} \ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index cb248b05e4bd6..f470715b92c4c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -111,6 +111,8 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Captor private ArgumentCaptor mBuildListenerCaptor; + private final FakeNotifPipelineChoreographer mPipelineChoreographer = + new FakeNotifPipelineChoreographer(); private CollectionReadyForBuildListener mReadyForBuildListener; private List mPendingSet = new ArrayList<>(); private List mEntrySet = new ArrayList<>(); @@ -127,11 +129,12 @@ public class ShadeListBuilderTest extends SysuiTestCase { allowTestableLooperAsMainThread(); mListBuilder = new ShadeListBuilder( - mSystemClock, - mNotifPipelineFlags, - mLogger, mDumpManager, - mInteractionTracker + mPipelineChoreographer, + mNotifPipelineFlags, + mInteractionTracker, + mLogger, + mSystemClock ); mListBuilder.setOnRenderListListener(mOnRenderListListener); @@ -567,6 +570,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN the pipeline is kicked off mReadyForBuildListener.onBuildList(singletonList(entry)); + mPipelineChoreographer.runIfScheduled(); // THEN the entry's initialization time is reset assertFalse(entry.hasFinishedInitialization()); @@ -1024,26 +1028,38 @@ public class ShadeListBuilderTest extends SysuiTestCase { clearInvocations(mOnRenderListListener); packageFilter.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); idPromoter.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); section.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); hypeComparator.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); sectionComparator.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); clearInvocations(mOnRenderListListener); preRenderInvalidator.invalidateList(); + assertTrue(mPipelineChoreographer.isScheduled()); + mPipelineChoreographer.runIfScheduled(); verify(mOnRenderListListener).onRenderList(anyList()); } @@ -1515,6 +1531,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN visual stability manager allows group changes again mStabilityManager.setAllowGroupChanges(true); mStabilityManager.invalidateList(); + mPipelineChoreographer.runIfScheduled(); // THEN entries are grouped verifyBuiltList( @@ -1553,6 +1570,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { // WHEN section changes are allowed again mStabilityManager.setAllowSectionChanges(true); mStabilityManager.invalidateList(); + mPipelineChoreographer.runIfScheduled(); // THEN the section updates assertEquals(newSectioner, mEntrySet.get(0).getSection().getSectioner()); @@ -1772,6 +1790,30 @@ public class ShadeListBuilderTest extends SysuiTestCase { verifyBuiltList(); } + @Test + public void testMultipleInvalidationsCoalesce() { + // GIVEN a PreGroupFilter and a FinalizeFilter + NotifFilter filter1 = new PackageFilter(PACKAGE_5); + NotifFilter filter2 = new PackageFilter(PACKAGE_0); + mListBuilder.addPreGroupFilter(filter1); + mListBuilder.addFinalizeFilter(filter2); + + // WHEN both filters invalidate + filter1.invalidateList(); + filter2.invalidateList(); + + // THEN the pipeline choreographer is scheduled to evaluate, AND the pipeline hasn't + // actually run. + assertTrue(mPipelineChoreographer.isScheduled()); + verify(mOnRenderListListener, never()).onRenderList(anyList()); + + // WHEN the pipeline choreographer actually runs + mPipelineChoreographer.runIfScheduled(); + + // THEN the pipeline runs + verify(mOnRenderListListener).onRenderList(anyList()); + } + @Test public void testIsSorted() { Comparator intCmp = Integer::compare; @@ -1914,6 +1956,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { } mReadyForBuildListener.onBuildList(mEntrySet); + mPipelineChoreographer.runIfScheduled(); } private void verifyBuiltList(ExpectedEntry ...expectedEntries) {